[PATCH] usb: typec: ucsi: Fix command cancellation

[PATCH] usb: typec: ucsi: Fix command cancellation

[Heikki Krogerus]

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(-)

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);
 }
 
-- 
2.39.2

Re: [PATCH] usb: typec: ucsi: Fix command cancellation

[Linux regression tracking (Thorsten Leemhuis)]

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);
>  }
>  

Re: [PATCH] usb: typec: ucsi: Fix command cancellation

[Greg Kroah-Hartman]

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

Re: [PATCH] usb: typec: ucsi: Fix command cancellation

[Linux regression tracking (Thorsten Leemhuis)]

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

Re: [PATCH] usb: typec: ucsi: Fix command cancellation

[Greg Kroah-Hartman]

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