All of lore.kernel.org
 help / color / mirror / Atom feed
From: Trond Myklebust <trondmy@hammerspace.com>
To: Dan Carpenter <error27@gmail.com>
Cc: "oe-kbuild@lists.linux.dev" <oe-kbuild@lists.linux.dev>,
	"lkp@intel.com" <lkp@intel.com>,
	"oe-kbuild-all@lists.linux.dev" <oe-kbuild-all@lists.linux.dev>
Subject: Re: [trondmy-nfs-2.6:testing 8/17] fs/nfs/read.c:358 nfs_read_folio() warn: variable dereferenced before check 'file' (see line 335)
Date: Wed, 11 Jan 2023 14:55:00 +0000	[thread overview]
Message-ID: <4CEE5F8A-9AE9-4235-8B94-77F842331C64@hammerspace.com> (raw)
In-Reply-To: <202301111617.RHJxuFNF-lkp@intel.com>



> On Jan 11, 2023, at 03:54, Dan Carpenter <error27@gmail.com> wrote:
> 
> [You don't often get email from error27@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> tree:   git://git.linux-nfs.org/projects/trondmy/nfs-2.6.git testing
> head:   653d2b50cb73b541465920a748c6b1a1edf240d0
> commit: c87e83b9f933cc75e6c5ce2f99d333713e87cb6f [8/17] NFS: Convert buffered reads to use folios
> config: i386-randconfig-m021
> compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
> 
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <error27@gmail.com>
> 
> smatch warnings:
> fs/nfs/read.c:358 nfs_read_folio() warn: variable dereferenced before check 'file' (see line 335)
> 
> vim +/file +358 fs/nfs/read.c
> 
> 65d023af7f29eb Matthew Wilcox (Oracle  2022-04-29  332) int nfs_read_folio(struct file *file, struct folio *folio)
> ^1da177e4c3f41 Linus Torvalds          2005-04-16  333  {
> 1af7e7f8c12f52 Dave Wysochanski        2021-01-28  334          struct nfs_readdesc desc;
> c87e83b9f933cc Trond Myklebust         2023-01-03 @335          struct inode *inode = file_inode(file);
>                                                                                                 ^^^^
> file dereferenced here.
> 
> 49dee70052b894 Dave Wysochanski        2021-01-28  336          int ret;
> ^1da177e4c3f41 Linus Torvalds          2005-04-16  337
> c87e83b9f933cc Trond Myklebust         2023-01-03  338          trace_nfs_aop_readpage(inode, folio);
> 91d5b47023b608 Chuck Lever             2006-03-20  339          nfs_inc_stats(inode, NFSIOS_VFSREADPAGE);
> 91d5b47023b608 Chuck Lever             2006-03-20  340
> ^1da177e4c3f41 Linus Torvalds          2005-04-16  341          /*
> ^1da177e4c3f41 Linus Torvalds          2005-04-16  342           * Try to flush any pending writes to the file..
> ^1da177e4c3f41 Linus Torvalds          2005-04-16  343           *
> c87e83b9f933cc Trond Myklebust         2023-01-03  344           * NOTE! Because we own the folio lock, there cannot
> ^1da177e4c3f41 Linus Torvalds          2005-04-16  345           * be any new pending writes generated at this point
> c87e83b9f933cc Trond Myklebust         2023-01-03  346           * for this folio (other folios can be written to).
> ^1da177e4c3f41 Linus Torvalds          2005-04-16  347           */
> c87e83b9f933cc Trond Myklebust         2023-01-03  348          ret = nfs_wb_folio(inode, folio);
> 49dee70052b894 Dave Wysochanski        2021-01-28  349          if (ret)
> de05a0cc2a2ae1 Trond Myklebust         2007-05-20  350                  goto out_unlock;
> c87e83b9f933cc Trond Myklebust         2023-01-03  351          if (folio_test_uptodate(folio))
> de05a0cc2a2ae1 Trond Myklebust         2007-05-20  352                  goto out_unlock;
> ^1da177e4c3f41 Linus Torvalds          2005-04-16  353
> 49dee70052b894 Dave Wysochanski        2021-01-28  354          ret = -ESTALE;
> 5f004cf2aa8494 Trond Myklebust         2006-09-14  355          if (NFS_STALE(inode))
> de05a0cc2a2ae1 Trond Myklebust         2007-05-20  356                  goto out_unlock;
> 5f004cf2aa8494 Trond Myklebust         2006-09-14  357
> ^1da177e4c3f41 Linus Torvalds          2005-04-16 @358          if (file == NULL) {
>                                                                    ^^^^^^^^^^^^
> But apparently it can be NULL.
> 
> 49dee70052b894 Dave Wysochanski        2021-01-28  359                  ret = -EBADF;
> 1af7e7f8c12f52 Dave Wysochanski        2021-01-28  360                  desc.ctx = nfs_find_open_context(inode, NULL, FMODE_READ);
> 1af7e7f8c12f52 Dave Wysochanski        2021-01-28  361                  if (desc.ctx == NULL)
> de05a0cc2a2ae1 Trond Myklebust         2007-05-20  362                          goto out_unlock;
> ^1da177e4c3f41 Linus Torvalds          2005-04-16  363          } else
> 1af7e7f8c12f52 Dave Wysochanski        2021-01-28  364                  desc.ctx = get_nfs_open_context(nfs_file_open_context(file));
> 8e0969f0451eaf Trond Myklebust         2006-12-13  365
> ba512c1bc32321 Dave Wysochanski        2021-06-29  366          xchg(&desc.ctx->error, 0);
> 1e83b173b2663b Dave Wysochanski        2021-01-28  367          nfs_pageio_init_read(&desc.pgio, inode, false,
> 1e83b173b2663b Dave Wysochanski        2021-01-28  368                               &nfs_async_read_completion_ops);
> 1e83b173b2663b Dave Wysochanski        2021-01-28  369
> c87e83b9f933cc Trond Myklebust         2023-01-03  370          ret = readpage_async_filler(&desc, folio);
> e0340f16a08d03 Dave Wysochanski        2021-06-29  371          if (ret)
> e0340f16a08d03 Dave Wysochanski        2021-06-29  372                  goto out;
> 1e83b173b2663b Dave Wysochanski        2021-01-28  373
> b42ad64f5f216d Dave Wysochanski        2021-06-24  374          nfs_pageio_complete_read(&desc.pgio);
> 1e83b173b2663b Dave Wysochanski        2021-01-28  375          ret = desc.pgio.pg_error < 0 ? desc.pgio.pg_error : 0;
> 49dee70052b894 Dave Wysochanski        2021-01-28  376          if (!ret) {
> c87e83b9f933cc Trond Myklebust         2023-01-03  377                  ret = folio_wait_locked_killable(folio);
> c87e83b9f933cc Trond Myklebust         2023-01-03  378                  if (!folio_test_uptodate(folio) && !ret)
> 1af7e7f8c12f52 Dave Wysochanski        2021-01-28  379                          ret = xchg(&desc.ctx->error, 0);
> 8f54c7a4babf58 Trond Myklebust         2019-08-15  380          }
> 9a9fc1c03315f1 David Howells           2009-04-03  381  out:
> 1af7e7f8c12f52 Dave Wysochanski        2021-01-28  382          put_nfs_open_context(desc.ctx);
> c87e83b9f933cc Trond Myklebust         2023-01-03  383          trace_nfs_aop_readpage_done(inode, folio, ret);
> 49dee70052b894 Dave Wysochanski        2021-01-28  384          return ret;
> de05a0cc2a2ae1 Trond Myklebust         2007-05-20  385  out_unlock:
> c87e83b9f933cc Trond Myklebust         2023-01-03  386          folio_unlock(folio);
> c87e83b9f933cc Trond Myklebust         2023-01-03  387          trace_nfs_aop_readpage_done(inode, folio, ret);
> 49dee70052b894 Dave Wysochanski        2021-01-28  388          return ret;
> ^1da177e4c3f41 Linus Torvalds          2005-04-16  389  }
> 

I think that’s a false positive. From what I can see after auditing all the callers, ‘file’ is guaranteed to be non-NULL, so the above check should probably have been retired years ago.


_________________________________
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com


  reply	other threads:[~2023-01-11 14:55 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-11  8:35 [trondmy-nfs-2.6:testing 8/17] fs/nfs/read.c:358 nfs_read_folio() warn: variable dereferenced before check 'file' (see line 335) kernel test robot
2023-01-11  8:54 ` Dan Carpenter
2023-01-11 14:55 ` Trond Myklebust [this message]
2023-01-11 18:23   ` Dan Carpenter
2023-01-11 18:45     ` Trond Myklebust

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=4CEE5F8A-9AE9-4235-8B94-77F842331C64@hammerspace.com \
    --to=trondmy@hammerspace.com \
    --cc=error27@gmail.com \
    --cc=lkp@intel.com \
    --cc=oe-kbuild-all@lists.linux.dev \
    --cc=oe-kbuild@lists.linux.dev \
    /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.