Chrome platform driver development
 help / color / mirror / Atom feed
* [PATCH] platform/chrome: cros_ec_uart: fix negative type promoted to high
@ 2023-01-09  8:15 Tzung-Bi Shih
  2023-01-09 18:51 ` Mark Hasemeyer
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Tzung-Bi Shih @ 2023-01-09  8:15 UTC (permalink / raw)
  To: bleung, groeck; +Cc: chrome-platform, tzungbi, markhas, bhanumaiya, error27

serdev_device_write_buf() returns negative numbers on errors.  When
the return value compares to unsigned integer `len`, it promotes to
quite large positive number.

Fix it.

Fixes: 04a8bdd135cc ("platform/chrome: cros_ec_uart: Add transport layer")
Reported-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
Reported in https://lore.kernel.org/chrome-platform/Y7fhw4S7Tb0GcHMF@kili/T/#u.

 drivers/platform/chrome/cros_ec_uart.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_uart.c b/drivers/platform/chrome/cros_ec_uart.c
index 6916069f1599..788246559bbb 100644
--- a/drivers/platform/chrome/cros_ec_uart.c
+++ b/drivers/platform/chrome/cros_ec_uart.c
@@ -149,9 +149,10 @@ static int cros_ec_uart_pkt_xfer(struct cros_ec_device *ec_dev,
 	resp->status = 0;
 
 	ret = serdev_device_write_buf(serdev, ec_dev->dout, len);
-	if (ret < len) {
+	if (ret < 0 || ret < len) {
 		dev_err(ec_dev->dev, "Unable to write data\n");
-		ret = -EIO;
+		if (ret >= 0)
+			ret = -EIO;
 		goto exit;
 	}
 
-- 
2.39.0.314.g84b9a713c41-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] platform/chrome: cros_ec_uart: fix negative type promoted to high
  2023-01-09  8:15 [PATCH] platform/chrome: cros_ec_uart: fix negative type promoted to high Tzung-Bi Shih
@ 2023-01-09 18:51 ` Mark Hasemeyer
  2023-01-10  3:27   ` Tzung-Bi Shih
  2023-01-17  8:12 ` Tzung-Bi Shih
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Mark Hasemeyer @ 2023-01-09 18:51 UTC (permalink / raw)
  To: Tzung-Bi Shih; +Cc: bleung, groeck, chrome-platform, bhanumaiya, error27

>     175         if (host_response->data_len > ec_msg->insize) {
>     176                 dev_err(ec_dev->dev, "Resp too long (%d bytes, expected %d)\n",
>     177                         host_response->data_len, ec_msg->insize);
>     178                 ret = -ENOSPC;
>
> ret = -EINVAL;  (Unless you are discussing harddrives).
>
>     179                 goto exit;
>     180         }
The report also suggested modifying the return value if a response packet is too
long.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] platform/chrome: cros_ec_uart: fix negative type promoted to high
  2023-01-09 18:51 ` Mark Hasemeyer
@ 2023-01-10  3:27   ` Tzung-Bi Shih
  0 siblings, 0 replies; 8+ messages in thread
From: Tzung-Bi Shih @ 2023-01-10  3:27 UTC (permalink / raw)
  To: Mark Hasemeyer; +Cc: bleung, groeck, chrome-platform, bhanumaiya, error27

On Mon, Jan 09, 2023 at 11:51:56AM -0700, Mark Hasemeyer wrote:
> >     175         if (host_response->data_len > ec_msg->insize) {
> >     176                 dev_err(ec_dev->dev, "Resp too long (%d bytes, expected %d)\n",
> >     177                         host_response->data_len, ec_msg->insize);
> >     178                 ret = -ENOSPC;
> >
> > ret = -EINVAL;  (Unless you are discussing harddrives).
> >
> >     179                 goto exit;
> >     180         }
> The report also suggested modifying the return value if a response packet is too
> long.

Replied on the original mail.  Let's discuss the case there if possible.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] platform/chrome: cros_ec_uart: fix negative type promoted to high
  2023-01-09  8:15 [PATCH] platform/chrome: cros_ec_uart: fix negative type promoted to high Tzung-Bi Shih
  2023-01-09 18:51 ` Mark Hasemeyer
@ 2023-01-17  8:12 ` Tzung-Bi Shih
  2023-01-17  8:19   ` Dan Carpenter
  2023-01-17 14:45 ` Benson Leung
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Tzung-Bi Shih @ 2023-01-17  8:12 UTC (permalink / raw)
  To: bleung, groeck; +Cc: chrome-platform, markhas, bhanumaiya, error27

On Mon, Jan 09, 2023 at 04:15:54PM +0800, Tzung-Bi Shih wrote:
> serdev_device_write_buf() returns negative numbers on errors.  When
> the return value compares to unsigned integer `len`, it promotes to
> quite large positive number.
> 
> Fix it.
> 
> Fixes: 04a8bdd135cc ("platform/chrome: cros_ec_uart: Add transport layer")
> Reported-by: Dan Carpenter <error27@gmail.com>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> ---
> Reported in https://lore.kernel.org/chrome-platform/Y7fhw4S7Tb0GcHMF@kili/T/#u.
> 
>  drivers/platform/chrome/cros_ec_uart.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/chrome/cros_ec_uart.c b/drivers/platform/chrome/cros_ec_uart.c
> index 6916069f1599..788246559bbb 100644
> --- a/drivers/platform/chrome/cros_ec_uart.c
> +++ b/drivers/platform/chrome/cros_ec_uart.c
> @@ -149,9 +149,10 @@ static int cros_ec_uart_pkt_xfer(struct cros_ec_device *ec_dev,
>  	resp->status = 0;
>  
>  	ret = serdev_device_write_buf(serdev, ec_dev->dout, len);
> -	if (ret < len) {
> +	if (ret < 0 || ret < len) {
>  		dev_err(ec_dev->dev, "Unable to write data\n");
> -		ret = -EIO;
> +		if (ret >= 0)
> +			ret = -EIO;
>  		goto exit;
>  	}

Does this patch make sense to someone in the mailing list?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] platform/chrome: cros_ec_uart: fix negative type promoted to high
  2023-01-17  8:12 ` Tzung-Bi Shih
@ 2023-01-17  8:19   ` Dan Carpenter
  0 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2023-01-17  8:19 UTC (permalink / raw)
  To: Tzung-Bi Shih; +Cc: bleung, groeck, chrome-platform, markhas, bhanumaiya

On Tue, Jan 17, 2023 at 04:12:40PM +0800, Tzung-Bi Shih wrote:
> On Mon, Jan 09, 2023 at 04:15:54PM +0800, Tzung-Bi Shih wrote:
> > serdev_device_write_buf() returns negative numbers on errors.  When
> > the return value compares to unsigned integer `len`, it promotes to
> > quite large positive number.
> > 
> > Fix it.
> > 
> > Fixes: 04a8bdd135cc ("platform/chrome: cros_ec_uart: Add transport layer")
> > Reported-by: Dan Carpenter <error27@gmail.com>
> > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> > ---
> > Reported in https://lore.kernel.org/chrome-platform/Y7fhw4S7Tb0GcHMF@kili/T/#u.
> > 
> >  drivers/platform/chrome/cros_ec_uart.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/platform/chrome/cros_ec_uart.c b/drivers/platform/chrome/cros_ec_uart.c
> > index 6916069f1599..788246559bbb 100644
> > --- a/drivers/platform/chrome/cros_ec_uart.c
> > +++ b/drivers/platform/chrome/cros_ec_uart.c
> > @@ -149,9 +149,10 @@ static int cros_ec_uart_pkt_xfer(struct cros_ec_device *ec_dev,
> >  	resp->status = 0;
> >  
> >  	ret = serdev_device_write_buf(serdev, ec_dev->dout, len);
> > -	if (ret < len) {
> > +	if (ret < 0 || ret < len) {
> >  		dev_err(ec_dev->dev, "Unable to write data\n");
> > -		ret = -EIO;
> > +		if (ret >= 0)
> > +			ret = -EIO;
> >  		goto exit;
> >  	}
> 
> Does this patch make sense to someone in the mailing list?

Yeah.  It looks fine.

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] platform/chrome: cros_ec_uart: fix negative type promoted to high
  2023-01-09  8:15 [PATCH] platform/chrome: cros_ec_uart: fix negative type promoted to high Tzung-Bi Shih
  2023-01-09 18:51 ` Mark Hasemeyer
  2023-01-17  8:12 ` Tzung-Bi Shih
@ 2023-01-17 14:45 ` Benson Leung
  2023-01-18  3:00 ` patchwork-bot+chrome-platform
  2023-01-18  6:20 ` patchwork-bot+chrome-platform
  4 siblings, 0 replies; 8+ messages in thread
From: Benson Leung @ 2023-01-17 14:45 UTC (permalink / raw)
  To: Tzung-Bi Shih; +Cc: groeck, chrome-platform, markhas, bhanumaiya, error27

Hi Tzung-Bi,

On Mon, Jan 9, 2023 at 12:16 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> serdev_device_write_buf() returns negative numbers on errors.  When
> the return value compares to unsigned integer `len`, it promotes to
> quite large positive number.
>
> Fix it.
>
> Fixes: 04a8bdd135cc ("platform/chrome: cros_ec_uart: Add transport layer")
> Reported-by: Dan Carpenter <error27@gmail.com>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>

Reviewed-by: Benson Leung <bleung@chromium.org>

> ---
> Reported in https://lore.kernel.org/chrome-platform/Y7fhw4S7Tb0GcHMF@kili/T/#u.
>
>  drivers/platform/chrome/cros_ec_uart.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_uart.c b/drivers/platform/chrome/cros_ec_uart.c
> index 6916069f1599..788246559bbb 100644
> --- a/drivers/platform/chrome/cros_ec_uart.c
> +++ b/drivers/platform/chrome/cros_ec_uart.c
> @@ -149,9 +149,10 @@ static int cros_ec_uart_pkt_xfer(struct cros_ec_device *ec_dev,
>         resp->status = 0;
>
>         ret = serdev_device_write_buf(serdev, ec_dev->dout, len);
> -       if (ret < len) {
> +       if (ret < 0 || ret < len) {
>                 dev_err(ec_dev->dev, "Unable to write data\n");
> -               ret = -EIO;
> +               if (ret >= 0)
> +                       ret = -EIO;
>                 goto exit;
>         }
>
> --
> 2.39.0.314.g84b9a713c41-goog
>


-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] platform/chrome: cros_ec_uart: fix negative type promoted to high
  2023-01-09  8:15 [PATCH] platform/chrome: cros_ec_uart: fix negative type promoted to high Tzung-Bi Shih
                   ` (2 preceding siblings ...)
  2023-01-17 14:45 ` Benson Leung
@ 2023-01-18  3:00 ` patchwork-bot+chrome-platform
  2023-01-18  6:20 ` patchwork-bot+chrome-platform
  4 siblings, 0 replies; 8+ messages in thread
From: patchwork-bot+chrome-platform @ 2023-01-18  3:00 UTC (permalink / raw)
  To: Tzung-Bi Shih
  Cc: bleung, groeck, chrome-platform, markhas, bhanumaiya, error27

Hello:

This patch was applied to chrome-platform/linux.git (for-kernelci)
by Tzung-Bi Shih <tzungbi@kernel.org>:

On Mon,  9 Jan 2023 16:15:54 +0800 you wrote:
> serdev_device_write_buf() returns negative numbers on errors.  When
> the return value compares to unsigned integer `len`, it promotes to
> quite large positive number.
> 
> Fix it.
> 
> Fixes: 04a8bdd135cc ("platform/chrome: cros_ec_uart: Add transport layer")
> Reported-by: Dan Carpenter <error27@gmail.com>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> 
> [...]

Here is the summary with links:
  - platform/chrome: cros_ec_uart: fix negative type promoted to high
    https://git.kernel.org/chrome-platform/c/8bb233b27fb7

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] platform/chrome: cros_ec_uart: fix negative type promoted to high
  2023-01-09  8:15 [PATCH] platform/chrome: cros_ec_uart: fix negative type promoted to high Tzung-Bi Shih
                   ` (3 preceding siblings ...)
  2023-01-18  3:00 ` patchwork-bot+chrome-platform
@ 2023-01-18  6:20 ` patchwork-bot+chrome-platform
  4 siblings, 0 replies; 8+ messages in thread
From: patchwork-bot+chrome-platform @ 2023-01-18  6:20 UTC (permalink / raw)
  To: Tzung-Bi Shih
  Cc: bleung, groeck, chrome-platform, markhas, bhanumaiya, error27

Hello:

This patch was applied to chrome-platform/linux.git (for-next)
by Tzung-Bi Shih <tzungbi@kernel.org>:

On Mon,  9 Jan 2023 16:15:54 +0800 you wrote:
> serdev_device_write_buf() returns negative numbers on errors.  When
> the return value compares to unsigned integer `len`, it promotes to
> quite large positive number.
> 
> Fix it.
> 
> Fixes: 04a8bdd135cc ("platform/chrome: cros_ec_uart: Add transport layer")
> Reported-by: Dan Carpenter <error27@gmail.com>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> 
> [...]

Here is the summary with links:
  - platform/chrome: cros_ec_uart: fix negative type promoted to high
    https://git.kernel.org/chrome-platform/c/8bb233b27fb7

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-01-18  6:20 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-09  8:15 [PATCH] platform/chrome: cros_ec_uart: fix negative type promoted to high Tzung-Bi Shih
2023-01-09 18:51 ` Mark Hasemeyer
2023-01-10  3:27   ` Tzung-Bi Shih
2023-01-17  8:12 ` Tzung-Bi Shih
2023-01-17  8:19   ` Dan Carpenter
2023-01-17 14:45 ` Benson Leung
2023-01-18  3:00 ` patchwork-bot+chrome-platform
2023-01-18  6:20 ` patchwork-bot+chrome-platform

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox