linux-kernel - Re: [PATCH] staging: rtl8712: remove unnecessary alignment of pxmitpriv->pxmitbuf

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] [day] [month] [year] [list]

Date: Mon, 27 May 2024 10:15:39 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Nam Cao <namcao@...utronix.de>
Cc: Larry Finger <Larry.Finger@...inger.net>,
	Florian Schilhabel <florian.c.schilhabel@...glemail.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org,
	Nikita Zhandarovich <n.zhandarovich@...tech.ru>,
	syzbot+83763e624cfec6b462cb@...kaller.appspotmail.com
Subject: Re: [PATCH] staging: rtl8712: remove unnecessary alignment of
 pxmitpriv->pxmitbuf

On Mon, May 27, 2024 at 10:06:22AM +0300, Dan Carpenter wrote:
> On Sat, May 25, 2024 at 09:32:29AM +0200, Nam Cao wrote:
> > This driver wants pxmitpriv->pxmitbuf to be 4-byte aligned. This is ensured
> > by allocating 4 more bytes than required with kmalloc(), then do the
> > p = p + 4 - (p & 3) trick to make sure the pointer is 4-byte aligned.
> > 
> > This is unnecessary. Pointers from kmalloc() are already at least
> > 8-byte-aligned.
> > 
> > Remove this alignment trick to simplify the code, and also to stop wasting
> > 4 extra bytes of dynamic memory allocator.
> > 
> > This also gets rid of a (false) warning from kmemleak. This 4-byte-aligned
> > buffer is used to store pointers from kmalloc(). For 64-bit platforms,
> > pointer size is 8 bytes and kmemleak only scans for pointers in 8-byte
> > blocks, thus it misses the pointers stored in this 4-byte-aligned buffer
> > and thinks that these pointers have been leaked. This is just a false
> > warning, not a real problem. But still, it would be nice to get rid of
> > these warnings.
> 
> Are you sure it's a false positive?  I've always wondered what happens
> when you do:
> 
> 	p = kmalloc();
> 	kfree((char *)p + 4);

Ah, never mind, it actually frees p.

It took me a while to figure out that it's actually pointers stored in
the buffer which are reported as leaks and not the buffer itself.  This
was explained quite well in the commit message but I just misunderstood.

regards,
dan carpenter