| Message ID | 1493812123-12053-2-git-send-email-huxinming820@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
On 3-5-2017 13:48, Xinming Hu wrote: > From: Xinming Hu <huxm@marvell.com> > > urb/context might be freed in cornel case, add sanity check to avoid > use-after-free. > > Signed-off-by: Xinming Hu <huxm@marvell.com> > --- > drivers/net/wireless/marvell/mwifiex/usb.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c > index 2f7705c..ee5f488 100644 > --- a/drivers/net/wireless/marvell/mwifiex/usb.c > +++ b/drivers/net/wireless/marvell/mwifiex/usb.c > @@ -169,6 +169,11 @@ static void mwifiex_usb_rx_complete(struct urb *urb) > int recv_length = urb->actual_length; > int size, status; > > + if (!urb || !urb->context) { > + pr_err("URB or URB context is not valid in USB Rx complete\n"); > + return; > + } Something is really off if you need !urb here. Furthermore, you are already initializing stack variables using fields inside the urb before this check causing a null-deref so it is bogus anyway. The completion function is a member in struct urb. If your driver has a list of these urb's somewhere and they are free in parallel then your design is wrong and this does not solve the use-after-free. The urb here will not be NULL and still points to the old memory location as it was handed to the usb subsystem. That piece of memory might already been handed out to some other piece of the kernel making any access to it invalid and potentially crashing the kernel. Regards, Arend
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c index 2f7705c..ee5f488 100644 --- a/drivers/net/wireless/marvell/mwifiex/usb.c +++ b/drivers/net/wireless/marvell/mwifiex/usb.c @@ -169,6 +169,11 @@ static void mwifiex_usb_rx_complete(struct urb *urb) int recv_length = urb->actual_length; int size, status; + if (!urb || !urb->context) { + pr_err("URB or URB context is not valid in USB Rx complete\n"); + return; + } + if (!adapter || !adapter->card) { pr_err("mwifiex adapter or card structure is not valid\n"); return; @@ -260,6 +265,11 @@ static void mwifiex_usb_tx_complete(struct urb *urb) struct usb_tx_data_port *port; int i; + if (!urb || !urb->context) { + pr_err("URB or URB context is not valid in USB Tx complete\n"); + return; + } + mwifiex_dbg(adapter, INFO, "%s: status: %d\n", __func__, urb->status);