2021-02-02 05:50:11

by Dan Carpenter

[permalink] [raw]
Subject: [bug report] NFS: Convert to the netfs API and nfs_readpage to use netfs_readpage

Hello Dave Wysochanski,

This is a semi-automatic email about new static checker warnings.

The patch bc6d7b12e4ea: "NFS: Convert to the netfs API and
nfs_readpage to use netfs_readpage" from Nov 14, 2020, leads to the
following Smatch complaint:

fs/nfs/read.c:365 nfs_readpage()
error: we previously assumed 'file' could be null (see line 356)

fs/nfs/read.c
355
356 if (file == NULL) {
^^^^^^^^^^^^
"file" is NULL here

357 ret = -EBADF;
358 desc.ctx = nfs_find_open_context(inode, NULL, FMODE_READ);
359 if (desc.ctx == NULL)
360 goto out_unlock;
361 } else
362 desc.ctx = get_nfs_open_context(nfs_file_open_context(file));
363
364 if (!IS_SYNC(inode)) {
365 ret = nfs_readpage_from_fscache(file, page, &desc);
^^^^
Unchecked dereference inside function call.

366 if (ret == 0)
367 goto out;

regards,
dan carpenter


2021-02-02 12:26:26

by David Wysochanski

[permalink] [raw]
Subject: Re: [bug report] NFS: Convert to the netfs API and nfs_readpage to use netfs_readpage

On Tue, Feb 2, 2021 at 12:48 AM Dan Carpenter <[email protected]> wrote:
>
> Hello Dave Wysochanski,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch bc6d7b12e4ea: "NFS: Convert to the netfs API and
> nfs_readpage to use netfs_readpage" from Nov 14, 2020, leads to the
> following Smatch complaint:
>
> fs/nfs/read.c:365 nfs_readpage()
> error: we previously assumed 'file' could be null (see line 356)
>
> fs/nfs/read.c
> 355
> 356 if (file == NULL) {
> ^^^^^^^^^^^^
> "file" is NULL here
>
> 357 ret = -EBADF;
> 358 desc.ctx = nfs_find_open_context(inode, NULL, FMODE_READ);
> 359 if (desc.ctx == NULL)
> 360 goto out_unlock;
> 361 } else
> 362 desc.ctx = get_nfs_open_context(nfs_file_open_context(file));
> 363
> 364 if (!IS_SYNC(inode)) {
> 365 ret = nfs_readpage_from_fscache(file, page, &desc);
> ^^^^
> Unchecked dereference inside function call.
>

Thanks for flagging this.

I confess I don't understand why we could get file == NULL in
nfs_readpage() but I'm looking into it.


> 366 if (ret == 0)
> 367 goto out;
>
> regards,
> dan carpenter
>