Received-SPF: pass (google.com: domain of linux-kernel+bounces-190123-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Mon, 27 May 2024 10:15:39 +0300
From: Dan Carpenter <dan.carpenter@linaro.org>
To: Nam Cao <namcao@linutronix.de>
Cc: Larry Finger <Larry.Finger@lwfinger.net>,
	Florian Schilhabel <florian.c.schilhabel@googlemail.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
	Nikita Zhandarovich <n.zhandarovich@fintech.ru>,
	syzbot+83763e624cfec6b462cb@syzkaller.appspotmail.com
Subject: Re: [PATCH] staging: rtl8712: remove unnecessary alignment of
 pxmitpriv->pxmitbuf
Message-ID: <839d7d29-84ee-4da3-8024-67b79d581780@moroto.mountain>
References: <20240525073229.4144612-1-namcao@linutronix.de>
 <92412c38-8fb0-4a81-9205-7735ae21459c@moroto.mountain>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <92412c38-8fb0-4a81-9205-7735ae21459c@moroto.mountain>

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