All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@linaro.org>
To: Trond Myklebust <trondmy@kernel.org>
Cc: liubaolin <liubaolin12138@163.com>,
	anna@kernel.org, linux-nfs@vger.kernel.org,
	linux-kernel@vger.kernel.org, Baolin Liu <liubaolin@kylinos.cn>
Subject: Re: [PATCH v1] NFS: Fix possible NULL pointer dereference in nfs_inode_remove_request()
Date: Wed, 22 Oct 2025 10:36:50 +0300	[thread overview]
Message-ID: <aPiJkhNJ4dgOlMIj@stanley.mountain> (raw)
In-Reply-To: <aPiJIBTsQit5jyUg@stanley.mountain>

On Wed, Oct 22, 2025 at 10:34:56AM +0300, Dan Carpenter wrote:
> On Tue, Oct 21, 2025 at 11:15:21PM -0400, Trond Myklebust wrote:
> > On Wed, 2025-10-22 at 10:44 +0800, liubaolin wrote:
> > > > Sorry, I didn’t actually see any case where req->wb_head == NULL. 
> > > > I found this through a smatch warning that pointed out a potential
> > > > null pointer dereference. 
> > > > Instead of removing the NULL folio check, I prefer to keep it to
> > > > prevent this potential issue. Checking pointer validity before use
> > > > is a good practice. 
> > > > From a maintenance perspective, we can’t rule out the possibility
> > > > that future changes might introduce a req->wb_head == NULL case, so
> > > > I suggest keeping the NULL folio check.
> > > 
> > 
> > I think you need to look at how smatch works in these situations. It is
> > not looking at the call chain, but is rather looking at how the
> > function is structured.
> > Specifically, as I understand it, smatch looks at whether a test for a
> > NULL pointer exists, and whether it is placed before or after the
> > pointer is dereferenced. So it has nothing to say about whether the
> > check is needed; all it says is that *if* the check is needed, then it
> > should be placed differently.
> > Dan Carpenter, please correct me if my information above is outdated...
> 
> Yes.  That's the gist of it.
> 
> However Smatch can tell that the check is not needed then the warning
> won't be printed.

However IF Smatch...

Gar.

regards,
dan carpenter


      reply	other threads:[~2025-10-22  7:36 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-12  8:39 [PATCH v1] NFS: Fix possible NULL pointer dereference in nfs_inode_remove_request() Baolin Liu
2025-10-12 11:21 ` [PATCH] " Markus Elfring
2025-10-13  4:47 ` [PATCH v1] " Trond Myklebust
2025-10-17  6:57   ` liubaolin
2025-10-17 15:02     ` Trond Myklebust
2025-10-22  2:44       ` liubaolin
2025-10-22  3:15         ` Trond Myklebust
2025-10-22  3:26           ` Trond Myklebust
2025-10-22  7:34           ` Dan Carpenter
2025-10-22  7:36             ` Dan Carpenter [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aPiJkhNJ4dgOlMIj@stanley.mountain \
    --to=dan.carpenter@linaro.org \
    --cc=anna@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=liubaolin12138@163.com \
    --cc=liubaolin@kylinos.cn \
    --cc=trondmy@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.