mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3] staging: rtl8188eu: Fix potential NULL pointer dereference of kcalloc
@ 2019-03-20 17:21 Aditya Pakki
  2019-03-20 19:42 ` Hans de Goede
  2019-03-20 20:20 ` Mukesh Ojha
  0 siblings, 2 replies; 3+ messages in thread
From: Aditya Pakki @ 2019-03-20 17:21 UTC (permalink / raw)
  To: pakki001
  Cc: kjlu, Larry Finger, Greg Kroah-Hartman, Michael Straube,
	Colin Ian King, Hardik Singh Rathore, Hans de Goede,
	Arnd Bergmann, Nathan Chancellor, devel, linux-kernel

hwxmits is allocated via kcalloc and not checked for failure before its
dereference. The patch fixes this problem by returning error upstream
in rtl8723bs, rtl8188eu.

Signed-off-by: Aditya Pakki <pakki001@umn.edu>

---
v2: Move signed off above version change log.
v1: Return error and remove print in case of failure, per Greg
---
 drivers/staging/rtl8188eu/core/rtw_xmit.c    |  9 +++++++--
 drivers/staging/rtl8188eu/include/rtw_xmit.h |  2 +-
 drivers/staging/rtl8723bs/core/rtw_xmit.c    | 14 +++++++-------
 drivers/staging/rtl8723bs/include/rtw_xmit.h |  2 +-
 4 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_xmit.c b/drivers/staging/rtl8188eu/core/rtw_xmit.c
index 1723a47a96b4..952f2ab51347 100644
--- a/drivers/staging/rtl8188eu/core/rtw_xmit.c
+++ b/drivers/staging/rtl8188eu/core/rtw_xmit.c
@@ -174,7 +174,9 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter)
 
 	pxmitpriv->free_xmit_extbuf_cnt = num_xmit_extbuf;
 
-	rtw_alloc_hwxmits(padapter);
+	res = rtw_alloc_hwxmits(padapter);
+	if (res == _FAIL)
+		goto exit;
 	rtw_init_hwxmits(pxmitpriv->hwxmits, pxmitpriv->hwxmit_entry);
 
 	for (i = 0; i < 4; i++)
@@ -1503,7 +1505,7 @@ s32 rtw_xmit_classifier(struct adapter *padapter, struct xmit_frame *pxmitframe)
 	return res;
 }
 
-void rtw_alloc_hwxmits(struct adapter *padapter)
+s32 rtw_alloc_hwxmits(struct adapter *padapter)
 {
 	struct hw_xmit *hwxmits;
 	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
@@ -1512,6 +1514,8 @@ void rtw_alloc_hwxmits(struct adapter *padapter)
 
 	pxmitpriv->hwxmits = kcalloc(pxmitpriv->hwxmit_entry,
 				     sizeof(struct hw_xmit), GFP_KERNEL);
+	if (!pxmitpriv->hwxmits)
+		return _FAIL;
 
 	hwxmits = pxmitpriv->hwxmits;
 
@@ -1519,6 +1523,7 @@ void rtw_alloc_hwxmits(struct adapter *padapter)
 	hwxmits[1] .sta_queue = &pxmitpriv->vi_pending;
 	hwxmits[2] .sta_queue = &pxmitpriv->be_pending;
 	hwxmits[3] .sta_queue = &pxmitpriv->bk_pending;
+	return _SUCCESS;
 }
 
 void rtw_free_hwxmits(struct adapter *padapter)
diff --git a/drivers/staging/rtl8188eu/include/rtw_xmit.h b/drivers/staging/rtl8188eu/include/rtw_xmit.h
index 788f59c74ea1..ba7e15fbde72 100644
--- a/drivers/staging/rtl8188eu/include/rtw_xmit.h
+++ b/drivers/staging/rtl8188eu/include/rtw_xmit.h
@@ -336,7 +336,7 @@ s32 rtw_txframes_sta_ac_pending(struct adapter *padapter,
 void rtw_init_hwxmits(struct hw_xmit *phwxmit, int entry);
 s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter);
 void _rtw_free_xmit_priv(struct xmit_priv *pxmitpriv);
-void rtw_alloc_hwxmits(struct adapter *padapter);
+s32 rtw_alloc_hwxmits(struct adapter *padapter);
 void rtw_free_hwxmits(struct adapter *padapter);
 s32 rtw_xmit(struct adapter *padapter, struct sk_buff **pkt);
 
diff --git a/drivers/staging/rtl8723bs/core/rtw_xmit.c b/drivers/staging/rtl8723bs/core/rtw_xmit.c
index 094d61bcb469..b87f13a0b563 100644
--- a/drivers/staging/rtl8723bs/core/rtw_xmit.c
+++ b/drivers/staging/rtl8723bs/core/rtw_xmit.c
@@ -260,7 +260,9 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter)
 		}
 	}
 
-	rtw_alloc_hwxmits(padapter);
+	res = rtw_alloc_hwxmits(padapter);
+	if (res == _FAIL)
+		goto exit;
 	rtw_init_hwxmits(pxmitpriv->hwxmits, pxmitpriv->hwxmit_entry);
 
 	for (i = 0; i < 4; i++) {
@@ -2144,7 +2146,7 @@ s32 rtw_xmit_classifier(struct adapter *padapter, struct xmit_frame *pxmitframe)
 	return res;
 }
 
-void rtw_alloc_hwxmits(struct adapter *padapter)
+s32 rtw_alloc_hwxmits(struct adapter *padapter)
 {
 	struct hw_xmit *hwxmits;
 	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
@@ -2155,10 +2157,8 @@ void rtw_alloc_hwxmits(struct adapter *padapter)
 
 	pxmitpriv->hwxmits = rtw_zmalloc(sizeof(struct hw_xmit) * pxmitpriv->hwxmit_entry);
 
-	if (pxmitpriv->hwxmits == NULL) {
-		DBG_871X("alloc hwxmits fail!...\n");
-		return;
-	}
+	if (!pxmitpriv->hwxmits)
+		return _FAIL;
 
 	hwxmits = pxmitpriv->hwxmits;
 
@@ -2204,7 +2204,7 @@ void rtw_alloc_hwxmits(struct adapter *padapter)
 
 	}
 
-
+	return _SUCCESS;
 }
 
 void rtw_free_hwxmits(struct adapter *padapter)
diff --git a/drivers/staging/rtl8723bs/include/rtw_xmit.h b/drivers/staging/rtl8723bs/include/rtw_xmit.h
index 1b38b9182b31..37f42b2f22f1 100644
--- a/drivers/staging/rtl8723bs/include/rtw_xmit.h
+++ b/drivers/staging/rtl8723bs/include/rtw_xmit.h
@@ -487,7 +487,7 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter);
 void _rtw_free_xmit_priv (struct xmit_priv *pxmitpriv);
 
 
-void rtw_alloc_hwxmits(struct adapter *padapter);
+s32 rtw_alloc_hwxmits(struct adapter *padapter);
 void rtw_free_hwxmits(struct adapter *padapter);
 
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] staging: rtl8188eu: Fix potential NULL pointer dereference of kcalloc
  2019-03-20 17:21 [PATCH v3] staging: rtl8188eu: Fix potential NULL pointer dereference of kcalloc Aditya Pakki
@ 2019-03-20 19:42 ` Hans de Goede
  2019-03-20 20:20 ` Mukesh Ojha
  1 sibling, 0 replies; 3+ messages in thread
From: Hans de Goede @ 2019-03-20 19:42 UTC (permalink / raw)
  To: Aditya Pakki
  Cc: kjlu, Larry Finger, Greg Kroah-Hartman, Michael Straube,
	Colin Ian King, Hardik Singh Rathore, Arnd Bergmann,
	Nathan Chancellor, devel, linux-kernel

Hi,

On 20-03-19 18:21, Aditya Pakki wrote:
> hwxmits is allocated via kcalloc and not checked for failure before its
> dereference. The patch fixes this problem by returning error upstream
> in rtl8723bs, rtl8188eu.
> 
> Signed-off-by: Aditya Pakki <pakki001@umn.edu>

Patch look good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans

> 
> ---
> v2: Move signed off above version change log.
> v1: Return error and remove print in case of failure, per Greg
> ---
>   drivers/staging/rtl8188eu/core/rtw_xmit.c    |  9 +++++++--
>   drivers/staging/rtl8188eu/include/rtw_xmit.h |  2 +-
>   drivers/staging/rtl8723bs/core/rtw_xmit.c    | 14 +++++++-------
>   drivers/staging/rtl8723bs/include/rtw_xmit.h |  2 +-
>   4 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/core/rtw_xmit.c b/drivers/staging/rtl8188eu/core/rtw_xmit.c
> index 1723a47a96b4..952f2ab51347 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_xmit.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_xmit.c
> @@ -174,7 +174,9 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter)
>   
>   	pxmitpriv->free_xmit_extbuf_cnt = num_xmit_extbuf;
>   
> -	rtw_alloc_hwxmits(padapter);
> +	res = rtw_alloc_hwxmits(padapter);
> +	if (res == _FAIL)
> +		goto exit;
>   	rtw_init_hwxmits(pxmitpriv->hwxmits, pxmitpriv->hwxmit_entry);
>   
>   	for (i = 0; i < 4; i++)
> @@ -1503,7 +1505,7 @@ s32 rtw_xmit_classifier(struct adapter *padapter, struct xmit_frame *pxmitframe)
>   	return res;
>   }
>   
> -void rtw_alloc_hwxmits(struct adapter *padapter)
> +s32 rtw_alloc_hwxmits(struct adapter *padapter)
>   {
>   	struct hw_xmit *hwxmits;
>   	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
> @@ -1512,6 +1514,8 @@ void rtw_alloc_hwxmits(struct adapter *padapter)
>   
>   	pxmitpriv->hwxmits = kcalloc(pxmitpriv->hwxmit_entry,
>   				     sizeof(struct hw_xmit), GFP_KERNEL);
> +	if (!pxmitpriv->hwxmits)
> +		return _FAIL;
>   
>   	hwxmits = pxmitpriv->hwxmits;
>   
> @@ -1519,6 +1523,7 @@ void rtw_alloc_hwxmits(struct adapter *padapter)
>   	hwxmits[1] .sta_queue = &pxmitpriv->vi_pending;
>   	hwxmits[2] .sta_queue = &pxmitpriv->be_pending;
>   	hwxmits[3] .sta_queue = &pxmitpriv->bk_pending;
> +	return _SUCCESS;
>   }
>   
>   void rtw_free_hwxmits(struct adapter *padapter)
> diff --git a/drivers/staging/rtl8188eu/include/rtw_xmit.h b/drivers/staging/rtl8188eu/include/rtw_xmit.h
> index 788f59c74ea1..ba7e15fbde72 100644
> --- a/drivers/staging/rtl8188eu/include/rtw_xmit.h
> +++ b/drivers/staging/rtl8188eu/include/rtw_xmit.h
> @@ -336,7 +336,7 @@ s32 rtw_txframes_sta_ac_pending(struct adapter *padapter,
>   void rtw_init_hwxmits(struct hw_xmit *phwxmit, int entry);
>   s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter);
>   void _rtw_free_xmit_priv(struct xmit_priv *pxmitpriv);
> -void rtw_alloc_hwxmits(struct adapter *padapter);
> +s32 rtw_alloc_hwxmits(struct adapter *padapter);
>   void rtw_free_hwxmits(struct adapter *padapter);
>   s32 rtw_xmit(struct adapter *padapter, struct sk_buff **pkt);
>   
> diff --git a/drivers/staging/rtl8723bs/core/rtw_xmit.c b/drivers/staging/rtl8723bs/core/rtw_xmit.c
> index 094d61bcb469..b87f13a0b563 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_xmit.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_xmit.c
> @@ -260,7 +260,9 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter)
>   		}
>   	}
>   
> -	rtw_alloc_hwxmits(padapter);
> +	res = rtw_alloc_hwxmits(padapter);
> +	if (res == _FAIL)
> +		goto exit;
>   	rtw_init_hwxmits(pxmitpriv->hwxmits, pxmitpriv->hwxmit_entry);
>   
>   	for (i = 0; i < 4; i++) {
> @@ -2144,7 +2146,7 @@ s32 rtw_xmit_classifier(struct adapter *padapter, struct xmit_frame *pxmitframe)
>   	return res;
>   }
>   
> -void rtw_alloc_hwxmits(struct adapter *padapter)
> +s32 rtw_alloc_hwxmits(struct adapter *padapter)
>   {
>   	struct hw_xmit *hwxmits;
>   	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
> @@ -2155,10 +2157,8 @@ void rtw_alloc_hwxmits(struct adapter *padapter)
>   
>   	pxmitpriv->hwxmits = rtw_zmalloc(sizeof(struct hw_xmit) * pxmitpriv->hwxmit_entry);
>   
> -	if (pxmitpriv->hwxmits == NULL) {
> -		DBG_871X("alloc hwxmits fail!...\n");
> -		return;
> -	}
> +	if (!pxmitpriv->hwxmits)
> +		return _FAIL;
>   
>   	hwxmits = pxmitpriv->hwxmits;
>   
> @@ -2204,7 +2204,7 @@ void rtw_alloc_hwxmits(struct adapter *padapter)
>   
>   	}
>   
> -
> +	return _SUCCESS;
>   }
>   
>   void rtw_free_hwxmits(struct adapter *padapter)
> diff --git a/drivers/staging/rtl8723bs/include/rtw_xmit.h b/drivers/staging/rtl8723bs/include/rtw_xmit.h
> index 1b38b9182b31..37f42b2f22f1 100644
> --- a/drivers/staging/rtl8723bs/include/rtw_xmit.h
> +++ b/drivers/staging/rtl8723bs/include/rtw_xmit.h
> @@ -487,7 +487,7 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter);
>   void _rtw_free_xmit_priv (struct xmit_priv *pxmitpriv);
>   
>   
> -void rtw_alloc_hwxmits(struct adapter *padapter);
> +s32 rtw_alloc_hwxmits(struct adapter *padapter);
>   void rtw_free_hwxmits(struct adapter *padapter);
>   
>   
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] staging: rtl8188eu: Fix potential NULL pointer dereference of kcalloc
  2019-03-20 17:21 [PATCH v3] staging: rtl8188eu: Fix potential NULL pointer dereference of kcalloc Aditya Pakki
  2019-03-20 19:42 ` Hans de Goede
@ 2019-03-20 20:20 ` Mukesh Ojha
  1 sibling, 0 replies; 3+ messages in thread
From: Mukesh Ojha @ 2019-03-20 20:20 UTC (permalink / raw)
  To: Aditya Pakki
  Cc: kjlu, Larry Finger, Greg Kroah-Hartman, Michael Straube,
	Colin Ian King, Hardik Singh Rathore, Hans de Goede,
	Arnd Bergmann, Nathan Chancellor, devel, linux-kernel


On 3/20/2019 10:51 PM, Aditya Pakki wrote:
> hwxmits is allocated via kcalloc and not checked for failure before its

No need to mention  kcalloc as the other place allocates the memory 
through kmalloc.

Otherwise looks good.
Acked-by: Mukesh Ojha <mojha@codeaurora.org>

> dereference. The patch fixes this problem by returning error upstream
> in rtl8723bs, rtl8188eu.
>
> Signed-off-by: Aditya Pakki <pakki001@umn.edu>
>
> ---
> v2: Move signed off above version change log.
> v1: Return error and remove print in case of failure, per Greg
> ---
>   drivers/staging/rtl8188eu/core/rtw_xmit.c    |  9 +++++++--
>   drivers/staging/rtl8188eu/include/rtw_xmit.h |  2 +-
>   drivers/staging/rtl8723bs/core/rtw_xmit.c    | 14 +++++++-------
>   drivers/staging/rtl8723bs/include/rtw_xmit.h |  2 +-
>   4 files changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/rtl8188eu/core/rtw_xmit.c b/drivers/staging/rtl8188eu/core/rtw_xmit.c
> index 1723a47a96b4..952f2ab51347 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_xmit.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_xmit.c
> @@ -174,7 +174,9 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter)
>   
>   	pxmitpriv->free_xmit_extbuf_cnt = num_xmit_extbuf;
>   
> -	rtw_alloc_hwxmits(padapter);
> +	res = rtw_alloc_hwxmits(padapter);
> +	if (res == _FAIL)
> +		goto exit;
>   	rtw_init_hwxmits(pxmitpriv->hwxmits, pxmitpriv->hwxmit_entry);
>   
>   	for (i = 0; i < 4; i++)
> @@ -1503,7 +1505,7 @@ s32 rtw_xmit_classifier(struct adapter *padapter, struct xmit_frame *pxmitframe)
>   	return res;
>   }
>   
> -void rtw_alloc_hwxmits(struct adapter *padapter)
> +s32 rtw_alloc_hwxmits(struct adapter *padapter)
>   {
>   	struct hw_xmit *hwxmits;
>   	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
> @@ -1512,6 +1514,8 @@ void rtw_alloc_hwxmits(struct adapter *padapter)
>   
>   	pxmitpriv->hwxmits = kcalloc(pxmitpriv->hwxmit_entry,
>   				     sizeof(struct hw_xmit), GFP_KERNEL);
> +	if (!pxmitpriv->hwxmits)
> +		return _FAIL;
>   
>   	hwxmits = pxmitpriv->hwxmits;
>   
> @@ -1519,6 +1523,7 @@ void rtw_alloc_hwxmits(struct adapter *padapter)
>   	hwxmits[1] .sta_queue = &pxmitpriv->vi_pending;
>   	hwxmits[2] .sta_queue = &pxmitpriv->be_pending;
>   	hwxmits[3] .sta_queue = &pxmitpriv->bk_pending;
> +	return _SUCCESS;
>   }
>   
>   void rtw_free_hwxmits(struct adapter *padapter)
> diff --git a/drivers/staging/rtl8188eu/include/rtw_xmit.h b/drivers/staging/rtl8188eu/include/rtw_xmit.h
> index 788f59c74ea1..ba7e15fbde72 100644
> --- a/drivers/staging/rtl8188eu/include/rtw_xmit.h
> +++ b/drivers/staging/rtl8188eu/include/rtw_xmit.h
> @@ -336,7 +336,7 @@ s32 rtw_txframes_sta_ac_pending(struct adapter *padapter,
>   void rtw_init_hwxmits(struct hw_xmit *phwxmit, int entry);
>   s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter);
>   void _rtw_free_xmit_priv(struct xmit_priv *pxmitpriv);
> -void rtw_alloc_hwxmits(struct adapter *padapter);
> +s32 rtw_alloc_hwxmits(struct adapter *padapter);
>   void rtw_free_hwxmits(struct adapter *padapter);
>   s32 rtw_xmit(struct adapter *padapter, struct sk_buff **pkt);
>   
> diff --git a/drivers/staging/rtl8723bs/core/rtw_xmit.c b/drivers/staging/rtl8723bs/core/rtw_xmit.c
> index 094d61bcb469..b87f13a0b563 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_xmit.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_xmit.c
> @@ -260,7 +260,9 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter)
>   		}
>   	}
>   
> -	rtw_alloc_hwxmits(padapter);
> +	res = rtw_alloc_hwxmits(padapter);
> +	if (res == _FAIL)
> +		goto exit;
>   	rtw_init_hwxmits(pxmitpriv->hwxmits, pxmitpriv->hwxmit_entry);
>   
>   	for (i = 0; i < 4; i++) {
> @@ -2144,7 +2146,7 @@ s32 rtw_xmit_classifier(struct adapter *padapter, struct xmit_frame *pxmitframe)
>   	return res;
>   }
>   
> -void rtw_alloc_hwxmits(struct adapter *padapter)
> +s32 rtw_alloc_hwxmits(struct adapter *padapter)
>   {
>   	struct hw_xmit *hwxmits;
>   	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
> @@ -2155,10 +2157,8 @@ void rtw_alloc_hwxmits(struct adapter *padapter)
>   
>   	pxmitpriv->hwxmits = rtw_zmalloc(sizeof(struct hw_xmit) * pxmitpriv->hwxmit_entry);
>   
> -	if (pxmitpriv->hwxmits == NULL) {
> -		DBG_871X("alloc hwxmits fail!...\n");
> -		return;
> -	}
> +	if (!pxmitpriv->hwxmits)
> +		return _FAIL;
>   
>   	hwxmits = pxmitpriv->hwxmits;
>   
> @@ -2204,7 +2204,7 @@ void rtw_alloc_hwxmits(struct adapter *padapter)
>   
>   	}
>   
> -
> +	return _SUCCESS;
>   }
>   
>   void rtw_free_hwxmits(struct adapter *padapter)
> diff --git a/drivers/staging/rtl8723bs/include/rtw_xmit.h b/drivers/staging/rtl8723bs/include/rtw_xmit.h
> index 1b38b9182b31..37f42b2f22f1 100644
> --- a/drivers/staging/rtl8723bs/include/rtw_xmit.h
> +++ b/drivers/staging/rtl8723bs/include/rtw_xmit.h
> @@ -487,7 +487,7 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter);
>   void _rtw_free_xmit_priv (struct xmit_priv *pxmitpriv);
>   
>   
> -void rtw_alloc_hwxmits(struct adapter *padapter);
> +s32 rtw_alloc_hwxmits(struct adapter *padapter);
>   void rtw_free_hwxmits(struct adapter *padapter);
>   
>   

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-03-20 20:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-20 17:21 [PATCH v3] staging: rtl8188eu: Fix potential NULL pointer dereference of kcalloc Aditya Pakki
2019-03-20 19:42 ` Hans de Goede
2019-03-20 20:20 ` Mukesh Ojha

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®