* [PATCH v2 1/2] staging: r8188eu: check the return of kzalloc() @ 2022-03-31 10:56 xkernel.wang 2022-03-31 10:58 ` [PATCH v2 2/2] staging: r8188eu: fix potential memory leak in _rtw_init_xmit_priv() xkernel.wang 0 siblings, 1 reply; 3+ messages in thread From: xkernel.wang @ 2022-03-31 10:56 UTC (permalink / raw) To: gregkh, dan.carpenter Cc: Larry.Finger, phil, linux-staging, linux-kernel, Xiaoke Wang From: Xiaoke Wang <xkernel.wang@foxmail.com> kzalloc() is a memory allocation function which can return NULL when some internal memory errors happen. So it is better to handle the return of it to prevent potential wrong memory access. Signed-off-by: Xiaoke Wang <xkernel.wang@foxmail.com> --- Note: The subsequent patch is specific to properly release the resources, while this one is just take aware of the validation for the return of kzalloc(). ChangeLog: v1->v2: optimize the style and seperate an another patch. drivers/staging/r8188eu/core/rtw_xmit.c | 10 ++++++++-- drivers/staging/r8188eu/include/rtw_xmit.h | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/staging/r8188eu/core/rtw_xmit.c b/drivers/staging/r8188eu/core/rtw_xmit.c index 46fe62c..299fe26 100644 --- a/drivers/staging/r8188eu/core/rtw_xmit.c +++ b/drivers/staging/r8188eu/core/rtw_xmit.c @@ -179,7 +179,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++) @@ -1516,7 +1518,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; @@ -1524,6 +1526,8 @@ void rtw_alloc_hwxmits(struct adapter *padapter) pxmitpriv->hwxmit_entry = HWXMIT_ENTRY; pxmitpriv->hwxmits = kzalloc(sizeof(struct hw_xmit) * pxmitpriv->hwxmit_entry, GFP_KERNEL); + if (!pxmitpriv->hwxmits) + return _FAIL; hwxmits = pxmitpriv->hwxmits; @@ -1540,6 +1544,8 @@ void rtw_alloc_hwxmits(struct adapter *padapter) hwxmits[3] .sta_queue = &pxmitpriv->bk_pending; } else { } + + return _SUCCESS; } void rtw_free_hwxmits(struct adapter *padapter) diff --git a/drivers/staging/r8188eu/include/rtw_xmit.h b/drivers/staging/r8188eu/include/rtw_xmit.h index 5f6e240..b45cd29 100644 --- a/drivers/staging/r8188eu/include/rtw_xmit.h +++ b/drivers/staging/r8188eu/include/rtw_xmit.h @@ -345,7 +345,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); -- ^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH v2 2/2] staging: r8188eu: fix potential memory leak in _rtw_init_xmit_priv() 2022-03-31 10:56 [PATCH v2 1/2] staging: r8188eu: check the return of kzalloc() xkernel.wang @ 2022-03-31 10:58 ` xkernel.wang 2022-03-31 11:23 ` Dan Carpenter 0 siblings, 1 reply; 3+ messages in thread From: xkernel.wang @ 2022-03-31 10:58 UTC (permalink / raw) To: gregkh, dan.carpenter Cc: Larry.Finger, phil, linux-staging, linux-kernel, Xiaoke Wang From: Xiaoke Wang <xkernel.wang@foxmail.com> In _rtw_init_xmit_priv(), there are several error paths for allocation failures without releasing the resources. To properly release them, several error handling paths are added. Signed-off-by: Xiaoke Wang <xkernel.wang@foxmail.com> --- ChangeLog: v1->v2 unified all the error handling code in free_* section. remove the unnecessary modifications for rtw_os_xmit_resource_free(). drivers/staging/r8188eu/core/rtw_xmit.c | 37 +++++++++++++++++-------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/drivers/staging/r8188eu/core/rtw_xmit.c b/drivers/staging/r8188eu/core/rtw_xmit.c index 299fe26..e6bbbf7 100644 --- a/drivers/staging/r8188eu/core/rtw_xmit.c +++ b/drivers/staging/r8188eu/core/rtw_xmit.c @@ -112,7 +112,7 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter) if (!pxmitpriv->pallocated_xmitbuf) { res = _FAIL; - goto exit; + goto free_frame_buf; } pxmitpriv->pxmitbuf = (u8 *)N_BYTE_ALIGMENT((size_t)(pxmitpriv->pallocated_xmitbuf), 4); @@ -133,9 +133,8 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter) if (res == _FAIL) { msleep(10); res = rtw_os_xmit_resource_alloc(padapter, pxmitbuf, (MAX_XMITBUF_SZ + XMITBUF_ALIGN_SZ)); - if (res == _FAIL) { - goto exit; - } + if (res == _FAIL) + goto free_xmitbuf; } pxmitbuf->flags = XMIT_VO_QUEUE; @@ -153,7 +152,7 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter) if (!pxmitpriv->pallocated_xmit_extbuf) { res = _FAIL; - goto exit; + goto free_xmitbuf; } pxmitpriv->pxmit_extbuf = (u8 *)N_BYTE_ALIGMENT((size_t)(pxmitpriv->pallocated_xmit_extbuf), 4); @@ -168,10 +167,8 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter) pxmitbuf->ext_tag = true; res = rtw_os_xmit_resource_alloc(padapter, pxmitbuf, max_xmit_extbuf_size + XMITBUF_ALIGN_SZ); - if (res == _FAIL) { - res = _FAIL; - goto exit; - } + if (res == _FAIL) + goto free_xmit_extbuf; list_add_tail(&pxmitbuf->list, &pxmitpriv->free_xmit_extbuf_queue.queue); pxmitbuf++; @@ -181,7 +178,7 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter) res = rtw_alloc_hwxmits(padapter); if (res == _FAIL) - goto exit; + goto free_xmit_extbuf; rtw_init_hwxmits(pxmitpriv->hwxmits, pxmitpriv->hwxmit_entry); for (i = 0; i < 4; i++) @@ -203,8 +200,26 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter) rtw_hal_init_xmit_priv(padapter); -exit: + return res; +free_xmit_extbuf: + pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmit_extbuf; + while (i-- > 0) { + rtw_os_xmit_resource_free(padapter, pxmitbuf, (max_xmit_extbuf_size + XMITBUF_ALIGN_SZ)); + pxmitbuf++; + } + vfree(pxmitpriv->pallocated_xmit_extbuf); + i = NR_XMITBUFF; +free_xmitbuf: + pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmitbuf; + while (i-- > 0) { + rtw_os_xmit_resource_free(padapter, pxmitbuf, (MAX_XMITBUF_SZ + XMITBUF_ALIGN_SZ)); + pxmitbuf++; + } + vfree(pxmitpriv->pallocated_xmitbuf); +free_frame_buf: + vfree(pxmitpriv->pallocated_frame_buf); +exit: return res; } -- ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2 2/2] staging: r8188eu: fix potential memory leak in _rtw_init_xmit_priv() 2022-03-31 10:58 ` [PATCH v2 2/2] staging: r8188eu: fix potential memory leak in _rtw_init_xmit_priv() xkernel.wang @ 2022-03-31 11:23 ` Dan Carpenter 0 siblings, 0 replies; 3+ messages in thread From: Dan Carpenter @ 2022-03-31 11:23 UTC (permalink / raw) To: xkernel.wang; +Cc: gregkh, Larry.Finger, phil, linux-staging, linux-kernel On Thu, Mar 31, 2022 at 06:58:16PM +0800, xkernel.wang@foxmail.com wrote: > +free_xmit_extbuf: > + pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmit_extbuf; > + while (i-- > 0) { > + rtw_os_xmit_resource_free(padapter, pxmitbuf, (max_xmit_extbuf_size + XMITBUF_ALIGN_SZ)); > + pxmitbuf++; > + } > + vfree(pxmitpriv->pallocated_xmit_extbuf); > + i = NR_XMITBUFF; > +free_xmitbuf: > + pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmitbuf; > + while (i-- > 0) { > + rtw_os_xmit_resource_free(padapter, pxmitbuf, (MAX_XMITBUF_SZ + XMITBUF_ALIGN_SZ)); > + pxmitbuf++; > + } This works... Presumably, it applies to staging-next even though it doesn't apply to linux-next. So: Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com> In an ideal world, pxmitpriv->pxmitbuf would be declared as an array of struct instead of an array of u8. That would make it much simpler because we could do free_xmit_extbuf: while (--i >= 0) rtw_os_xmit_resource_free(pxmitpriv->pxmit_extbuf[i]); vfree(pxmitpriv->pxmit_extbuf); i = NR_XMITBUFF; free_xmitbuf: while (--i >= 0) rtw_os_xmit_resource_free(pxmitpriv->pxmitbuf[i]); regards, dan carpenter ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-03-31 11:24 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-03-31 10:56 [PATCH v2 1/2] staging: r8188eu: check the return of kzalloc() xkernel.wang 2022-03-31 10:58 ` [PATCH v2 2/2] staging: r8188eu: fix potential memory leak in _rtw_init_xmit_priv() xkernel.wang 2022-03-31 11:23 ` Dan Carpenter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox