* [PATCH 1/1] ax88172a: fix ax88172a_unbind() failures
@ 2020-07-13 11:58 George Kennedy
2020-07-14 0:08 ` David Miller
0 siblings, 1 reply; 9+ messages in thread
From: George Kennedy @ 2020-07-13 11:58 UTC (permalink / raw)
To: george.kennedy, davem, kuba, dan.carpenter, dhaval.giani, netdev
If ax88172a_unbind() fails, make sure that the return code is
less than zero so that cleanup is done properly and avoid UAF.
Signed-off-by: George Kennedy <george.kennedy@oracle.com>
Reported-by: syzbot+4cd84f527bf4a10fc9c1@syzkaller.appspotmail.com
---
drivers/net/usb/ax88172a.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/usb/ax88172a.c b/drivers/net/usb/ax88172a.c
index 4e514f5..fd9faf2 100644
--- a/drivers/net/usb/ax88172a.c
+++ b/drivers/net/usb/ax88172a.c
@@ -237,6 +237,8 @@ static int ax88172a_bind(struct usbnet *dev, struct usb_interface *intf)
free:
kfree(priv);
+ if (ret >= 0)
+ ret = -EIO;
return ret;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] ax88172a: fix ax88172a_unbind() failures
2020-07-13 11:58 [PATCH 1/1] ax88172a: fix ax88172a_unbind() failures George Kennedy
@ 2020-07-14 0:08 ` David Miller
2020-07-14 8:00 ` Dan Carpenter
0 siblings, 1 reply; 9+ messages in thread
From: David Miller @ 2020-07-14 0:08 UTC (permalink / raw)
To: george.kennedy; +Cc: kuba, dan.carpenter, dhaval.giani, netdev
From: George Kennedy <george.kennedy@oracle.com>
Date: Mon, 13 Jul 2020 07:58:57 -0400
> @@ -237,6 +237,8 @@ static int ax88172a_bind(struct usbnet *dev, struct usb_interface *intf)
>
> free:
> kfree(priv);
> + if (ret >= 0)
> + ret = -EIO;
> return ret;
Success paths reach here, so ">= 0" is not appropriate. Maybe you
meant "> 0"?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] ax88172a: fix ax88172a_unbind() failures
2020-07-14 0:08 ` David Miller
@ 2020-07-14 8:00 ` Dan Carpenter
2020-07-14 21:03 ` David Miller
0 siblings, 1 reply; 9+ messages in thread
From: Dan Carpenter @ 2020-07-14 8:00 UTC (permalink / raw)
To: David Miller; +Cc: george.kennedy, kuba, dhaval.giani, netdev
On Mon, Jul 13, 2020 at 05:08:59PM -0700, David Miller wrote:
> From: George Kennedy <george.kennedy@oracle.com>
> Date: Mon, 13 Jul 2020 07:58:57 -0400
>
> > @@ -237,6 +237,8 @@ static int ax88172a_bind(struct usbnet *dev, struct usb_interface *intf)
> >
> > free:
> > kfree(priv);
> > + if (ret >= 0)
> > + ret = -EIO;
> > return ret;
>
> Success paths reach here, so ">= 0" is not appropriate. Maybe you
> meant "> 0"?
No, the success path is the "return 0;" one line before the start of the
diff. This is always a failure path.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] ax88172a: fix ax88172a_unbind() failures
2020-07-14 8:00 ` Dan Carpenter
@ 2020-07-14 21:03 ` David Miller
2020-07-14 21:34 ` George Kennedy
0 siblings, 1 reply; 9+ messages in thread
From: David Miller @ 2020-07-14 21:03 UTC (permalink / raw)
To: dan.carpenter; +Cc: george.kennedy, kuba, dhaval.giani, netdev
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Tue, 14 Jul 2020 11:00:38 +0300
> On Mon, Jul 13, 2020 at 05:08:59PM -0700, David Miller wrote:
>> From: George Kennedy <george.kennedy@oracle.com>
>> Date: Mon, 13 Jul 2020 07:58:57 -0400
>>
>> > @@ -237,6 +237,8 @@ static int ax88172a_bind(struct usbnet *dev, struct usb_interface *intf)
>> >
>> > free:
>> > kfree(priv);
>> > + if (ret >= 0)
>> > + ret = -EIO;
>> > return ret;
>>
>> Success paths reach here, so ">= 0" is not appropriate. Maybe you
>> meant "> 0"?
>
> No, the success path is the "return 0;" one line before the start of the
> diff. This is always a failure path.
Is zero ever a possibility, therefore?
You have two cases, one with an explicit -EIO and another which jumps
here "if (ret)"
So it seems the answer is no.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] ax88172a: fix ax88172a_unbind() failures
2020-07-14 21:03 ` David Miller
@ 2020-07-14 21:34 ` George Kennedy
2020-07-14 21:37 ` David Miller
0 siblings, 1 reply; 9+ messages in thread
From: George Kennedy @ 2020-07-14 21:34 UTC (permalink / raw)
To: David Miller, dan.carpenter; +Cc: kuba, dhaval.giani, netdev
On 7/14/2020 5:03 PM, David Miller wrote:
> From: Dan Carpenter <dan.carpenter@oracle.com>
> Date: Tue, 14 Jul 2020 11:00:38 +0300
>
>> On Mon, Jul 13, 2020 at 05:08:59PM -0700, David Miller wrote:
>>> From: George Kennedy <george.kennedy@oracle.com>
>>> Date: Mon, 13 Jul 2020 07:58:57 -0400
>>>
>>>> @@ -237,6 +237,8 @@ static int ax88172a_bind(struct usbnet *dev, struct usb_interface *intf)
>>>>
>>>> free:
>>>> kfree(priv);
>>>> + if (ret >= 0)
>>>> + ret = -EIO;
>>>> return ret;
>>> Success paths reach here, so ">= 0" is not appropriate. Maybe you
>>> meant "> 0"?
>> No, the success path is the "return 0;" one line before the start of the
>> diff. This is always a failure path.
> Is zero ever a possibility, therefore?
>
> You have two cases, one with an explicit -EIO and another which jumps
> here "if (ret)"
>
> So it seems the answer is no.
The "free:" label is the failure path. The "free:" label can be gotten
to with "ret" >= 0, but the failure path must exit with ret < 0 for
proper failure cleanup.
For example, the failing case here has "ret" = 0 (#define ETH_ALEN 6):
172 static int ax88172a_bind(struct usbnet *dev, struct
usb_interface *intf)
173 {
...
186 /* Get the MAC address */
187 ret = asix_read_cmd(dev, AX_CMD_READ_NODE_ID, 0, 0,
ETH_ALEN, buf, 0);
188 if (ret < ETH_ALEN) {
189 netdev_err(dev->net, "Failed to read MAC
address: %d\n", ret);
190 goto free;
191 }
"drivers/net/usb/ax88172a.c"
The caller to ax88172a_bind() is usbnet_probe() and in the case of
failure, it needs the return value to be < 0.
1653 int
1654 usbnet_probe (struct usb_interface *udev, const struct
usb_device_id *prod)
1655 {
...
1736 if (info->bind) {
1737 status = info->bind (dev, udev);
1738 if (status < 0)
1739 goto out1;
"drivers/net/usb/usbnet.c"
George
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] ax88172a: fix ax88172a_unbind() failures
2020-07-14 21:34 ` George Kennedy
@ 2020-07-14 21:37 ` David Miller
2020-07-15 14:01 ` George Kennedy
0 siblings, 1 reply; 9+ messages in thread
From: David Miller @ 2020-07-14 21:37 UTC (permalink / raw)
To: george.kennedy; +Cc: dan.carpenter, kuba, dhaval.giani, netdev
From: George Kennedy <george.kennedy@oracle.com>
Date: Tue, 14 Jul 2020 17:34:33 -0400
> For example, the failing case here has "ret" = 0 (#define ETH_ALEN 6):
>
> 172 static int ax88172a_bind(struct usbnet *dev, struct
> usb_interface *intf)
> 173 {
> ...
> 186 /* Get the MAC address */
> 187 ret = asix_read_cmd(dev, AX_CMD_READ_NODE_ID, 0, 0,
> ETH_ALEN, buf, 0);
> 188 if (ret < ETH_ALEN) {
> 189 netdev_err(dev->net, "Failed to read MAC
> address: %d\n", ret);
> 190 goto free;
> 191 }
> "drivers/net/usb/ax88172a.c"
Then this is the spot that should set 'ret' to -EINVAL or similar?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] ax88172a: fix ax88172a_unbind() failures
2020-07-14 21:37 ` David Miller
@ 2020-07-15 14:01 ` George Kennedy
0 siblings, 0 replies; 9+ messages in thread
From: George Kennedy @ 2020-07-15 14:01 UTC (permalink / raw)
To: David Miller; +Cc: dan.carpenter, kuba, dhaval.giani, netdev
On 7/14/2020 5:37 PM, David Miller wrote:
> From: George Kennedy <george.kennedy@oracle.com>
> Date: Tue, 14 Jul 2020 17:34:33 -0400
>
>> For example, the failing case here has "ret" = 0 (#define ETH_ALEN 6):
>>
>> 172 static int ax88172a_bind(struct usbnet *dev, struct
>> usb_interface *intf)
>> 173 {
>> ...
>> 186 /* Get the MAC address */
>> 187 ret = asix_read_cmd(dev, AX_CMD_READ_NODE_ID, 0, 0,
>> ETH_ALEN, buf, 0);
>> 188 if (ret < ETH_ALEN) {
>> 189 netdev_err(dev->net, "Failed to read MAC
>> address: %d\n", ret);
>> 190 goto free;
>> 191 }
>> "drivers/net/usb/ax88172a.c"
> Then this is the spot that should set 'ret' to -EINVAL or similar?
Made the suggested fix and sent the updated patch with following:
Subject: [PATCH v2 1/1] ax88172a: fix ax88172a_unbind() failures
George
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] ax88172a: fix ax88172a_unbind() failures
2020-07-10 19:38 George Kennedy
@ 2020-07-10 21:42 ` David Miller
0 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2020-07-10 21:42 UTC (permalink / raw)
To: george.kennedy; +Cc: kuba, dan.carpenter, dhaval.giani, linux-usb
From: George Kennedy <george.kennedy@oracle.com>
Date: Fri, 10 Jul 2020 15:38:48 -0400
> If ax88172a_unbind() fails, make sure that the return code is
> less than zero so that cleanup is done properly and avoid UAF.
>
> Signed-off-by: George Kennedy <george.kennedy@oracle.com>
> Reported-by: syzbot+4cd84f527bf4a10fc9c1@syzkaller.appspotmail.com
Networking patches should be sent to netdev@vger.kernel.org
Do not attempt to fix this by adding that list to the CC: of this
discussion. Make a fresh new patch posting instead.
Thank you.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/1] ax88172a: fix ax88172a_unbind() failures
@ 2020-07-10 19:38 George Kennedy
2020-07-10 21:42 ` David Miller
0 siblings, 1 reply; 9+ messages in thread
From: George Kennedy @ 2020-07-10 19:38 UTC (permalink / raw)
To: george.kennedy, davem, kuba, dan.carpenter, dhaval.giani, linux-usb
If ax88172a_unbind() fails, make sure that the return code is
less than zero so that cleanup is done properly and avoid UAF.
Signed-off-by: George Kennedy <george.kennedy@oracle.com>
Reported-by: syzbot+4cd84f527bf4a10fc9c1@syzkaller.appspotmail.com
---
drivers/net/usb/ax88172a.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/usb/ax88172a.c b/drivers/net/usb/ax88172a.c
index 4e514f5..fd9faf2 100644
--- a/drivers/net/usb/ax88172a.c
+++ b/drivers/net/usb/ax88172a.c
@@ -237,6 +237,8 @@ static int ax88172a_bind(struct usbnet *dev, struct usb_interface *intf)
free:
kfree(priv);
+ if (ret >= 0)
+ ret = -EIO;
return ret;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-07-15 14:01 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-13 11:58 [PATCH 1/1] ax88172a: fix ax88172a_unbind() failures George Kennedy
2020-07-14 0:08 ` David Miller
2020-07-14 8:00 ` Dan Carpenter
2020-07-14 21:03 ` David Miller
2020-07-14 21:34 ` George Kennedy
2020-07-14 21:37 ` David Miller
2020-07-15 14:01 ` George Kennedy
-- strict thread matches above, loose matches on Subject: below --
2020-07-10 19:38 George Kennedy
2020-07-10 21:42 ` David Miller
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.