linux-kernel - Re: [PATCH v4 1/4] staging: rtl8723bs: replace rtw

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aYILnssCFT8ZPIvp@smile.fi.intel.com>
Date: Tue, 3 Feb 2026 16:52:14 +0200
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Minu Jin <s9430939@...er.com>
Cc: gregkh@...uxfoundation.org, dan.carpenter@...aro.org,
	trohan2000@...il.com, andy@...nel.org,
	linux-staging@...ts.linux.dev, straube.linux@...il.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 1/4] staging: rtl8723bs: replace rtw_malloc() with
 kmalloc()

On Sun, Feb 01, 2026 at 04:29:58AM +0900, Minu Jin wrote:
> Remove wrapper function _rtw_malloc() and macro rtw_malloc().
> Replace all rtw_malloc with kmalloc.

rtw_malloc()
kmalloc()

> All call sites are reviewed to select GFP_KERNEL or GFP_ATOMIC.
> 
> 1. GFP_KERNEL:
>    Used in paths that are executed in process context and are allowed to sleep.
> 
>    - Driver initialization and probe paths.
>    - Workqueue callbacks and cfg80211 configuration callbacks.
> 
> 2. GFP_ATOMIC:
>     Used in paths that must not sleep because they operate in atomic contexts.
> 
>    - Interrupt handlers and SoftIRQ contexts.
>    - Functions called while holding spinlocks.
>    - Low-level I/O operations (SDIO) (eg, sdio_read32())

> Replace kmalloc()/memcpy() with kmemdup() where possible.

This one probably better to have in a separate change (obviously before
this one).

> Replace sizeof(struct val) with sizeof(*ptr).
> Remove blank line after kmalloc().

...

> -	if (remainder_ielen > 0) {
> -		pbackup_remainder_ie = rtw_malloc(remainder_ielen);
> -		if (pbackup_remainder_ie && premainder_ie)
> -			memcpy(pbackup_remainder_ie, premainder_ie, remainder_ielen);
> +	if (remainder_ielen > 0 && premainder_ie) {
> +		pbackup_remainder_ie = kmemdup(premainder_ie, remainder_ielen, GFP_ATOMIC);
>  	}

No {} and it will be better to check pointer followed up with length check:

	if (premainder_ie && remainder_ielen)
		pbackup_remainder_ie = kmemdup(premainder_ie, remainder_ielen, GFP_ATOMIC);


...

> -	if (remainder_ielen > 0) {
> -		pbackup_remainder_ie = rtw_malloc(remainder_ielen);
> -		if (pbackup_remainder_ie)
> -			memcpy(pbackup_remainder_ie, premainder_ie, remainder_ielen);
> +	if (remainder_ielen > 0 && premainder_ie) {
> +		pbackup_remainder_ie = kmemdup(premainder_ie, remainder_ielen, GFP_ATOMIC);
>  	}

Ditto.

...

So, the above two should go before this patch.

...

> -		tmpbuf = rtw_malloc(n);
> +		tmpbuf = kmalloc(n, GFP_ATOMIC);
>  		if (!tmpbuf)
>  			return -1;

Side note, these '-1':s probably should be converted to '-ENOMEM':s.

...

> -			pmlmepriv->wps_probe_req_ie = rtw_malloc(wps_ielen);
> +			pmlmepriv->wps_probe_req_ie = kmemdup(wps_ie, wps_ielen, GFP_KERNEL);
>  			if (!pmlmepriv->wps_probe_req_ie)
>  				return -EINVAL;
>  
> -			memcpy(pmlmepriv->wps_probe_req_ie, wps_ie, wps_ielen);
>  			pmlmepriv->wps_probe_req_ie_len = wps_ielen;
>  		}

Move this to the 'kmemdup()' conversion patch (as mentioned above).


...

>  void *_rtw_zmalloc(u32 sz)
>  {
> -	void *pbuf = _rtw_malloc(sz);
> -
> +	void *pbuf = kmalloc(sz, in_interrupt() ? GFP_ATOMIC : GFP_KERNEL);

No, this has to be

	void *pbuf;

	pbuf = kmalloc(sz, in_interrupt() ? GFP_ATOMIC : GFP_KERNEL);

>  	if (pbuf)
>  		memset(pbuf, 0, sz);

So, why this simply can't be moved to kzalloc()?

...

>  	/* duplicate src */
> -	dup = rtw_malloc(src_len);
> +	dup = kmalloc(src_len, GFP_ATOMIC);
>  	if (dup) {
>  		dup_len = src_len;
>  		memcpy(dup, src, dup_len);

Obviously this is candidate for kmemdup().

...

>  	struct rtw_cbuf *cbuf;
>  
> -	cbuf = rtw_malloc(struct_size(cbuf, bufs, size));
> -
> +	cbuf = kmalloc(struct_size(cbuf, bufs, size), GFP_ATOMIC);
>  	if (cbuf) {

>  		cbuf->write = 0;
>  		cbuf->read = 0;

Maybe you want kzalloc() to begin with?

-- 
With Best Regards,
Andy Shevchenko