| Message ID | 20190804002905.11292-1-benquike@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 39d170b3cb62ba98567f5c4f40c27b5864b304e5 |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [1/2] Fix a NULL-ptr-deref bug in ath6kl_usb_alloc_urb_from_pipe | expand |
On Sat, Aug 03, 2019 at 08:29:04PM -0400, Hui Peng wrote: > The `ar_usb` field of `ath6kl_usb_pipe_usb_pipe` objects > are initialized to point to the containing `ath6kl_usb` object > according to endpoint descriptors read from the device side, as shown > below in `ath6kl_usb_setup_pipe_resources`: > > for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) { > endpoint = &iface_desc->endpoint[i].desc; > > // get the address from endpoint descriptor > pipe_num = ath6kl_usb_get_logical_pipe_num(ar_usb, > endpoint->bEndpointAddress, > &urbcount); > ...... > // select the pipe object > pipe = &ar_usb->pipes[pipe_num]; > > // initialize the ar_usb field > pipe->ar_usb = ar_usb; > } > > The driver assumes that the addresses reported in endpoint > descriptors from device side to be complete. If a device is > malicious and does not report complete addresses, it may trigger > NULL-ptr-deref `ath6kl_usb_alloc_urb_from_pipe` and > `ath6kl_usb_free_urb_to_pipe`. > > This patch fixes the bug by preventing potential NULL-ptr-deref. > > Signed-off-by: Hui Peng <benquike@gmail.com> > Reported-by: Hui Peng <benquike@gmail.com> > Reported-by: Mathias Payer <mathias.payer@nebelwelt.net> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
On Sat, Aug 03, 2019 at 08:29:04PM -0400, Hui Peng wrote: > The `ar_usb` field of `ath6kl_usb_pipe_usb_pipe` objects > are initialized to point to the containing `ath6kl_usb` object > according to endpoint descriptors read from the device side, as shown > below in `ath6kl_usb_setup_pipe_resources`: > > for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) { > endpoint = &iface_desc->endpoint[i].desc; > > // get the address from endpoint descriptor > pipe_num = ath6kl_usb_get_logical_pipe_num(ar_usb, > endpoint->bEndpointAddress, > &urbcount); > ...... > // select the pipe object > pipe = &ar_usb->pipes[pipe_num]; > > // initialize the ar_usb field > pipe->ar_usb = ar_usb; > } > > The driver assumes that the addresses reported in endpoint > descriptors from device side to be complete. If a device is > malicious and does not report complete addresses, it may trigger > NULL-ptr-deref `ath6kl_usb_alloc_urb_from_pipe` and > `ath6kl_usb_free_urb_to_pipe`. > > This patch fixes the bug by preventing potential NULL-ptr-deref. > > Signed-off-by: Hui Peng <benquike@gmail.com> > Reported-by: Hui Peng <benquike@gmail.com> > Reported-by: Mathias Payer <mathias.payer@nebelwelt.net> I don't see this patch in the upstream kernel or in -next. At the same time, it is supposed to fix CVE-2019-15098, which has a CVSS v2.0 score of 7.8 (high). Is this patch going to be applied to the upstream kernel ? If not, are there reasons to believe that the vulnerability is not as severe as its CVSS score indicates ? Thanks, Guenter > --- > drivers/net/wireless/ath/ath6kl/usb.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/net/wireless/ath/ath6kl/usb.c b/drivers/net/wireless/ath/ath6kl/usb.c > index 4defb7a0330f..53b66e9434c9 100644 > --- a/drivers/net/wireless/ath/ath6kl/usb.c > +++ b/drivers/net/wireless/ath/ath6kl/usb.c > @@ -132,6 +132,10 @@ ath6kl_usb_alloc_urb_from_pipe(struct ath6kl_usb_pipe *pipe) > struct ath6kl_urb_context *urb_context = NULL; > unsigned long flags; > > + /* bail if this pipe is not initialized */ > + if (!pipe->ar_usb) > + return NULL; > + > spin_lock_irqsave(&pipe->ar_usb->cs_lock, flags); > if (!list_empty(&pipe->urb_list_head)) { > urb_context = > @@ -150,6 +154,10 @@ static void ath6kl_usb_free_urb_to_pipe(struct ath6kl_usb_pipe *pipe, > { > unsigned long flags; > > + /* bail if this pipe is not initialized */ > + if (!pipe->ar_usb) > + return; > + > spin_lock_irqsave(&pipe->ar_usb->cs_lock, flags); > pipe->urb_cnt++; > > -- > 2.22.0 >
Guenter Roeck <linux@roeck-us.net> writes: > On Sat, Aug 03, 2019 at 08:29:04PM -0400, Hui Peng wrote: >> The `ar_usb` field of `ath6kl_usb_pipe_usb_pipe` objects >> are initialized to point to the containing `ath6kl_usb` object >> according to endpoint descriptors read from the device side, as shown >> below in `ath6kl_usb_setup_pipe_resources`: >> >> for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) { >> endpoint = &iface_desc->endpoint[i].desc; >> >> // get the address from endpoint descriptor >> pipe_num = ath6kl_usb_get_logical_pipe_num(ar_usb, >> endpoint->bEndpointAddress, >> &urbcount); >> ...... >> // select the pipe object >> pipe = &ar_usb->pipes[pipe_num]; >> >> // initialize the ar_usb field >> pipe->ar_usb = ar_usb; >> } >> >> The driver assumes that the addresses reported in endpoint >> descriptors from device side to be complete. If a device is >> malicious and does not report complete addresses, it may trigger >> NULL-ptr-deref `ath6kl_usb_alloc_urb_from_pipe` and >> `ath6kl_usb_free_urb_to_pipe`. >> >> This patch fixes the bug by preventing potential NULL-ptr-deref. >> >> Signed-off-by: Hui Peng <benquike@gmail.com> >> Reported-by: Hui Peng <benquike@gmail.com> >> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net> > > I don't see this patch in the upstream kernel or in -next. > > At the same time, it is supposed to fix CVE-2019-15098, which has > a CVSS v2.0 score of 7.8 (high). > > Is this patch going to be applied to the upstream kernel ? Lately I have been very busy and I have not had a chance to apply ath6kl nor ath10k patches. This patch is on my queue and my plan is to go through my patch queue next week.
Hui Peng <benquike@gmail.com> writes: > The reason that this patch is still in the pending state is that it > has not reviewed by maintainers (they are not responding). > @Greg: can we apply it? Who is "we" in this case? But anyway, I'll review the patch and if it's ok I'll take it through my ath.git tree, as normal.
Hui Peng <benquike@gmail.com> wrote: > The `ar_usb` field of `ath6kl_usb_pipe_usb_pipe` objects > are initialized to point to the containing `ath6kl_usb` object > according to endpoint descriptors read from the device side, as shown > below in `ath6kl_usb_setup_pipe_resources`: > > for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) { > endpoint = &iface_desc->endpoint[i].desc; > > // get the address from endpoint descriptor > pipe_num = ath6kl_usb_get_logical_pipe_num(ar_usb, > endpoint->bEndpointAddress, > &urbcount); > ...... > // select the pipe object > pipe = &ar_usb->pipes[pipe_num]; > > // initialize the ar_usb field > pipe->ar_usb = ar_usb; > } > > The driver assumes that the addresses reported in endpoint > descriptors from device side to be complete. If a device is > malicious and does not report complete addresses, it may trigger > NULL-ptr-deref `ath6kl_usb_alloc_urb_from_pipe` and > `ath6kl_usb_free_urb_to_pipe`. > > This patch fixes the bug by preventing potential NULL-ptr-deref > (CVE-2019-15098). > > Signed-off-by: Hui Peng <benquike@gmail.com> > Reported-by: Hui Peng <benquike@gmail.com> > Reported-by: Mathias Payer <mathias.payer@nebelwelt.net> > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Kalle Valo <kvalo@codeaurora.org> Patch applied to ath-next branch of ath.git, thanks. 39d170b3cb62 ath6kl: fix a NULL-ptr-deref bug in ath6kl_usb_alloc_urb_from_pipe()
diff --git a/drivers/net/wireless/ath/ath6kl/usb.c b/drivers/net/wireless/ath/ath6kl/usb.c index 4defb7a0330f..53b66e9434c9 100644 --- a/drivers/net/wireless/ath/ath6kl/usb.c +++ b/drivers/net/wireless/ath/ath6kl/usb.c @@ -132,6 +132,10 @@ ath6kl_usb_alloc_urb_from_pipe(struct ath6kl_usb_pipe *pipe) struct ath6kl_urb_context *urb_context = NULL; unsigned long flags; + /* bail if this pipe is not initialized */ + if (!pipe->ar_usb) + return NULL; + spin_lock_irqsave(&pipe->ar_usb->cs_lock, flags); if (!list_empty(&pipe->urb_list_head)) { urb_context = @@ -150,6 +154,10 @@ static void ath6kl_usb_free_urb_to_pipe(struct ath6kl_usb_pipe *pipe, { unsigned long flags; + /* bail if this pipe is not initialized */ + if (!pipe->ar_usb) + return; + spin_lock_irqsave(&pipe->ar_usb->cs_lock, flags); pipe->urb_cnt++;
The `ar_usb` field of `ath6kl_usb_pipe_usb_pipe` objects are initialized to point to the containing `ath6kl_usb` object according to endpoint descriptors read from the device side, as shown below in `ath6kl_usb_setup_pipe_resources`: for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) { endpoint = &iface_desc->endpoint[i].desc; // get the address from endpoint descriptor pipe_num = ath6kl_usb_get_logical_pipe_num(ar_usb, endpoint->bEndpointAddress, &urbcount); ...... // select the pipe object pipe = &ar_usb->pipes[pipe_num]; // initialize the ar_usb field pipe->ar_usb = ar_usb; } The driver assumes that the addresses reported in endpoint descriptors from device side to be complete. If a device is malicious and does not report complete addresses, it may trigger NULL-ptr-deref `ath6kl_usb_alloc_urb_from_pipe` and `ath6kl_usb_free_urb_to_pipe`. This patch fixes the bug by preventing potential NULL-ptr-deref. Signed-off-by: Hui Peng <benquike@gmail.com> Reported-by: Hui Peng <benquike@gmail.com> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net> --- drivers/net/wireless/ath/ath6kl/usb.c | 8 ++++++++ 1 file changed, 8 insertions(+)