linux-kernel - Re: [PATCH] smb/client: avoid possible NULL dereference in cifs_free

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: <a08c6b03-6d23-4711-a891-14b0250b90be@stanley.mountain>
Date: Fri, 9 Aug 2024 18:11:11 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Su Hui <suhui@...china.com>, David Howells <dhowells@...hat.com>
Cc: sfrench@...ba.org, pc@...guebit.com, ronniesahlberg@...il.com,
	sprasad@...rosoft.com, tom@...pey.com, bharathsm@...rosoft.com,
	nathan@...nel.org, ndesaulniers@...gle.com, morbo@...gle.com,
	justinstitt@...gle.com, linux-cifs@...r.kernel.org,
	samba-technical@...ts.samba.org, linux-kernel@...r.kernel.org,
	llvm@...ts.linux.dev, kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] smb/client: avoid possible NULL dereference in
 cifs_free_subrequest()

On Fri, Aug 09, 2024 at 06:00:26PM +0300, Dan Carpenter wrote:
> On Thu, Aug 08, 2024 at 08:23:32PM +0800, Su Hui wrote:
> > Clang static checker (scan-build) warning:
> > 	cifsglob.h:line 890, column 3
> > 	Access to field 'ops' results in a dereference of a null pointer.
> > 
> > Commit 519be989717c ("cifs: Add a tracepoint to track credits involved in
> > R/W requests") adds a check for 'rdata->server', and let clang throw this
> > warning about NULL dereference.
> > 
> > When 'rdata->credits.value != 0 && rdata->server == NULL' happens,
> > add_credits_and_wake_if() will call rdata->server->ops->add_credits().
> > This will cause NULL dereference problem. Add a check for 'rdata->server'
> > to avoid NULL dereference.
> > 
> > Signed-off-by: Su Hui <suhui@...china.com>
> 
> Needs a Fixes tag.
> 
> Also when you add a Fixes tag, then get_maintainer will add the David Howells
> automatically.  I've added him manually.
> 

Actually, David should have been CC'd but the fixes tag wouldn't have pointed
to his patch.

This is an inconsistent NULL checking warning.  It's not clear to me if the NULL
checks should be removed or more added.  If David were trying to fix a NULL
pointer dereference and accidentally left one unchecked dereference out then the
Fixes tag would point to his patch.  Since David was doing something unrelated
then we don't point to his patch.  Instead it would point to the first patch to
introduce the dereference.

regards,
dan carpenter