Re: [PATCH 2/4] FS: cifs, remove unneeded NULL tests

From: Jeff Layton
Date: Wed Oct 27 2010 - 12:12:46 EST


On Wed, 27 Oct 2010 13:50:20 +0200
Jiri Slaby <jslaby@xxxxxxx> wrote:

> Stanse found that pSMBFile in cifs_ioctl and file->f_path.dentry in
> cifs_user_write are dereferenced prior their test to NULL.
>
> The alternative is not to dereference them before the tests. The patch is
> to point out the problem, you have to decide.
>
> Signed-off-by: Jiri Slaby <jslaby@xxxxxxx>
> Cc: Steve French <sfrench@xxxxxxxxx>
> Cc: linux-cifs@xxxxxxxxxxxxxxx
> ---
> fs/cifs/file.c | 2 +-
> fs/cifs/ioctl.c | 4 ----
> 2 files changed, 1 insertions(+), 5 deletions(-)
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 45af003..db7eaf7 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -1031,7 +1031,7 @@ ssize_t cifs_user_write(struct file *file, const char __user *write_data,
> cifs_stats_bytes_written(pTcon, total_written);
>
> /* since the write may have blocked check these pointers again */
> - if ((file->f_path.dentry) && (file->f_path.dentry->d_inode)) {
> + if (file->f_path.dentry->d_inode) {

^^^^^^
This check is bogus too. An open filp on a negative dentry
isn't possible, right?

> struct inode *inode = file->f_path.dentry->d_inode;
> /* Do not update local mtime - server will set its actual value on write
> * inode->i_ctime = inode->i_mtime =
> diff --git a/fs/cifs/ioctl.c b/fs/cifs/ioctl.c
> index 077bf75..2fa22f2 100644
> --- a/fs/cifs/ioctl.c
> +++ b/fs/cifs/ioctl.c
> @@ -63,8 +63,6 @@ long cifs_ioctl(struct file *filep, unsigned int command, unsigned long arg)
> #ifdef CONFIG_CIFS_POSIX
> case FS_IOC_GETFLAGS:
> if (CIFS_UNIX_EXTATTR_CAP & caps) {
> - if (pSMBFile == NULL)
> - break;
> rc = CIFSGetExtAttr(xid, tcon, pSMBFile->netfid,
> &ExtAttrBits, &ExtAttrMask);
> if (rc == 0)
> @@ -80,8 +78,6 @@ long cifs_ioctl(struct file *filep, unsigned int command, unsigned long arg)
> rc = -EFAULT;
> break;
> }
> - if (pSMBFile == NULL)
> - break;
> /* rc= CIFSGetExtAttr(xid,tcon,pSMBFile->netfid,
> extAttrBits, &ExtAttrMask);*/
> }

Acked-by: Jeff Layton <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/