* [PATCH 2/4] FS: cifs, remove unneeded NULL tests
@ 2010-10-27 11:50 Jiri Slaby
2010-10-27 16:13 ` Jeff Layton
0 siblings, 1 reply; 3+ messages in thread
From: Jiri Slaby @ 2010-10-27 11:50 UTC (permalink / raw)
To: sfrench; +Cc: linux-kernel, jirislaby, linux-cifs
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@suse.cz>
Cc: Steve French <sfrench@samba.org>
Cc: linux-cifs@vger.kernel.org
---
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) {
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);*/
}
--
1.7.3.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 2/4] FS: cifs, remove unneeded NULL tests
2010-10-27 11:50 [PATCH 2/4] FS: cifs, remove unneeded NULL tests Jiri Slaby
@ 2010-10-27 16:13 ` Jeff Layton
2010-10-27 16:14 ` Christoph Hellwig
0 siblings, 1 reply; 3+ messages in thread
From: Jeff Layton @ 2010-10-27 16:13 UTC (permalink / raw)
To: Jiri Slaby; +Cc: sfrench, linux-kernel, jirislaby, linux-cifs
On Wed, 27 Oct 2010 13:50:20 +0200
Jiri Slaby <jslaby@suse.cz> 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@suse.cz>
> Cc: Steve French <sfrench@samba.org>
> Cc: linux-cifs@vger.kernel.org
> ---
> 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@redhat.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 2/4] FS: cifs, remove unneeded NULL tests
2010-10-27 16:13 ` Jeff Layton
@ 2010-10-27 16:14 ` Christoph Hellwig
0 siblings, 0 replies; 3+ messages in thread
From: Christoph Hellwig @ 2010-10-27 16:14 UTC (permalink / raw)
To: Jeff Layton; +Cc: Jiri Slaby, sfrench, linux-kernel, jirislaby, linux-cifs
On Wed, Oct 27, 2010 at 12:13:38PM -0400, Jeff Layton wrote:
> > 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?
Indeed, it's impossible.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-10-27 16:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-27 11:50 [PATCH 2/4] FS: cifs, remove unneeded NULL tests Jiri Slaby
2010-10-27 16:13 ` Jeff Layton
2010-10-27 16:14 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®