| Message ID | 20230606115802.79339-1-heikki.krogerus@linux.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c4a8bfabefed706bb9150867db528ceefd5cb5fe |
| Headers | show |
| Series | usb: typec: ucsi: Fix command cancellation | expand |
On 06.06.23 13:58, Heikki Krogerus wrote: > The Cancel command was passed to the write callback as the > offset instead of as the actual command which caused NULL > pointer dereference. > > Reported-by: Stephan Bolten <stephan.bolten@gmx.net> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217517 > Fixes: 094902bc6a3c ("usb: typec: ucsi: Always cancel the command if PPM reports BUSY condition") > Cc: stable@vger.kernel.org > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> Gentle reminder that this made no progress for a week now. Or was there and I just missed it? Then apologies in advance. I'm asking, as it afaics would be nice to have this (or some other fix for the regression linked above) mainlined before the next -rc. That would be ideal, as then it can get at least one week of testing before the final is released. Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page. > drivers/usb/typec/ucsi/ucsi.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c > index 2b472ec01dc42..b664ecbb798be 100644 > --- a/drivers/usb/typec/ucsi/ucsi.c > +++ b/drivers/usb/typec/ucsi/ucsi.c > @@ -132,10 +132,8 @@ static int ucsi_exec_command(struct ucsi *ucsi, u64 cmd) > if (ret) > return ret; > > - if (cci & UCSI_CCI_BUSY) { > - ucsi->ops->async_write(ucsi, UCSI_CANCEL, NULL, 0); > - return -EBUSY; > - } > + if (cmd != UCSI_CANCEL && cci & UCSI_CCI_BUSY) > + return ucsi_exec_command(ucsi, UCSI_CANCEL); > > if (!(cci & UCSI_CCI_COMMAND_COMPLETE)) > return -EIO; > @@ -149,6 +147,11 @@ static int ucsi_exec_command(struct ucsi *ucsi, u64 cmd) > return ucsi_read_error(ucsi); > } > > + if (cmd == UCSI_CANCEL && cci & UCSI_CCI_CANCEL_COMPLETE) { > + ret = ucsi_acknowledge_command(ucsi); > + return ret ? ret : -EBUSY; > + } > + > return UCSI_CCI_LENGTH(cci); > } >
On Tue, Jun 13, 2023 at 04:51:58PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote: > On 06.06.23 13:58, Heikki Krogerus wrote: > > The Cancel command was passed to the write callback as the > > offset instead of as the actual command which caused NULL > > pointer dereference. > > > > Reported-by: Stephan Bolten <stephan.bolten@gmx.net> > > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217517 > > Fixes: 094902bc6a3c ("usb: typec: ucsi: Always cancel the command if PPM reports BUSY condition") > > Cc: stable@vger.kernel.org > > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > Gentle reminder that this made no progress for a week now. Or was there > and I just missed it? Then apologies in advance. This just landed in my usb-linus branch a few hours before you sent this, and will show up in linux-next tomorrow as: c4a8bfabefed ("usb: typec: ucsi: Fix command cancellation") > I'm asking, as it afaics would be nice to have this (or some other fix > for the regression linked above) mainlined before the next -rc. That > would be ideal, as then it can get at least one week of testing before > the final is released. It will get there, sorry for the delay, now caught up on all pending USB and TTY/serial fixes. greg k-h
On 13.06.23 17:09, Greg Kroah-Hartman wrote: > On Tue, Jun 13, 2023 at 04:51:58PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote: >> On 06.06.23 13:58, Heikki Krogerus wrote: >>> The Cancel command was passed to the write callback as the >>> offset instead of as the actual command which caused NULL >>> pointer dereference. >>> >>> Reported-by: Stephan Bolten <stephan.bolten@gmx.net> >>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217517 >>> Fixes: 094902bc6a3c ("usb: typec: ucsi: Always cancel the command if PPM reports BUSY condition") >>> Cc: stable@vger.kernel.org >>> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> >> >> Gentle reminder that this made no progress for a week now. Or was there >> and I just missed it? Then apologies in advance. > > This just landed in my usb-linus branch a few hours before you sent > this, and will show up in linux-next tomorrow as: > c4a8bfabefed ("usb: typec: ucsi: Fix command cancellation") Ahh, great! Sorry, I check next in cases like this before sending mails, but not the subsystem trees directly. :-/ >> I'm asking, as it afaics would be nice to have this (or some other fix >> for the regression linked above) mainlined before the next -rc. That >> would be ideal, as then it can get at least one week of testing before >> the final is released. > > It will get there, sorry for the delay, now caught up on all pending USB > and TTY/serial fixes. No worries and thx for the update. It just looked like something where a quick "what's up" seemed appropriate. Thx again. Ciao, Thorsten
On Tue, Jun 13, 2023 at 05:36:50PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote: > On 13.06.23 17:09, Greg Kroah-Hartman wrote: > > On Tue, Jun 13, 2023 at 04:51:58PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote: > >> On 06.06.23 13:58, Heikki Krogerus wrote: > >>> The Cancel command was passed to the write callback as the > >>> offset instead of as the actual command which caused NULL > >>> pointer dereference. > >>> > >>> Reported-by: Stephan Bolten <stephan.bolten@gmx.net> > >>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217517 > >>> Fixes: 094902bc6a3c ("usb: typec: ucsi: Always cancel the command if PPM reports BUSY condition") > >>> Cc: stable@vger.kernel.org > >>> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > >> > >> Gentle reminder that this made no progress for a week now. Or was there > >> and I just missed it? Then apologies in advance. > > > > This just landed in my usb-linus branch a few hours before you sent > > this, and will show up in linux-next tomorrow as: > > c4a8bfabefed ("usb: typec: ucsi: Fix command cancellation") > > Ahh, great! Sorry, I check next in cases like this before sending mails, > but not the subsystem trees directly. :-/ I wouldn't expect you to look in subsystem trees, not a problem at all, thanks for the ping, you're doing great work here. greg k-h
diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c index 2b472ec01dc42..b664ecbb798be 100644 --- a/drivers/usb/typec/ucsi/ucsi.c +++ b/drivers/usb/typec/ucsi/ucsi.c @@ -132,10 +132,8 @@ static int ucsi_exec_command(struct ucsi *ucsi, u64 cmd) if (ret) return ret; - if (cci & UCSI_CCI_BUSY) { - ucsi->ops->async_write(ucsi, UCSI_CANCEL, NULL, 0); - return -EBUSY; - } + if (cmd != UCSI_CANCEL && cci & UCSI_CCI_BUSY) + return ucsi_exec_command(ucsi, UCSI_CANCEL); if (!(cci & UCSI_CCI_COMMAND_COMPLETE)) return -EIO; @@ -149,6 +147,11 @@ static int ucsi_exec_command(struct ucsi *ucsi, u64 cmd) return ucsi_read_error(ucsi); } + if (cmd == UCSI_CANCEL && cci & UCSI_CCI_CANCEL_COMPLETE) { + ret = ucsi_acknowledge_command(ucsi); + return ret ? ret : -EBUSY; + } + return UCSI_CCI_LENGTH(cci); }
The Cancel command was passed to the write callback as the offset instead of as the actual command which caused NULL pointer dereference. Reported-by: Stephan Bolten <stephan.bolten@gmx.net> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217517 Fixes: 094902bc6a3c ("usb: typec: ucsi: Always cancel the command if PPM reports BUSY condition") Cc: stable@vger.kernel.org Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> --- drivers/usb/typec/ucsi/ucsi.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)