linux-kernel - Re: [PATCH 5/5] wifi: mwifiex: use vmalloc

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: <aJtQS-Tpzw90Hqtl@stanley.mountain>
Date: Tue, 12 Aug 2025 17:31:39 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Qianfeng Rong <rongqianfeng@...o.com>
Cc: Brian Norris <briannorris@...omium.org>,
	Francesco Dolcini <francesco@...cini.it>,
	Johannes Berg <johannes.berg@...el.com>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Kalle Valo <kvalo@...nel.org>,
	Aditya Kumar Singh <quic_adisi@...cinc.com>,
	Rameshkumar Sundaram <quic_ramess@...cinc.com>,
	Roopni Devanathan <quic_rdevanat@...cinc.com>,
	"open list:MARVELL MWIFIEX WIRELESS DRIVER" <linux-wireless@...r.kernel.org>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 5/5] wifi: mwifiex: use vmalloc_array() to simplify code

On Tue, Aug 12, 2025 at 10:13:43PM +0800, Qianfeng Rong wrote:
> 
> 在 2025/8/12 21:48, Dan Carpenter 写道:
> > On Tue, Aug 12, 2025 at 09:32:18PM +0800, Qianfeng Rong wrote:
> > > Remove array_size() calls and replace vmalloc() with vmalloc_array() to
> > > simplify the code.
> > > 
> > > Signed-off-by: Qianfeng Rong <rongqianfeng@...o.com>
> > > ---
> > >   drivers/net/wireless/marvell/mwifiex/cfg80211.c | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> > > index 3498743d5ec0..fb4183ff02a9 100644
> > > --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> > > +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> > > @@ -4673,8 +4673,8 @@ int mwifiex_init_channel_scan_gap(struct mwifiex_adapter *adapter)
> > >   	 * additional active scan request for hidden SSIDs on passive channels.
> > >   	 */
> > >   	adapter->num_in_chan_stats = 2 * (n_channels_bg + n_channels_a);
> > > -	adapter->chan_stats = vmalloc(array_size(sizeof(*adapter->chan_stats),
> > > -						 adapter->num_in_chan_stats));
> > > +	adapter->chan_stats = vmalloc_array(adapter->num_in_chan_stats,
> > > +					    sizeof(*adapter->chan_stats));
> > n_channels_bg is 14
> > n_channels_a is either 0 or 31 depending on if we're using BAND_A.
> > sizeof(*adapter->chan_stats) is 10.
> > 
> > So we're either allocating 280 or 900 bytes, which is quite small.  We
> > should just use kmalloc_array() instead of vmalloc_array().
> Switching to kmalloc_array() requires changing vfree() to kfree(),
> but I couldn't locate the memory release code.

It's not hard to locate with a bit of looking.  ;)

>  This modification
> likely requires deep familiarity with the codebase.  Also, for
> variable-sized allocations, kvmalloc_array() is preferable, but
> requires kvfree() for proper memory release.

Don't use kvmalloc_array().  The rules are:
1) small amounts of memory: kmalloc()
2) possibly large ammounts of memory up to 2GB: kvmalloc()
3) definitely large ammounts of memory or larger than 2GB: vmalloc()

There are also places where the memory needs to be contiguous and for
those situations kvmalloc() and vmalloc() can't be used.

Here we're allocating less than a PAGE so the appropriate thing is
kmalloc_array().

I'm looking at this some more and these statistics are updated in
mwifiex_update_chan_statistics() and not necessarily all of them
are updated.  But they're used in mwifiex_cfg80211_dump_survey()
when potentially the memory is still uninitialized.

I really think we should zero this memory, so lets allocate it
with kcalloc().

regards,
dan carpenter