| Message ID | 20230109081554.3792547-1-tzungbi@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 8bb233b27fb7c11deefbe2318e75490b22cf3d1a |
| Headers | show |
| Series | platform/chrome: cros_ec_uart: fix negative type promoted to high | expand |
> 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.
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.
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?
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
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 >
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!
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!
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; }
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(-)