* Re: [PATCH] USB: legousbtower: fix a signedness bug in tower_probe()
@ 2019-10-11 13:51 ` walter harms
0 siblings, 0 replies; 14+ messages in thread
From: walter harms @ 2019-10-11 13:51 UTC (permalink / raw)
To: Dan Carpenter
Cc: Juergen Stuber, Johan Hovold, Greg Kroah-Hartman, legousb-devel,
linux-usb, kernel-janitors
Am 11.10.2019 15:35, schrieb Dan Carpenter:
> The problem is that sizeof() is unsigned long so negative error codes
> are type promoted to high positive values and the condition becomes
> false.
>
> Fixes: 1d427be4a39d ("USB: legousbtower: fix slab info leak at probe")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> drivers/usb/misc/legousbtower.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
> index 9d4c52a7ebe0..835908fe1e65 100644
> --- a/drivers/usb/misc/legousbtower.c
> +++ b/drivers/usb/misc/legousbtower.c
> @@ -881,7 +881,7 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
> get_version_reply,
> sizeof(*get_version_reply),
> 1000);
> - if (result < sizeof(*get_version_reply)) {
> + if (result < 0 || result < sizeof(*get_version_reply)) {
> if (result >= 0)
> result = -EIO;
> dev_err(idev, "get version request failed: %d\n", result);
i am not an USB expert but it seems that this is a complicated way
to check for result != sizeof(*get_version_reply).
re,
wh
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] USB: legousbtower: fix a signedness bug in tower_probe()
2019-10-11 13:51 ` walter harms
@ 2019-10-11 13:58 ` Dan Carpenter
-1 siblings, 0 replies; 14+ messages in thread
From: Dan Carpenter @ 2019-10-11 13:58 UTC (permalink / raw)
To: walter harms
Cc: Juergen Stuber, Johan Hovold, Greg Kroah-Hartman, legousb-devel,
linux-usb, kernel-janitors
On Fri, Oct 11, 2019 at 03:51:26PM +0200, walter harms wrote:
>
>
> Am 11.10.2019 15:35, schrieb Dan Carpenter:
> > The problem is that sizeof() is unsigned long so negative error codes
> > are type promoted to high positive values and the condition becomes
> > false.
> >
> > Fixes: 1d427be4a39d ("USB: legousbtower: fix slab info leak at probe")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > drivers/usb/misc/legousbtower.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
> > index 9d4c52a7ebe0..835908fe1e65 100644
> > --- a/drivers/usb/misc/legousbtower.c
> > +++ b/drivers/usb/misc/legousbtower.c
> > @@ -881,7 +881,7 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
> > get_version_reply,
> > sizeof(*get_version_reply),
> > 1000);
> > - if (result < sizeof(*get_version_reply)) {
> > + if (result < 0 || result < sizeof(*get_version_reply)) {
> > if (result >= 0)
> > result = -EIO;
> > dev_err(idev, "get version request failed: %d\n", result);
>
> i am not an USB expert but it seems that this is a complicated way
> to check for result != sizeof(*get_version_reply).
Yeah. You're right. That would look nicer. I will resend.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] USB: legousbtower: fix a signedness bug in tower_probe()
@ 2019-10-11 13:58 ` Dan Carpenter
0 siblings, 0 replies; 14+ messages in thread
From: Dan Carpenter @ 2019-10-11 13:58 UTC (permalink / raw)
To: walter harms
Cc: Juergen Stuber, Johan Hovold, Greg Kroah-Hartman, legousb-devel,
linux-usb, kernel-janitors
On Fri, Oct 11, 2019 at 03:51:26PM +0200, walter harms wrote:
>
>
> Am 11.10.2019 15:35, schrieb Dan Carpenter:
> > The problem is that sizeof() is unsigned long so negative error codes
> > are type promoted to high positive values and the condition becomes
> > false.
> >
> > Fixes: 1d427be4a39d ("USB: legousbtower: fix slab info leak at probe")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > drivers/usb/misc/legousbtower.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
> > index 9d4c52a7ebe0..835908fe1e65 100644
> > --- a/drivers/usb/misc/legousbtower.c
> > +++ b/drivers/usb/misc/legousbtower.c
> > @@ -881,7 +881,7 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
> > get_version_reply,
> > sizeof(*get_version_reply),
> > 1000);
> > - if (result < sizeof(*get_version_reply)) {
> > + if (result < 0 || result < sizeof(*get_version_reply)) {
> > if (result >= 0)
> > result = -EIO;
> > dev_err(idev, "get version request failed: %d\n", result);
>
> i am not an USB expert but it seems that this is a complicated way
> to check for result != sizeof(*get_version_reply).
Yeah. You're right. That would look nicer. I will resend.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] USB: legousbtower: fix a signedness bug in tower_probe()
2019-10-11 13:58 ` Dan Carpenter
@ 2019-10-11 14:08 ` Johan Hovold
-1 siblings, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2019-10-11 14:08 UTC (permalink / raw)
To: Dan Carpenter
Cc: walter harms, Juergen Stuber, Johan Hovold, Greg Kroah-Hartman,
legousb-devel, linux-usb, kernel-janitors
On Fri, Oct 11, 2019 at 04:58:56PM +0300, Dan Carpenter wrote:
> On Fri, Oct 11, 2019 at 03:51:26PM +0200, walter harms wrote:
> >
> >
> > Am 11.10.2019 15:35, schrieb Dan Carpenter:
> > > The problem is that sizeof() is unsigned long so negative error codes
> > > are type promoted to high positive values and the condition becomes
> > > false.
> > >
> > > Fixes: 1d427be4a39d ("USB: legousbtower: fix slab info leak at probe")
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > ---
> > > drivers/usb/misc/legousbtower.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
> > > index 9d4c52a7ebe0..835908fe1e65 100644
> > > --- a/drivers/usb/misc/legousbtower.c
> > > +++ b/drivers/usb/misc/legousbtower.c
> > > @@ -881,7 +881,7 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
> > > get_version_reply,
> > > sizeof(*get_version_reply),
> > > 1000);
> > > - if (result < sizeof(*get_version_reply)) {
> > > + if (result < 0 || result < sizeof(*get_version_reply)) {
> > > if (result >= 0)
> > > result = -EIO;
> > > dev_err(idev, "get version request failed: %d\n", result);
> >
> > i am not an USB expert but it seems that this is a complicated way
> > to check for result != sizeof(*get_version_reply).
>
> Yeah. You're right. That would look nicer. I will resend.
Your version, or adding an explicit cast to int, may be preferred as
they document that there's something to watch out for here.
Either way you have my ack.
Johan
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] USB: legousbtower: fix a signedness bug in tower_probe()
@ 2019-10-11 14:08 ` Johan Hovold
0 siblings, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2019-10-11 14:08 UTC (permalink / raw)
To: Dan Carpenter
Cc: walter harms, Juergen Stuber, Johan Hovold, Greg Kroah-Hartman,
legousb-devel, linux-usb, kernel-janitors
On Fri, Oct 11, 2019 at 04:58:56PM +0300, Dan Carpenter wrote:
> On Fri, Oct 11, 2019 at 03:51:26PM +0200, walter harms wrote:
> >
> >
> > Am 11.10.2019 15:35, schrieb Dan Carpenter:
> > > The problem is that sizeof() is unsigned long so negative error codes
> > > are type promoted to high positive values and the condition becomes
> > > false.
> > >
> > > Fixes: 1d427be4a39d ("USB: legousbtower: fix slab info leak at probe")
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > ---
> > > drivers/usb/misc/legousbtower.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
> > > index 9d4c52a7ebe0..835908fe1e65 100644
> > > --- a/drivers/usb/misc/legousbtower.c
> > > +++ b/drivers/usb/misc/legousbtower.c
> > > @@ -881,7 +881,7 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
> > > get_version_reply,
> > > sizeof(*get_version_reply),
> > > 1000);
> > > - if (result < sizeof(*get_version_reply)) {
> > > + if (result < 0 || result < sizeof(*get_version_reply)) {
> > > if (result >= 0)
> > > result = -EIO;
> > > dev_err(idev, "get version request failed: %d\n", result);
> >
> > i am not an USB expert but it seems that this is a complicated way
> > to check for result != sizeof(*get_version_reply).
>
> Yeah. You're right. That would look nicer. I will resend.
Your version, or adding an explicit cast to int, may be preferred as
they document that there's something to watch out for here.
Either way you have my ack.
Johan
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] USB: legousbtower: fix a signedness bug in tower_probe()
2019-10-11 13:51 ` walter harms
@ 2019-10-11 14:11 ` Dan Carpenter
-1 siblings, 0 replies; 14+ messages in thread
From: Dan Carpenter @ 2019-10-11 14:11 UTC (permalink / raw)
To: Juergen Stuber, Johan Hovold
Cc: Greg Kroah-Hartman, legousb-devel, linux-usb, kernel-janitors,
walter harms
The problem is that sizeof() is unsigned long so negative error codes
are type promoted to high positive values and the condition becomes
false.
Fixes: 1d427be4a39d ("USB: legousbtower: fix slab info leak at probe")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: style improvement suggested by Walter Harms.
drivers/usb/misc/legousbtower.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
index 9d4c52a7ebe0..9bd240df8f4c 100644
--- a/drivers/usb/misc/legousbtower.c
+++ b/drivers/usb/misc/legousbtower.c
@@ -881,7 +881,7 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
get_version_reply,
sizeof(*get_version_reply),
1000);
- if (result < sizeof(*get_version_reply)) {
+ if (result != sizeof(*get_version_reply)) {
if (result >= 0)
result = -EIO;
dev_err(idev, "get version request failed: %d\n", result);
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2] USB: legousbtower: fix a signedness bug in tower_probe()
@ 2019-10-11 14:11 ` Dan Carpenter
0 siblings, 0 replies; 14+ messages in thread
From: Dan Carpenter @ 2019-10-11 14:11 UTC (permalink / raw)
To: Juergen Stuber, Johan Hovold
Cc: Greg Kroah-Hartman, legousb-devel, linux-usb, kernel-janitors,
walter harms
The problem is that sizeof() is unsigned long so negative error codes
are type promoted to high positive values and the condition becomes
false.
Fixes: 1d427be4a39d ("USB: legousbtower: fix slab info leak at probe")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: style improvement suggested by Walter Harms.
drivers/usb/misc/legousbtower.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
index 9d4c52a7ebe0..9bd240df8f4c 100644
--- a/drivers/usb/misc/legousbtower.c
+++ b/drivers/usb/misc/legousbtower.c
@@ -881,7 +881,7 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
get_version_reply,
sizeof(*get_version_reply),
1000);
- if (result < sizeof(*get_version_reply)) {
+ if (result != sizeof(*get_version_reply)) {
if (result >= 0)
result = -EIO;
dev_err(idev, "get version request failed: %d\n", result);
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2] USB: legousbtower: fix a signedness bug in tower_probe()
2019-10-11 14:11 ` Dan Carpenter
@ 2019-10-11 14:23 ` Johan Hovold
-1 siblings, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2019-10-11 14:23 UTC (permalink / raw)
To: Dan Carpenter
Cc: Juergen Stuber, Johan Hovold, Greg Kroah-Hartman, legousb-devel,
linux-usb, kernel-janitors, walter harms
On Fri, Oct 11, 2019 at 05:11:15PM +0300, Dan Carpenter wrote:
> The problem is that sizeof() is unsigned long so negative error codes
> are type promoted to high positive values and the condition becomes
> false.
>
> Fixes: 1d427be4a39d ("USB: legousbtower: fix slab info leak at probe")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Johan Hovold <johan@kernel.org>
> ---
> v2: style improvement suggested by Walter Harms.
>
> drivers/usb/misc/legousbtower.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
> index 9d4c52a7ebe0..9bd240df8f4c 100644
> --- a/drivers/usb/misc/legousbtower.c
> +++ b/drivers/usb/misc/legousbtower.c
> @@ -881,7 +881,7 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
> get_version_reply,
> sizeof(*get_version_reply),
> 1000);
> - if (result < sizeof(*get_version_reply)) {
> + if (result != sizeof(*get_version_reply)) {
> if (result >= 0)
> result = -EIO;
> dev_err(idev, "get version request failed: %d\n", result);
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2] USB: legousbtower: fix a signedness bug in tower_probe()
@ 2019-10-11 14:23 ` Johan Hovold
0 siblings, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2019-10-11 14:23 UTC (permalink / raw)
To: Dan Carpenter
Cc: Juergen Stuber, Johan Hovold, Greg Kroah-Hartman, legousb-devel,
linux-usb, kernel-janitors, walter harms
On Fri, Oct 11, 2019 at 05:11:15PM +0300, Dan Carpenter wrote:
> The problem is that sizeof() is unsigned long so negative error codes
> are type promoted to high positive values and the condition becomes
> false.
>
> Fixes: 1d427be4a39d ("USB: legousbtower: fix slab info leak at probe")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Johan Hovold <johan@kernel.org>
> ---
> v2: style improvement suggested by Walter Harms.
>
> drivers/usb/misc/legousbtower.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
> index 9d4c52a7ebe0..9bd240df8f4c 100644
> --- a/drivers/usb/misc/legousbtower.c
> +++ b/drivers/usb/misc/legousbtower.c
> @@ -881,7 +881,7 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
> get_version_reply,
> sizeof(*get_version_reply),
> 1000);
> - if (result < sizeof(*get_version_reply)) {
> + if (result != sizeof(*get_version_reply)) {
> if (result >= 0)
> result = -EIO;
> dev_err(idev, "get version request failed: %d\n", result);
^ permalink raw reply [flat|nested] 14+ messages in thread