[PATCH-V2] [fs/9p] Fix NULL point deref in v9fs_dir_release

[PATCH-V2] [fs/9p] Fix NULL point deref in v9fs_dir_release

[Venkateswararao Jujjuri (JV)]

changes from v1:
Now checking fid instead of filp.

There are situations in VFS where we endup calling v9fs_dir_release() before
even we instantiate the fid in filp. Hence the check.

Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
---
 fs/9p/vfs_dir.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/fs/9p/vfs_dir.c b/fs/9p/vfs_dir.c
index 16c8a2a..5b01842 100644
--- a/fs/9p/vfs_dir.c
+++ b/fs/9p/vfs_dir.c
@@ -291,6 +291,8 @@ int v9fs_dir_release(struct inode *inode, struct file *filp)
 	struct p9_fid *fid;
 
 	fid = filp->private_data;
+	if (!fid)
+		return 0;
 	P9_DPRINTK(P9_DEBUG_VFS,
 			"inode: %p filp: %p fid: %d\n", inode, filp, fid->fid);
 	filemap_write_and_wait(inode->i_mapping);
-- 
1.6.5.2

Re: [V9fs-developer] [PATCH-V2] [fs/9p] Fix NULL point deref in v9fs_dir_release

[Eric Van Hensbergen]

On Sun, Aug 22, 2010 at 9:08 PM, Venkateswararao Jujjuri (JV)
<jvrao@linux.vnet.ibm.com> wrote:
> changes from v1:
> Now checking fid instead of filp.
>
> There are situations in VFS where we endup calling v9fs_dir_release() before
> even we instantiate the fid in filp. Hence the check.
>
> Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
> ---
>  fs/9p/vfs_dir.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/fs/9p/vfs_dir.c b/fs/9p/vfs_dir.c
> index 16c8a2a..5b01842 100644
> --- a/fs/9p/vfs_dir.c
> +++ b/fs/9p/vfs_dir.c
> @@ -291,6 +291,8 @@ int v9fs_dir_release(struct inode *inode, struct file *filp)
>        struct p9_fid *fid;
>
>        fid = filp->private_data;
> +       if (!fid)
> +               return 0;
>        P9_DPRINTK(P9_DEBUG_VFS,
>                        "inode: %p filp: %p fid: %d\n", inode, filp, fid->fid);
>        filemap_write_and_wait(inode->i_mapping);

It's probably a safe bet that if don't have a fid then
filemap_write_and_wait either won't have anything to write or won't be
able to.  Still, I'm a bit uncomfortable as if we lost the fid for
some reason and it did have something to write it would be treated as
a silent error.  I think it would probably be safer to remove the
fid->fid from the debug print (or make it conditional on fid being
non-null) and then change the clunk to be conditional on the presence
of a fid.

          -eric
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html