Patchwork skip posix open if nameidata is null

login
register
mail settings
Submitter Shirish Pargaonkar
Date April 2, 2010, 5:32 p.m.
Message ID <1270229578-14227-1-git-send-email-shirishpargaonkar@gmail.com>
Download mbox | patch
Permalink /patch/49318/
State New
Headers show

Comments

Shirish Pargaonkar - April 2, 2010, 5:32 p.m.
While creating a file on a server which supports unix extensions
such as Samba, if a file is being created which does not supply
nameidata (i.e. nd is null), cifs client can oops when calling
cifs_posix_open.

The simplest solution is, do not open a file using posix semantics
if nameidata parameter is NULL even if server supports posix semantics.

I do not see a way to reach vfsmount (structure) field if nameidata
is not supplied to call posix open (cifs_posix_open).


Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@gmail.com>
Reported-by: Eugene Teo <eugeneteo@kernel.sg>
---
Jeff Layton - April 2, 2010, 10:44 p.m.
On Fri,  2 Apr 2010 12:32:58 -0500
shirishpargaonkar@gmail.com wrote:

> While creating a file on a server which supports unix extensions
> such as Samba, if a file is being created which does not supply
> nameidata (i.e. nd is null), cifs client can oops when calling
> cifs_posix_open.
> 
> The simplest solution is, do not open a file using posix semantics
> if nameidata parameter is NULL even if server supports posix semantics.
> 
> I do not see a way to reach vfsmount (structure) field if nameidata
> is not supplied to call posix open (cifs_posix_open).
> 
> 
> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@gmail.com>
> Reported-by: Eugene Teo <eugeneteo@kernel.sg>
> ---
> 
> diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
> index e9f7ecc..eef8d83 100644
> --- a/fs/cifs/dir.c
> +++ b/fs/cifs/dir.c
> @@ -317,7 +317,7 @@ cifs_create(struct inode *inode, struct dentry *direntry, int mode,
>  	else
>  		oflags = FMODE_READ;
>  
> -	if (tcon->unix_ext && (tcon->ses->capabilities & CAP_UNIX) &&
> +	if (nd && tcon->unix_ext && (tcon->ses->capabilities & CAP_UNIX) &&
>  	    (CIFS_UNIX_POSIX_PATH_OPS_CAP &
>  			le64_to_cpu(tcon->fsUnixInfo.Capability))) {
>  		rc = cifs_posix_open(full_path, &newinode, nd->path.mnt,

There's nothing inherently special about posix opens that prevents them
from working when nd is NULL. Instead of doing this, it would probably
be preferable to have posix opens do what "regular" opens do when nd is
NULL -- close the file before returning from cifs_create.
Shirish Pargaonkar - April 4, 2010, 12:32 p.m.
On Fri, Apr 2, 2010 at 5:44 PM, Jeff Layton <jlayton@samba.org> wrote:
> On Fri,  2 Apr 2010 12:32:58 -0500
> shirishpargaonkar@gmail.com wrote:
>
>> While creating a file on a server which supports unix extensions
>> such as Samba, if a file is being created which does not supply
>> nameidata (i.e. nd is null), cifs client can oops when calling
>> cifs_posix_open.
>>
>> The simplest solution is, do not open a file using posix semantics
>> if nameidata parameter is NULL even if server supports posix semantics.
>>
>> I do not see a way to reach vfsmount (structure) field if nameidata
>> is not supplied to call posix open (cifs_posix_open).
>>
>>
>> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@gmail.com>
>> Reported-by: Eugene Teo <eugeneteo@kernel.sg>
>> ---
>>
>> diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
>> index e9f7ecc..eef8d83 100644
>> --- a/fs/cifs/dir.c
>> +++ b/fs/cifs/dir.c
>> @@ -317,7 +317,7 @@ cifs_create(struct inode *inode, struct dentry *direntry, int mode,
>>       else
>>               oflags = FMODE_READ;
>>
>> -     if (tcon->unix_ext && (tcon->ses->capabilities & CAP_UNIX) &&
>> +     if (nd && tcon->unix_ext && (tcon->ses->capabilities & CAP_UNIX) &&
>>           (CIFS_UNIX_POSIX_PATH_OPS_CAP &
>>                       le64_to_cpu(tcon->fsUnixInfo.Capability))) {
>>               rc = cifs_posix_open(full_path, &newinode, nd->path.mnt,
>
> There's nothing inherently special about posix opens that prevents them
> from working when nd is NULL. Instead of doing this, it would probably
> be preferable to have posix opens do what "regular" opens do when nd is
> NULL -- close the file before returning from cifs_create.
>
> --
> Jeff Layton <jlayton@samba.org>
>

Agree that just because nd is null, caller should not miss out on
posix semantics, but the way code is written
now, there is no way to access super_block within cifs_posix_open if
nd is NULL, will have to modify
cifs_posix_open to expect both vfsmount and super_block when invoked
and handle null nd/vfsmount case
within.

Regards,

Shirish

Patch

diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index e9f7ecc..eef8d83 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -317,7 +317,7 @@  cifs_create(struct inode *inode, struct dentry *direntry, int mode,
 	else
 		oflags = FMODE_READ;
 
-	if (tcon->unix_ext && (tcon->ses->capabilities & CAP_UNIX) &&
+	if (nd && tcon->unix_ext && (tcon->ses->capabilities & CAP_UNIX) &&
 	    (CIFS_UNIX_POSIX_PATH_OPS_CAP &
 			le64_to_cpu(tcon->fsUnixInfo.Capability))) {
 		rc = cifs_posix_open(full_path, &newinode, nd->path.mnt,