drivers/net/ieee802154/atusb.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Alexander reported a use of uninitialized value in
atusb_set_extended_addr(), that is caused by reading 0 bytes via
usb_control_msg().
Fix it by validating if the number of bytes transferred is actually
correct, since usb_control_msg() may read less bytes, than was requested
by caller.
Fail log:
BUG: KASAN: uninit-cmp in ieee802154_is_valid_extended_unicast_addr include/linux/ieee802154.h:310 [inline]
BUG: KASAN: uninit-cmp in atusb_set_extended_addr drivers/net/ieee802154/atusb.c:1000 [inline]
BUG: KASAN: uninit-cmp in atusb_probe.cold+0x29f/0x14db drivers/net/ieee802154/atusb.c:1056
Uninit value used in comparison: 311daa649a2003bd stack handle: 000000009a2003bd
ieee802154_is_valid_extended_unicast_addr include/linux/ieee802154.h:310 [inline]
atusb_set_extended_addr drivers/net/ieee802154/atusb.c:1000 [inline]
atusb_probe.cold+0x29f/0x14db drivers/net/ieee802154/atusb.c:1056
usb_probe_interface+0x314/0x7f0 drivers/usb/core/driver.c:396
Fixes: 7490b008d123 ("ieee802154: add support for atusb transceiver")
Reported-by: Alexander Potapenko <glider@google.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
Changes in v2:
- Reworked fix approach, since moving to new USB API is not
suitable for backporting to stable kernels
---
drivers/net/ieee802154/atusb.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
index 23ee0b14cbfa..e6cc816dd7a1 100644
--- a/drivers/net/ieee802154/atusb.c
+++ b/drivers/net/ieee802154/atusb.c
@@ -93,7 +93,9 @@ static int atusb_control_msg(struct atusb *atusb, unsigned int pipe,
ret = usb_control_msg(usb_dev, pipe, request, requesttype,
value, index, data, size, timeout);
- if (ret < 0) {
+ if (ret < size) {
+ ret = ret < 0 ? ret : -ENODATA;
+
atusb->err = ret;
dev_err(&usb_dev->dev,
"%s: req 0x%02x val 0x%x idx 0x%x, error %d\n",
--
2.34.1
Hi,
On Mon, 3 Jan 2022 at 07:09, Pavel Skripkin <paskripkin@gmail.com> wrote:
>
> Alexander reported a use of uninitialized value in
> atusb_set_extended_addr(), that is caused by reading 0 bytes via
> usb_control_msg().
>
> Fix it by validating if the number of bytes transferred is actually
> correct, since usb_control_msg() may read less bytes, than was requested
> by caller.
>
> Fail log:
>
> BUG: KASAN: uninit-cmp in ieee802154_is_valid_extended_unicast_addr include/linux/ieee802154.h:310 [inline]
> BUG: KASAN: uninit-cmp in atusb_set_extended_addr drivers/net/ieee802154/atusb.c:1000 [inline]
> BUG: KASAN: uninit-cmp in atusb_probe.cold+0x29f/0x14db drivers/net/ieee802154/atusb.c:1056
> Uninit value used in comparison: 311daa649a2003bd stack handle: 000000009a2003bd
> ieee802154_is_valid_extended_unicast_addr include/linux/ieee802154.h:310 [inline]
> atusb_set_extended_addr drivers/net/ieee802154/atusb.c:1000 [inline]
> atusb_probe.cold+0x29f/0x14db drivers/net/ieee802154/atusb.c:1056
> usb_probe_interface+0x314/0x7f0 drivers/usb/core/driver.c:396
>
> Fixes: 7490b008d123 ("ieee802154: add support for atusb transceiver")
> Reported-by: Alexander Potapenko <glider@google.com>
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Acked-by: Alexander Aring <aahringo@redhat.com>
Although I think if such error exists the most common solution would
be to replug the usb device and hope it gets resolved? A retry with a
maximum amount would be another try... However, let's hope those
errors are rare and we care about them when they occur.
Thanks.
- Alex
Hello.
On 03.01.22 13:09, Pavel Skripkin wrote:
> Alexander reported a use of uninitialized value in
> atusb_set_extended_addr(), that is caused by reading 0 bytes via
> usb_control_msg().
>
> Fix it by validating if the number of bytes transferred is actually
> correct, since usb_control_msg() may read less bytes, than was requested
> by caller.
>
> Fail log:
>
> BUG: KASAN: uninit-cmp in ieee802154_is_valid_extended_unicast_addr include/linux/ieee802154.h:310 [inline]
> BUG: KASAN: uninit-cmp in atusb_set_extended_addr drivers/net/ieee802154/atusb.c:1000 [inline]
> BUG: KASAN: uninit-cmp in atusb_probe.cold+0x29f/0x14db drivers/net/ieee802154/atusb.c:1056
> Uninit value used in comparison: 311daa649a2003bd stack handle: 000000009a2003bd
> ieee802154_is_valid_extended_unicast_addr include/linux/ieee802154.h:310 [inline]
> atusb_set_extended_addr drivers/net/ieee802154/atusb.c:1000 [inline]
> atusb_probe.cold+0x29f/0x14db drivers/net/ieee802154/atusb.c:1056
> usb_probe_interface+0x314/0x7f0 drivers/usb/core/driver.c:396
>
> Fixes: 7490b008d123 ("ieee802154: add support for atusb transceiver")
> Reported-by: Alexander Potapenko <glider@google.com>
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> ---
>
> Changes in v2:
> - Reworked fix approach, since moving to new USB API is not
> suitable for backporting to stable kernels
>
> ---
> drivers/net/ieee802154/atusb.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
> index 23ee0b14cbfa..e6cc816dd7a1 100644
> --- a/drivers/net/ieee802154/atusb.c
> +++ b/drivers/net/ieee802154/atusb.c
> @@ -93,7 +93,9 @@ static int atusb_control_msg(struct atusb *atusb, unsigned int pipe,
>
> ret = usb_control_msg(usb_dev, pipe, request, requesttype,
> value, index, data, size, timeout);
> - if (ret < 0) {
> + if (ret < size) {
> + ret = ret < 0 ? ret : -ENODATA;
> +
> atusb->err = ret;
> dev_err(&usb_dev->dev,
> "%s: req 0x%02x val 0x%x idx 0x%x, error %d\n",
>
It compiles, but does not work on the real hardware.
[ 1.114698] usb 1-1: new full-speed USB device number 2 using uhci_hcd
[ 1.261691] usb 1-1: New USB device found, idVendor=20b7,
idProduct=1540, bcdDevice= 0.01
[ 1.263421] usb 1-1: New USB device strings: Mfr=0, Product=0,
SerialNumber=1
[ 1.264952] usb 1-1: SerialNumber: 4630333438371502231a
[ 1.278042] usb 1-1: ATUSB: AT86RF231 version 2
[ 1.281087] usb 1-1: Firmware: major: 0, minor: 3, hardware type:
ATUSB (2)
[ 1.285191] usb 1-1: atusb_control_msg: req 0x01 val 0x0 idx 0x0,
error -61
[ 1.286903] usb 1-1: failed to fetch extended address, random address set
[ 1.288757] usb 1-1: atusb_probe: initialization failed, error = -61
[ 1.290922] atusb: probe of 1-1:1.0 failed with error -61
Without your patch it works as expected:
[ 1.091925] usb 1-1: new full-speed USB device number 2 using uhci_hcd
[ 1.237743] usb 1-1: New USB device found, idVendor=20b7,
idProduct=1540, bcdDevice= 0.01
[ 1.239788] usb 1-1: New USB device strings: Mfr=0, Product=0,
SerialNumber=1
[ 1.241432] usb 1-1: SerialNumber: 4630333438371502231a
[ 1.255012] usb 1-1: ATUSB: AT86RF231 version 2
[ 1.258073] usb 1-1: Firmware: major: 0, minor: 3, hardware type:
ATUSB (2)
[ 1.262170] usb 1-1: Firmware: build #132 Mo 28. Nov 16:20:35 CET 2016
[ 1.266195] usb 1-1: Read permanent extended address
10:e2:d5:ff:ff:00:02:e8 from device
regards
Stefan Schmidt
On 1/4/22 18:40, Stefan Schmidt wrote: > > It compiles, but does not work on the real hardware. > > [ 1.114698] usb 1-1: new full-speed USB device number 2 using uhci_hcd > [ 1.261691] usb 1-1: New USB device found, idVendor=20b7, > idProduct=1540, bcdDevice= 0.01 > [ 1.263421] usb 1-1: New USB device strings: Mfr=0, Product=0, > SerialNumber=1 > [ 1.264952] usb 1-1: SerialNumber: 4630333438371502231a > [ 1.278042] usb 1-1: ATUSB: AT86RF231 version 2 > [ 1.281087] usb 1-1: Firmware: major: 0, minor: 3, hardware type: > ATUSB (2) > [ 1.285191] usb 1-1: atusb_control_msg: req 0x01 val 0x0 idx 0x0, > error -61 > [ 1.286903] usb 1-1: failed to fetch extended address, random address set > [ 1.288757] usb 1-1: atusb_probe: initialization failed, error = -61 > [ 1.290922] atusb: probe of 1-1:1.0 failed with error -61 > > > Without your patch it works as expected: > > [ 1.091925] usb 1-1: new full-speed USB device number 2 using uhci_hcd > [ 1.237743] usb 1-1: New USB device found, idVendor=20b7, > idProduct=1540, bcdDevice= 0.01 > [ 1.239788] usb 1-1: New USB device strings: Mfr=0, Product=0, > SerialNumber=1 > [ 1.241432] usb 1-1: SerialNumber: 4630333438371502231a > [ 1.255012] usb 1-1: ATUSB: AT86RF231 version 2 > [ 1.258073] usb 1-1: Firmware: major: 0, minor: 3, hardware type: > ATUSB (2) > [ 1.262170] usb 1-1: Firmware: build #132 Mo 28. Nov 16:20:35 CET 2016 > [ 1.266195] usb 1-1: Read permanent extended address > 10:e2:d5:ff:ff:00:02:e8 from device > Hi Stefan, thanks for testing on real hw. It looks like there is corner case, that Greg mentioned in this thread. atusb_get_and_show_build() reads firmware build info, which may have various length. Maybe we can change atusb_control_msg() to usb_control_msg() in atusb_get_and_show_build(), since other callers do not have this problem With regards, Pavel Skripkin
Hello. On 04.01.22 18:27, Pavel Skripkin wrote: > On 1/4/22 18:40, Stefan Schmidt wrote: >> >> It compiles, but does not work on the real hardware. >> >> [ 1.114698] usb 1-1: new full-speed USB device number 2 using uhci_hcd >> [ 1.261691] usb 1-1: New USB device found, idVendor=20b7, >> idProduct=1540, bcdDevice= 0.01 >> [ 1.263421] usb 1-1: New USB device strings: Mfr=0, Product=0, >> SerialNumber=1 >> [ 1.264952] usb 1-1: SerialNumber: 4630333438371502231a >> [ 1.278042] usb 1-1: ATUSB: AT86RF231 version 2 >> [ 1.281087] usb 1-1: Firmware: major: 0, minor: 3, hardware type: >> ATUSB (2) >> [ 1.285191] usb 1-1: atusb_control_msg: req 0x01 val 0x0 idx 0x0, >> error -61 >> [ 1.286903] usb 1-1: failed to fetch extended address, random >> address set >> [ 1.288757] usb 1-1: atusb_probe: initialization failed, error = -61 >> [ 1.290922] atusb: probe of 1-1:1.0 failed with error -61 >> >> >> Without your patch it works as expected: >> >> [ 1.091925] usb 1-1: new full-speed USB device number 2 using uhci_hcd >> [ 1.237743] usb 1-1: New USB device found, idVendor=20b7, >> idProduct=1540, bcdDevice= 0.01 >> [ 1.239788] usb 1-1: New USB device strings: Mfr=0, Product=0, >> SerialNumber=1 >> [ 1.241432] usb 1-1: SerialNumber: 4630333438371502231a >> [ 1.255012] usb 1-1: ATUSB: AT86RF231 version 2 >> [ 1.258073] usb 1-1: Firmware: major: 0, minor: 3, hardware type: >> ATUSB (2) >> [ 1.262170] usb 1-1: Firmware: build #132 Mo 28. Nov 16:20:35 CET 2016 >> [ 1.266195] usb 1-1: Read permanent extended address >> 10:e2:d5:ff:ff:00:02:e8 from device >> > > Hi Stefan, > > thanks for testing on real hw. > > It looks like there is corner case, that Greg mentioned in this thread. > atusb_get_and_show_build() reads firmware build info, which may have > various length. > > Maybe we can change atusb_control_msg() to usb_control_msg() in > atusb_get_and_show_build(), since other callers do not have this problem That works for me. I will also have a look at the use of the modern USB API for next. The fix here has a higher prio for me to get in and backported though. Once we have this we can look at bigger changes in atusb. regards Stefan Schmidt
On 1/4/22 21:04, Stefan Schmidt wrote: >> Hi Stefan, >> >> thanks for testing on real hw. >> >> It looks like there is corner case, that Greg mentioned in this thread. >> atusb_get_and_show_build() reads firmware build info, which may have >> various length. >> >> Maybe we can change atusb_control_msg() to usb_control_msg() in >> atusb_get_and_show_build(), since other callers do not have this problem > > That works for me. > Nice! Will prepare v3. Thanks for testing once again! > I will also have a look at the use of the modern USB API for next. The > fix here has a higher prio for me to get in and backported though. Once > we have this we can look at bigger changes in atusb. > With regards, Pavel Skripkin
Alexander reported a use of uninitialized value in
atusb_set_extended_addr(), that is caused by reading 0 bytes via
usb_control_msg().
Fix it by validating if the number of bytes transferred is actually
correct, since usb_control_msg() may read less bytes, than was requested
by caller.
Fail log:
BUG: KASAN: uninit-cmp in ieee802154_is_valid_extended_unicast_addr include/linux/ieee802154.h:310 [inline]
BUG: KASAN: uninit-cmp in atusb_set_extended_addr drivers/net/ieee802154/atusb.c:1000 [inline]
BUG: KASAN: uninit-cmp in atusb_probe.cold+0x29f/0x14db drivers/net/ieee802154/atusb.c:1056
Uninit value used in comparison: 311daa649a2003bd stack handle: 000000009a2003bd
ieee802154_is_valid_extended_unicast_addr include/linux/ieee802154.h:310 [inline]
atusb_set_extended_addr drivers/net/ieee802154/atusb.c:1000 [inline]
atusb_probe.cold+0x29f/0x14db drivers/net/ieee802154/atusb.c:1056
usb_probe_interface+0x314/0x7f0 drivers/usb/core/driver.c:396
Fixes: 7490b008d123 ("ieee802154: add support for atusb transceiver")
Reported-by: Alexander Potapenko <glider@google.com>
Acked-by: Alexander Aring <aahringo@redhat.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
Changes in v3:
- Changed atusb_control_msg() to usb_control_msg() in
atusb_get_and_show_build(), since request there may read various length
data
Changes in v2:
- Reworked fix approach, since moving to new USB API is not
suitable for backporting to stable kernels
---
drivers/net/ieee802154/atusb.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
index 23ee0b14cbfa..2f5e7b31032a 100644
--- a/drivers/net/ieee802154/atusb.c
+++ b/drivers/net/ieee802154/atusb.c
@@ -93,7 +93,9 @@ static int atusb_control_msg(struct atusb *atusb, unsigned int pipe,
ret = usb_control_msg(usb_dev, pipe, request, requesttype,
value, index, data, size, timeout);
- if (ret < 0) {
+ if (ret < size) {
+ ret = ret < 0 ? ret : -ENODATA;
+
atusb->err = ret;
dev_err(&usb_dev->dev,
"%s: req 0x%02x val 0x%x idx 0x%x, error %d\n",
@@ -861,9 +863,9 @@ static int atusb_get_and_show_build(struct atusb *atusb)
if (!build)
return -ENOMEM;
- ret = atusb_control_msg(atusb, usb_rcvctrlpipe(usb_dev, 0),
- ATUSB_BUILD, ATUSB_REQ_FROM_DEV, 0, 0,
- build, ATUSB_BUILD_SIZE, 1000);
+ /* We cannot call atusb_control_msg() here, since this request may read various length data */
+ ret = usb_control_msg(atusb->usb_dev, usb_rcvctrlpipe(usb_dev, 0), ATUSB_BUILD,
+ ATUSB_REQ_FROM_DEV, 0, 0, build, ATUSB_BUILD_SIZE, 1000);
if (ret >= 0) {
build[ret] = 0;
dev_info(&usb_dev->dev, "Firmware: build %s\n", build);
--
2.34.1
Hi,
On Tue, Jan 4, 2022 at 1:28 PM Pavel Skripkin <paskripkin@gmail.com> wrote:
>
> Alexander reported a use of uninitialized value in
> atusb_set_extended_addr(), that is caused by reading 0 bytes via
> usb_control_msg().
>
> Fix it by validating if the number of bytes transferred is actually
> correct, since usb_control_msg() may read less bytes, than was requested
> by caller.
>
> Fail log:
>
> BUG: KASAN: uninit-cmp in ieee802154_is_valid_extended_unicast_addr include/linux/ieee802154.h:310 [inline]
> BUG: KASAN: uninit-cmp in atusb_set_extended_addr drivers/net/ieee802154/atusb.c:1000 [inline]
> BUG: KASAN: uninit-cmp in atusb_probe.cold+0x29f/0x14db drivers/net/ieee802154/atusb.c:1056
> Uninit value used in comparison: 311daa649a2003bd stack handle: 000000009a2003bd
> ieee802154_is_valid_extended_unicast_addr include/linux/ieee802154.h:310 [inline]
> atusb_set_extended_addr drivers/net/ieee802154/atusb.c:1000 [inline]
> atusb_probe.cold+0x29f/0x14db drivers/net/ieee802154/atusb.c:1056
> usb_probe_interface+0x314/0x7f0 drivers/usb/core/driver.c:396
>
> Fixes: 7490b008d123 ("ieee802154: add support for atusb transceiver")
> Reported-by: Alexander Potapenko <glider@google.com>
> Acked-by: Alexander Aring <aahringo@redhat.com>
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> ---
>
> Changes in v3:
> - Changed atusb_control_msg() to usb_control_msg() in
> atusb_get_and_show_build(), since request there may read various length
> data
>
Thanks for catching this.
- Alex
Hello.
On 04.01.22 19:57, Alexander Aring wrote:
> Hi,
>
> On Tue, Jan 4, 2022 at 1:28 PM Pavel Skripkin <paskripkin@gmail.com> wrote:
>>
>> Alexander reported a use of uninitialized value in
>> atusb_set_extended_addr(), that is caused by reading 0 bytes via
>> usb_control_msg().
>>
>> Fix it by validating if the number of bytes transferred is actually
>> correct, since usb_control_msg() may read less bytes, than was requested
>> by caller.
>>
>> Fail log:
>>
>> BUG: KASAN: uninit-cmp in ieee802154_is_valid_extended_unicast_addr include/linux/ieee802154.h:310 [inline]
>> BUG: KASAN: uninit-cmp in atusb_set_extended_addr drivers/net/ieee802154/atusb.c:1000 [inline]
>> BUG: KASAN: uninit-cmp in atusb_probe.cold+0x29f/0x14db drivers/net/ieee802154/atusb.c:1056
>> Uninit value used in comparison: 311daa649a2003bd stack handle: 000000009a2003bd
>> ieee802154_is_valid_extended_unicast_addr include/linux/ieee802154.h:310 [inline]
>> atusb_set_extended_addr drivers/net/ieee802154/atusb.c:1000 [inline]
>> atusb_probe.cold+0x29f/0x14db drivers/net/ieee802154/atusb.c:1056
>> usb_probe_interface+0x314/0x7f0 drivers/usb/core/driver.c:396
>>
>> Fixes: 7490b008d123 ("ieee802154: add support for atusb transceiver")
>> Reported-by: Alexander Potapenko <glider@google.com>
>> Acked-by: Alexander Aring <aahringo@redhat.com>
>> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
>> ---
>>
>> Changes in v3:
>> - Changed atusb_control_msg() to usb_control_msg() in
>> atusb_get_and_show_build(), since request there may read various length
>> data
>>
>
> Thanks for catching this.
Test passed my testing.
This patch has been applied to the wpan tree and will be
part of the next pull request to net. Thanks!
regards
Stefan Schmidt
© 2016 - 2026 Red Hat, Inc.