Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.debian.kernel > #67480 > unrolled thread

Bug#964480: linux: ath9k (USB) broken in upstresm linux 5.7.3 after commit 6602f080cb

Back to article view | Back to linux.debian.kernel

Contents

  Bug#964480: linux: ath9k (USB) broken in upstresm linux 5.7.3 after commit 6602f080cb Juergen <j-r@online.de> - 2020-07-07 21:30 +0200
    Bug#964480: linux: ath9k (USB) broken in upstresm linux 5.7.3 after commit 6602f080cb Bjørn Mork <bjorn@mork.no> - 2020-07-08 12:00 +0200
    Bug#964480: linux: ath9k (USB) broken in upstresm linux 5.7.3 after commit 6602f080cb Leandro Cunha <leandrocunha016@gmail.com> - 2020-07-08 15:40 +0200
    Bug#964480: marked as done (linux: ath9k (USB) broken in upstresm  linux 5.7.3 after commit 6602f080cb) "Debian Bug Tracking System" <owner@bugs.debian.org> - 2020-07-24 21:40 +0200
    Bug#964480: marked as done (linux: ath9k (USB) broken in upstresm  linux 5.7.3 after commit 6602f080cb) "Debian Bug Tracking System" <owner@bugs.debian.org> - 2020-07-24 21:40 +0200

#67480 — Bug#964480: linux: ath9k (USB) broken in upstresm linux 5.7.3 after commit 6602f080cb

Source: linux
Version: 5.7.0-1
Severity: normal
Tags: upstream patch

Dear Maintainer,

see https://bugzilla.kernel.org/show_bug.cgi?id=208251.

This affects all kernel versions that include

	ath9k: Fix general protection fault in ath9k_hif_usb_rx_cb

by

	Qiujun Huang <hqjagain@gmail.com>

In 5.7 reverting 6602f080cb is known to fix the problem, but it hasn't yet been
analyzed by an expert, therefore I'd like to report my findings to perhaps get
an expert interested:-)

At least one problem is in function ath9k_hif_usb_reg_in_cb(). In the call to
usb_fill_int_urb() it should be rx_buf instead of nskb as penultimate
parameter.

I'm currently running the following patch to send this bug report using my ath9k
dongle.

Best regards

Jürgen

diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
index 4ed21dad6a8e..f3622d686002 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.c
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
@@ -693,15 +693,8 @@ static void ath9k_hif_usb_reg_in_cb(struct urb *urb)
        struct rx_buf *rx_buf = (struct rx_buf *)urb->context;
        struct hif_device_usb *hif_dev = rx_buf->hif_dev;
        struct sk_buff *skb = rx_buf->skb;
-       struct sk_buff *nskb;
        int ret;

-       if (!skb)
-               return;
-
-       if (!hif_dev)
-               goto free;
-
        switch (urb->status) {
        case 0:
                break;
@@ -711,9 +704,6 @@ static void ath9k_hif_usb_reg_in_cb(struct urb *urb)
        case -ESHUTDOWN:
                goto free;
        default:
-               skb_reset_tail_pointer(skb);
-               skb_trim(skb, 0);
-
                goto resubmit;
        }

@@ -725,22 +715,27 @@ static void ath9k_hif_usb_reg_in_cb(struct urb *urb)
                                 skb->len, USB_REG_IN_PIPE);


-               nskb = alloc_skb(MAX_REG_IN_BUF_SIZE, GFP_ATOMIC);
-               if (!nskb) {
+               skb = alloc_skb(MAX_REG_IN_BUF_SIZE, GFP_ATOMIC);
+               if (!skb) {
                        dev_err(&hif_dev->udev->dev,
                                "ath9k_htc: REG_IN memory allocation failure\n");
                        urb->context = NULL;
-                       return;
+                       goto free;
                }

+               rx_buf->skb = skb;
+
                usb_fill_int_urb(urb, hif_dev->udev,
                                 usb_rcvintpipe(hif_dev->udev,
                                                 USB_REG_IN_PIPE),
-                                nskb->data, MAX_REG_IN_BUF_SIZE,
-                                ath9k_hif_usb_reg_in_cb, nskb, 1);
+                                skb->data, MAX_REG_IN_BUF_SIZE,
+                                ath9k_hif_usb_reg_in_cb, rx_buf, 1);
        }

 resubmit:
+       skb_reset_tail_pointer(skb);
+       skb_trim(skb, 0);
+
        usb_anchor_urb(urb, &hif_dev->reg_in_submitted);
        ret = usb_submit_urb(urb, GFP_ATOMIC);
        if (ret) {

-- System Information:
Debian Release: bullseye/sid
  APT prefers stable-updates
  APT policy: (500, 'stable-updates'), (500, 'testing'), (500, 'stable')
Architecture: i386 (i686)

Kernel: Linux 5.7.0-1-686-pae (SMP w/8 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: sysvinit (via /sbin/init)

[toc] | [next] | [standalone]

#67486

Juergen <j-r@online.de> writes:

> In 5.7 reverting 6602f080cb is known to fix the problem, but it hasn't yet been
> analyzed by an expert, therefore I'd like to report my findings to perhaps get
> an expert interested:-)
>
> At least one problem is in function ath9k_hif_usb_reg_in_cb(). In the call to
> usb_fill_int_urb() it should be rx_buf instead of nskb as penultimate
> parameter.

Yes, that's an obvious killer bug.  That patch should definitely be
reverted, and replaced with a proper and tested fix.

IMHO, it would be much safer and simpler to validate the expected
descriptors when probing these devices instead of allocating new helper
structs and making changes all over the place.  There is no reason to
add support for the syzbot simulated device.  Returning -ENODEV on probe
is fine, and is much less likely to add new and critical bugs.

And it's not like the fix actually took care of all the hard coded well
known descriptor values in this driver anyway.  We still have stuff like

 usb_sndintpipe(hif_dev->udev, USB_REG_OUT_PIPE)

and

 usb_sndbulkpipe(hif_dev->udev, USB_WLAN_TX_PIPE)

These will not crash the driver, but are still solid indications that
the driver is written for a very specific USB device configuration.  It
will not work with arbitrary descriptors.

I assume the "0" interface is just as fixed as those endpoints, so that
simply validating the values in ath9k_hif_usb_probe() is a perfectly
fine and safe solution.  Without the need to mess up the rest of the
driver.



Bjørn

[toc] | [prev] | [next] | [standalone]

#67488

[Multipart message — attachments visible in raw view] — view raw

Hi,

I had previously reported this problem.
I tried to merge with 964153 the reports but apparently without
success, believe
that this function is only for maintainers of the package.

Greetings,

Leandro Cunha

[toc] | [prev] | [next] | [standalone]

#67597 — Bug#964480: marked as done (linux: ath9k (USB) broken in upstresm linux 5.7.3 after commit 6602f080cb)

[Multipart message — attachments visible in raw view] — view raw

Your message dated Fri, 24 Jul 2020 19:32:08 +0000
with message-id <E1jz3QO-000GCG-0g@fasolo.debian.org>
and subject line Bug#964153: fixed in linux 4.19.132-1
has caused the Debian Bug report #964153,
regarding linux: ath9k (USB) broken in upstresm linux 5.7.3 after commit 6602f080cb
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
964153: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=964153
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems

[toc] | [prev] | [next] | [standalone]

#67600 — Bug#964480: marked as done (linux: ath9k (USB) broken in upstresm linux 5.7.3 after commit 6602f080cb)

[Multipart message — attachments visible in raw view] — view raw

Your message dated Fri, 24 Jul 2020 19:32:08 +0000
with message-id <E1jz3QO-000GCL-37@fasolo.debian.org>
and subject line Bug#964480: fixed in linux 4.19.132-1
has caused the Debian Bug report #964480,
regarding linux: ath9k (USB) broken in upstresm linux 5.7.3 after commit 6602f080cb
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
964480: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=964480
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems

[toc] | [prev] | [standalone]

Back to top | Article view | linux.debian.kernel

csiph-web