On Tue, 29 Mar 2022 23:57:10 +0800, gregkh@xxxxxxxxxxxxxxxxxxx wrote:
>> pdata_attr = kzalloc(MAX_P2P_IE_LEN, GFP_KERNEL);
>> + if (!pdata_attr)
>> + return 0;
>
> 0 is not an error. Please propagate this error backwards properly.
Ok. But the function go_add_group_info_attr() is called by:
> p2pielen += go_add_group_info_attr(pwdinfo, p2pie + p2pielen);
I am not sure how to properly handle it before, so I just deal with it
lazily to reduce unintentional modification.
I will make it a negative code in next version, but there is a lack of
proper error handlers along the call chain.
>> --- a/drivers/staging/r8188eu/core/rtw_xmit.c
>> +++ b/drivers/staging/r8188eu/core/rtw_xmit.c
>> @@ -180,6 +180,10 @@ s32 _rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter)
>> pxmitpriv->free_xmit_extbuf_cnt = num_xmit_extbuf;
>>
>> rtw_alloc_hwxmits(padapter);
>> + if (!pxmitpriv->hwxmits) {
>> + res = _FAIL;
>> + goto exit;
>> + }
>
> You just leaked memory resources :(
> How did you test this?
Emmm, I do not think this will leak as `pxmitpriv` or `padapter` are all
arguments from the caller. And this is similar to the same function in
drivers\staging\rtl8723bs\core\rtw_xmit.c.
>> @@ -1524,6 +1528,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;
>
> You have to return an error, you can not keep going as if all is well.
Ok, then I will change it to the shape like s32 _rtw_init_xmit_priv()
in drivers\staging\rtl8723bs\core\rtw_xmit.c and return `_FAIL`.
>
> Please always be VERY careful with these types of fixes. Especially if
> you have not tested them.
Thank you for your suggestion, I will try my best ^_^.
Regards,
Xiaoke Wang
