* [PATCH v2] staging:rtl8192u: Check memory allocation
@ 2017-03-01 21:30 Georgiana Rodica Chelu
2017-03-01 22:46 ` [Outreachy kernel] " Julia Lawall
0 siblings, 1 reply; 2+ messages in thread
From: Georgiana Rodica Chelu @ 2017-03-01 21:30 UTC (permalink / raw)
To: outreachy-kernel; +Cc: gregkh
Check if the allocation is not successful and
return the error code -ENOMEM.
Signed-off-by: Georgiana Rodica Chelu <georgiana.chelu93@gmail.com>
---
Changes in v2:
- use a goto pattern to handle the allocation fails
- use exiting a function to free the memory in case of fail
- move the free function above the initialization
drivers/staging/rtl8192u/r8192U_core.c | 109 ++++++++++++++++++---------------
1 file changed, 60 insertions(+), 49 deletions(-)
diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
index b631990..08d7e70 100644
--- a/drivers/staging/rtl8192u/r8192U_core.c
+++ b/drivers/staging/rtl8192u/r8192U_core.c
@@ -1677,6 +1677,56 @@ short rtl8192_tx(struct net_device *dev, struct sk_buff *skb)
return -1;
}
+#ifdef THOMAS_BEACON
+static void rtl8192_usb_deleteendpoints(struct net_device *dev)
+{
+ int i;
+ struct r8192_priv *priv = ieee80211_priv(dev);
+
+ if (priv->rx_urb) {
+ for (i = 0; i < (MAX_RX_URB + 1); i++) {
+ usb_kill_urb(priv->rx_urb[i]);
+ usb_free_urb(priv->rx_urb[i]);
+ }
+ kfree(priv->rx_urb);
+ priv->rx_urb = NULL;
+ }
+ kfree(priv->oldaddr);
+ priv->oldaddr = NULL;
+
+ kfree(priv->pp_rxskb);
+ priv->pp_rxskb = NULL;
+}
+#else
+void rtl8192_usb_deleteendpoints(struct net_device *dev)
+{
+ int i;
+ struct r8192_priv *priv = ieee80211_priv(dev);
+
+#ifndef JACKSON_NEW_RX
+
+ if (priv->rx_urb) {
+ for (i = 0; i < (MAX_RX_URB + 1); i++) {
+ usb_kill_urb(priv->rx_urb[i]);
+ kfree(priv->rx_urb[i]->transfer_buffer);
+ usb_free_urb(priv->rx_urb[i]);
+ }
+ kfree(priv->rx_urb);
+ priv->rx_urb = NULL;
+ }
+#else
+ kfree(priv->rx_urb);
+ priv->rx_urb = NULL;
+ kfree(priv->oldaddr);
+ priv->oldaddr = NULL;
+
+ kfree(priv->pp_rxskb);
+ priv->pp_rxskb = 0;
+
+#endif
+}
+#endif
+
static short rtl8192_usb_initendpoints(struct net_device *dev)
{
struct r8192_priv *priv = ieee80211_priv(dev);
@@ -1689,9 +1739,13 @@ static short rtl8192_usb_initendpoints(struct net_device *dev)
#ifndef JACKSON_NEW_RX
for (i = 0; i < (MAX_RX_URB + 1); i++) {
priv->rx_urb[i] = usb_alloc_urb(0, GFP_KERNEL);
+ if(!priv->rx_urb[i])
+ goto exit;
priv->rx_urb[i]->transfer_buffer =
kmalloc(RX_URB_SIZE, GFP_KERNEL);
+ if(!priv->rx_urb[i]->transfer_buffer)
+ goto exit;
priv->rx_urb[i]->transfer_buffer_length = RX_URB_SIZE;
}
@@ -1704,6 +1758,9 @@ static short rtl8192_usb_initendpoints(struct net_device *dev)
priv->rx_urb[16] = usb_alloc_urb(0, GFP_KERNEL);
priv->oldaddr = kmalloc(16, GFP_KERNEL);
+ if(!priv->oldaddr)
+ goto exit;
+
oldaddr = priv->oldaddr;
align = ((long)oldaddr) & 3;
if (align) {
@@ -1732,57 +1789,11 @@ static short rtl8192_usb_initendpoints(struct net_device *dev)
netdev_dbg(dev, "End of initendpoints\n");
return 0;
-}
-
-#ifdef THOMAS_BEACON
-static void rtl8192_usb_deleteendpoints(struct net_device *dev)
-{
- int i;
- struct r8192_priv *priv = ieee80211_priv(dev);
-
- if (priv->rx_urb) {
- for (i = 0; i < (MAX_RX_URB + 1); i++) {
- usb_kill_urb(priv->rx_urb[i]);
- usb_free_urb(priv->rx_urb[i]);
- }
- kfree(priv->rx_urb);
- priv->rx_urb = NULL;
- }
- kfree(priv->oldaddr);
- priv->oldaddr = NULL;
-
- kfree(priv->pp_rxskb);
- priv->pp_rxskb = NULL;
-}
-#else
-void rtl8192_usb_deleteendpoints(struct net_device *dev)
-{
- int i;
- struct r8192_priv *priv = ieee80211_priv(dev);
-
-#ifndef JACKSON_NEW_RX
- if (priv->rx_urb) {
- for (i = 0; i < (MAX_RX_URB + 1); i++) {
- usb_kill_urb(priv->rx_urb[i]);
- kfree(priv->rx_urb[i]->transfer_buffer);
- usb_free_urb(priv->rx_urb[i]);
- }
- kfree(priv->rx_urb);
- priv->rx_urb = NULL;
- }
-#else
- kfree(priv->rx_urb);
- priv->rx_urb = NULL;
- kfree(priv->oldaddr);
- priv->oldaddr = NULL;
-
- kfree(priv->pp_rxskb);
- priv->pp_rxskb = 0;
-
-#endif
+exit:
+ rtl8192_usb_deleteendpoints(dev);
+ return -ENOMEM;
}
-#endif
static void rtl8192_update_ratr_table(struct net_device *dev);
static void rtl8192_link_change(struct net_device *dev)
--
2.7.4
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging:rtl8192u: Check memory allocation
2017-03-01 21:30 [PATCH v2] staging:rtl8192u: Check memory allocation Georgiana Rodica Chelu
@ 2017-03-01 22:46 ` Julia Lawall
0 siblings, 0 replies; 2+ messages in thread
From: Julia Lawall @ 2017-03-01 22:46 UTC (permalink / raw)
To: Georgiana Rodica Chelu; +Cc: outreachy-kernel, gregkh
On Wed, 1 Mar 2017, Georgiana Rodica Chelu wrote:
> Check if the allocation is not successful and
> return the error code -ENOMEM.
I see two issues.
First, in the JACKSON_NEW_RX code you are calling the freeing functions on
an array that may not be fully initialized. kfree and usb_free_urb will
just return immediately if a NULL pointer is provided as an argument, so
things are partially OK. But actually priv->rx_urb[i] will not be NULL in
the uninitialized case, because kmalloc was used. Kmalloc allocatesmemory
but doesn't zero it. For that you would need kzalloc.
Then in the JACKSON_NEW_RX ifdef of rtl8192_usb_deleteendpoints, it could
be that priv->rx_urb[i] is NULL for some values of i, which will give a
NULL pointer dereference in priv->rx_urb[i]->transfer_buffer. You may as
well test for a NULL value and break out of the loop. But be careful,
because it could be that the usb_alloc_urb succeeded and the subsequent
kmalloc to initialize priv->rx_urb[i]->transfer_buffer failed.
Finally, at the end of the init function, the error handling code that is
present there is not sufficient if either of the previous ifdefs is
satisfied.
julia
>
> Signed-off-by: Georgiana Rodica Chelu <georgiana.chelu93@gmail.com>
> ---
>
> Changes in v2:
> - use a goto pattern to handle the allocation fails
> - use exiting a function to free the memory in case of fail
> - move the free function above the initialization
>
> drivers/staging/rtl8192u/r8192U_core.c | 109 ++++++++++++++++++---------------
> 1 file changed, 60 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
> index b631990..08d7e70 100644
> --- a/drivers/staging/rtl8192u/r8192U_core.c
> +++ b/drivers/staging/rtl8192u/r8192U_core.c
> @@ -1677,6 +1677,56 @@ short rtl8192_tx(struct net_device *dev, struct sk_buff *skb)
> return -1;
> }
>
> +#ifdef THOMAS_BEACON
> +static void rtl8192_usb_deleteendpoints(struct net_device *dev)
> +{
> + int i;
> + struct r8192_priv *priv = ieee80211_priv(dev);
> +
> + if (priv->rx_urb) {
> + for (i = 0; i < (MAX_RX_URB + 1); i++) {
> + usb_kill_urb(priv->rx_urb[i]);
> + usb_free_urb(priv->rx_urb[i]);
> + }
> + kfree(priv->rx_urb);
> + priv->rx_urb = NULL;
> + }
> + kfree(priv->oldaddr);
> + priv->oldaddr = NULL;
> +
> + kfree(priv->pp_rxskb);
> + priv->pp_rxskb = NULL;
> +}
> +#else
> +void rtl8192_usb_deleteendpoints(struct net_device *dev)
> +{
> + int i;
> + struct r8192_priv *priv = ieee80211_priv(dev);
> +
> +#ifndef JACKSON_NEW_RX
> +
> + if (priv->rx_urb) {
> + for (i = 0; i < (MAX_RX_URB + 1); i++) {
> + usb_kill_urb(priv->rx_urb[i]);
> + kfree(priv->rx_urb[i]->transfer_buffer);
> + usb_free_urb(priv->rx_urb[i]);
> + }
> + kfree(priv->rx_urb);
> + priv->rx_urb = NULL;
> + }
> +#else
> + kfree(priv->rx_urb);
> + priv->rx_urb = NULL;
> + kfree(priv->oldaddr);
> + priv->oldaddr = NULL;
> +
> + kfree(priv->pp_rxskb);
> + priv->pp_rxskb = 0;
> +
> +#endif
> +}
> +#endif
> +
> static short rtl8192_usb_initendpoints(struct net_device *dev)
> {
> struct r8192_priv *priv = ieee80211_priv(dev);
> @@ -1689,9 +1739,13 @@ static short rtl8192_usb_initendpoints(struct net_device *dev)
> #ifndef JACKSON_NEW_RX
> for (i = 0; i < (MAX_RX_URB + 1); i++) {
> priv->rx_urb[i] = usb_alloc_urb(0, GFP_KERNEL);
> + if(!priv->rx_urb[i])
> + goto exit;
>
> priv->rx_urb[i]->transfer_buffer =
> kmalloc(RX_URB_SIZE, GFP_KERNEL);
> + if(!priv->rx_urb[i]->transfer_buffer)
> + goto exit;
>
> priv->rx_urb[i]->transfer_buffer_length = RX_URB_SIZE;
> }
> @@ -1704,6 +1758,9 @@ static short rtl8192_usb_initendpoints(struct net_device *dev)
>
> priv->rx_urb[16] = usb_alloc_urb(0, GFP_KERNEL);
> priv->oldaddr = kmalloc(16, GFP_KERNEL);
> + if(!priv->oldaddr)
> + goto exit;
> +
> oldaddr = priv->oldaddr;
> align = ((long)oldaddr) & 3;
> if (align) {
> @@ -1732,57 +1789,11 @@ static short rtl8192_usb_initendpoints(struct net_device *dev)
>
> netdev_dbg(dev, "End of initendpoints\n");
> return 0;
> -}
> -
> -#ifdef THOMAS_BEACON
> -static void rtl8192_usb_deleteendpoints(struct net_device *dev)
> -{
> - int i;
> - struct r8192_priv *priv = ieee80211_priv(dev);
> -
> - if (priv->rx_urb) {
> - for (i = 0; i < (MAX_RX_URB + 1); i++) {
> - usb_kill_urb(priv->rx_urb[i]);
> - usb_free_urb(priv->rx_urb[i]);
> - }
> - kfree(priv->rx_urb);
> - priv->rx_urb = NULL;
> - }
> - kfree(priv->oldaddr);
> - priv->oldaddr = NULL;
> -
> - kfree(priv->pp_rxskb);
> - priv->pp_rxskb = NULL;
> -}
> -#else
> -void rtl8192_usb_deleteendpoints(struct net_device *dev)
> -{
> - int i;
> - struct r8192_priv *priv = ieee80211_priv(dev);
> -
> -#ifndef JACKSON_NEW_RX
>
> - if (priv->rx_urb) {
> - for (i = 0; i < (MAX_RX_URB + 1); i++) {
> - usb_kill_urb(priv->rx_urb[i]);
> - kfree(priv->rx_urb[i]->transfer_buffer);
> - usb_free_urb(priv->rx_urb[i]);
> - }
> - kfree(priv->rx_urb);
> - priv->rx_urb = NULL;
> - }
> -#else
> - kfree(priv->rx_urb);
> - priv->rx_urb = NULL;
> - kfree(priv->oldaddr);
> - priv->oldaddr = NULL;
> -
> - kfree(priv->pp_rxskb);
> - priv->pp_rxskb = 0;
> -
> -#endif
> +exit:
> + rtl8192_usb_deleteendpoints(dev);
> + return -ENOMEM;
> }
> -#endif
>
> static void rtl8192_update_ratr_table(struct net_device *dev);
> static void rtl8192_link_change(struct net_device *dev)
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170301213000.GA14437%40fireworks.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-03-01 22:46 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-01 21:30 [PATCH v2] staging:rtl8192u: Check memory allocation Georgiana Rodica Chelu
2017-03-01 22:46 ` [Outreachy kernel] " Julia Lawall
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.