All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net: usb: pegasus: fix improper read if get_registers() fail
@ 2019-07-30 13:13 Denis Kirjanov
  2019-07-30 17:24 ` David Miller
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Denis Kirjanov @ 2019-07-30 13:13 UTC (permalink / raw)
  To: petkan, davem; +Cc: netdev, Denis Kirjanov

get_registers() may fail with -ENOMEM and in this
case we can read a garbage from the status variable tmp.

Reported-by: syzbot+3499a83b2d062ae409d4@syzkaller.appspotmail.com
Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
---
 drivers/net/usb/pegasus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index 6d25dea5ad4b..f7d117d80cfb 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -282,7 +282,7 @@ static void mdio_write(struct net_device *dev, int phy_id, int loc, int val)
 static int read_eprom_word(pegasus_t *pegasus, __u8 index, __u16 *retdata)
 {
 	int i;
-	__u8 tmp;
+	__u8 tmp = 0;
 	__le16 retdatai;
 	int ret;
 
-- 
2.12.3


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

* Re: [PATCH] net: usb: pegasus: fix improper read if get_registers() fail
  2019-07-30 13:13 [PATCH] net: usb: pegasus: fix improper read if get_registers() fail Denis Kirjanov
@ 2019-07-30 17:24 ` David Miller
  2019-07-30 18:23   ` Denis Kirjanov
       [not found]   ` <CAHj3AVm2EZB7n9UBxiBmA+6XN+EgAC_FRoHjh6kO3WMT8KVd6g@mail.gmail.com>
  2019-07-31 19:10 ` Petko Manolov
  2019-08-01 22:18 ` David Miller
  2 siblings, 2 replies; 8+ messages in thread
From: David Miller @ 2019-07-30 17:24 UTC (permalink / raw)
  To: kda; +Cc: petkan, netdev

From: Denis Kirjanov <kda@linux-powerpc.org>
Date: Tue, 30 Jul 2019 15:13:57 +0200

> get_registers() may fail with -ENOMEM and in this
> case we can read a garbage from the status variable tmp.
> 
> Reported-by: syzbot+3499a83b2d062ae409d4@syzkaller.appspotmail.com
> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>

Why did you post this patch twice?  What is different between the two
versions?

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

* Re: [PATCH] net: usb: pegasus: fix improper read if get_registers() fail
  2019-07-30 17:24 ` David Miller
@ 2019-07-30 18:23   ` Denis Kirjanov
       [not found]   ` <CAHj3AVm2EZB7n9UBxiBmA+6XN+EgAC_FRoHjh6kO3WMT8KVd6g@mail.gmail.com>
  1 sibling, 0 replies; 8+ messages in thread
From: Denis Kirjanov @ 2019-07-30 18:23 UTC (permalink / raw)
  To: David Miller; +Cc: petkan, netdev

On 7/30/19, David Miller <davem@davemloft.net> wrote:
> From: Denis Kirjanov <kda@linux-powerpc.org>
> Date: Tue, 30 Jul 2019 15:13:57 +0200
>
>> get_registers() may fail with -ENOMEM and in this
>> case we can read a garbage from the status variable tmp.
>>
>> Reported-by: syzbot+3499a83b2d062ae409d4@syzkaller.appspotmail.com
>> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
>
> Why did you post this patch twice?  What is different between the two
> versions?
>
Looks like it was the issue with git send-email. Sorry about that.
Do you want me to figure out the reason and resend?

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

* Re: [PATCH] net: usb: pegasus: fix improper read if get_registers() fail
       [not found]   ` <CAHj3AVm2EZB7n9UBxiBmA+6XN+EgAC_FRoHjh6kO3WMT8KVd6g@mail.gmail.com>
@ 2019-07-30 21:10     ` David Miller
  0 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2019-07-30 21:10 UTC (permalink / raw)
  To: kirjanov; +Cc: kda, petkan, netdev

From: Denis Kirjanov <kirjanov@gmail.com>
Date: Tue, 30 Jul 2019 21:19:46 +0300

> On Tuesday, July 30, 2019, David Miller <davem@davemloft.net> wrote:
> 
>> From: Denis Kirjanov <kda@linux-powerpc.org>
>> Date: Tue, 30 Jul 2019 15:13:57 +0200
>>
>> > get_registers() may fail with -ENOMEM and in this
>> > case we can read a garbage from the status variable tmp.
>> >
>> > Reported-by: syzbot+3499a83b2d062ae409d4@syzkaller.appspotmail.com
>> > Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
>>
>> Why did you post this patch twice?  What is different between the two
>> versions?
> 
> 
> Looks like it’s the issue with git send-email :/
> Do you want me to figure out the reason and resend?

No need, I was just curious.

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

* Re: [PATCH] net: usb: pegasus: fix improper read if get_registers() fail
  2019-07-30 13:13 [PATCH] net: usb: pegasus: fix improper read if get_registers() fail Denis Kirjanov
  2019-07-30 17:24 ` David Miller
@ 2019-07-31 19:10 ` Petko Manolov
  2019-07-31 19:14   ` Petko Manolov
  2019-08-01 22:18 ` David Miller
  2 siblings, 1 reply; 8+ messages in thread
From: Petko Manolov @ 2019-07-31 19:10 UTC (permalink / raw)
  To: Denis Kirjanov; +Cc: davem, netdev

On 19-07-30 15:13:57, Denis Kirjanov wrote:
> get_registers() may fail with -ENOMEM and in this
> case we can read a garbage from the status variable tmp.
> 
> Reported-by: syzbot+3499a83b2d062ae409d4@syzkaller.appspotmail.com
> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
> ---
>  drivers/net/usb/pegasus.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
> index 6d25dea5ad4b..f7d117d80cfb 100644
> --- a/drivers/net/usb/pegasus.c
> +++ b/drivers/net/usb/pegasus.c
> @@ -282,7 +282,7 @@ static void mdio_write(struct net_device *dev, int phy_id, int loc, int val)
>  static int read_eprom_word(pegasus_t *pegasus, __u8 index, __u16 *retdata)
>  {
>  	int i;
> -	__u8 tmp;
> +	__u8 tmp = 0;
>  	__le16 retdatai;
>  	int ret;

Unfortunately this patch does not fix anything.  Even if get_registers() fail 
with -ENOMEM the "for" loop will cover for it and will exit only if the 
operation was successful or the device got disconnected.  Please read the code 
carefully.

So while the patch is harmless it isn't solving a problem.


cheers,
Petko

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

* Re: [PATCH] net: usb: pegasus: fix improper read if get_registers() fail
  2019-07-31 19:10 ` Petko Manolov
@ 2019-07-31 19:14   ` Petko Manolov
  0 siblings, 0 replies; 8+ messages in thread
From: Petko Manolov @ 2019-07-31 19:14 UTC (permalink / raw)
  To: Denis Kirjanov; +Cc: davem, netdev

On 19-07-31 22:10:39, Petko Manolov wrote:
> On 19-07-30 15:13:57, Denis Kirjanov wrote:
> > get_registers() may fail with -ENOMEM and in this
> > case we can read a garbage from the status variable tmp.
> > 
> > Reported-by: syzbot+3499a83b2d062ae409d4@syzkaller.appspotmail.com
> > Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
> > ---
> >  drivers/net/usb/pegasus.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
> > index 6d25dea5ad4b..f7d117d80cfb 100644
> > --- a/drivers/net/usb/pegasus.c
> > +++ b/drivers/net/usb/pegasus.c
> > @@ -282,7 +282,7 @@ static void mdio_write(struct net_device *dev, int phy_id, int loc, int val)
> >  static int read_eprom_word(pegasus_t *pegasus, __u8 index, __u16 *retdata)
> >  {
> >  	int i;
> > -	__u8 tmp;
> > +	__u8 tmp = 0;
> >  	__le16 retdatai;
> >  	int ret;
> 
> Unfortunately this patch does not fix anything.  Even if get_registers() fail 
> with -ENOMEM the "for" loop will cover for it and will exit only if the 
> operation was successful or the device got disconnected.  Please read the code 
> carefully.
> 
> So while the patch is harmless it isn't solving a problem.

Actually i am wrong - if "tmp" contains a random value it may accidentally have 
the EPROM_DONE bit set.

Dave, please apply the patch.


thanks,
Petko

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

* Re: [PATCH] net: usb: pegasus: fix improper read if get_registers() fail
  2019-07-30 13:13 [PATCH] net: usb: pegasus: fix improper read if get_registers() fail Denis Kirjanov
  2019-07-30 17:24 ` David Miller
  2019-07-31 19:10 ` Petko Manolov
@ 2019-08-01 22:18 ` David Miller
  2 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2019-08-01 22:18 UTC (permalink / raw)
  To: kda; +Cc: petkan, netdev

From: Denis Kirjanov <kda@linux-powerpc.org>
Date: Tue, 30 Jul 2019 15:13:57 +0200

> get_registers() may fail with -ENOMEM and in this
> case we can read a garbage from the status variable tmp.
> 
> Reported-by: syzbot+3499a83b2d062ae409d4@syzkaller.appspotmail.com
> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>

Applied, thank you.

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

* [PATCH] net: usb: pegasus: fix improper read if get_registers() fail
@ 2019-07-30 12:45 Denis Kirjanov
  0 siblings, 0 replies; 8+ messages in thread
From: Denis Kirjanov @ 2019-07-30 12:45 UTC (permalink / raw)
  To: petkan, davem; +Cc: netdev, Denis Kirjanov

get_registers() may fail with -ENOMEM and in this
case we can read a garbage from the status variable tmp.

Reported-by: syzbot+3499a83b2d062ae409d4@syzkaller.appspotmail.com
Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
---
 drivers/net/usb/pegasus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index 6d25dea5ad4b..f7d117d80cfb 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -282,7 +282,7 @@ static void mdio_write(struct net_device *dev, int phy_id, int loc, int val)
 static int read_eprom_word(pegasus_t *pegasus, __u8 index, __u16 *retdata)
 {
 	int i;
-	__u8 tmp;
+	__u8 tmp = 0;
 	__le16 retdatai;
 	int ret;
 
-- 
2.12.3


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

end of thread, other threads:[~2019-08-01 22:18 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-30 13:13 [PATCH] net: usb: pegasus: fix improper read if get_registers() fail Denis Kirjanov
2019-07-30 17:24 ` David Miller
2019-07-30 18:23   ` Denis Kirjanov
     [not found]   ` <CAHj3AVm2EZB7n9UBxiBmA+6XN+EgAC_FRoHjh6kO3WMT8KVd6g@mail.gmail.com>
2019-07-30 21:10     ` David Miller
2019-07-31 19:10 ` Petko Manolov
2019-07-31 19:14   ` Petko Manolov
2019-08-01 22:18 ` David Miller
  -- strict thread matches above, loose matches on Subject: below --
2019-07-30 12:45 Denis Kirjanov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.