[linux-cifs-client] [PATCH] [CIFS] Prevent OOPs when mounting with remote prefixpath

Igor Mammedov niallain at gmail.com
Wed Feb 4 10:35:16 GMT 2009


Jeff Layton wrote:
> On Tue, 03 Feb 2009 13:22:30 +0300
> Igor Mammedov <niallain at gmail.com> wrote:
> 
>> From: Igor Mammedov <niallain at gmail.com>
>> To: Steve French <smfrench at gmail.com>
>> Cc: "linux-cifs-client at lists.samba.org" <linux-cifs-client at lists.samba.org>
>> Subject: [linux-cifs-client] [PATCH] [CIFS] Prevent OOPs when mounting with remote prefixpath
>> Date: Tue, 03 Feb 2009 13:22:30 +0300
>> Sender: linux-cifs-client-bounces+jlayton=poochiereds.net at lists.samba.org
>> User-Agent: Thunderbird 2.0.0.19 (X11/20081209)
>>
>> Managed to single out the part of  DFS root support from a big patch
>> with a little modification.
>> It will report error and fail to mount if perfixpath is on remote server.
>> As well it will not scare people off with a error 
>> 'kernel BUG at fs/cifs/cifs_dfs_ref.c:274!'.
>>
>>
>> ---------
>> Fixes OOPs with message 'kernel BUG at fs/cifs/cifs_dfs_ref.c:274!'.
>> Check if prefixpath in accessible while we are still in cifs_mount
>> and fail with reporting a error if we can't access prefixpath
>> (i.e. if prefixpath is located on another server)
>>
>>
>> Best regards,
>>
>> -------------------------
>> Igor Mammedov,
>> niallain "at" gmail.com
>>
>>
>>
>>
>>
>> >From c61a715cbeee404a1bcc2788090b4d453eb43b8e Mon Sep 17 00:00:00 2001  
>> From: Igor Mammedov <niallain at gmail.com>
>> Date: Tue, 3 Feb 2009 12:43:56 +0300
>> Subject: [PATCH] [CIFS] Prevent OOPs when mounting with remote prefixpath.
>>  	Fix OOPs with message 'kernel BUG at fs/cifs/cifs_dfs_ref.c:274!'.
>>  	Check if prefixpath in accesible while we are still in cifs_mount
>>  	and fail with reporting a error if we can't access prefixpath
>>          (i.e. if prefixpath is located on another server)
>>
>> Signed-off-by: Igor Mammedov <niallain at gmail.com>
>> ---
>>  fs/cifs/cifsproto.h |    1 +
>>  fs/cifs/connect.c   |   30 ++++++++++++++++++++++++++++++
>>  fs/cifs/inode.c     |    2 +-
>>  3 files changed, 32 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
>> index 382ba62..22ff09c 100644
>> --- a/fs/cifs/cifsproto.h
>> +++ b/fs/cifs/cifsproto.h
>> @@ -42,6 +42,7 @@ extern void _FreeXid(unsigned int);
>>  #define GetXid() (int)_GetXid(); cFYI(1,("CIFS VFS: in %s as Xid: %d with uid: %d",__func__, xid,current_fsuid()));
>>  #define FreeXid(curr_xid) {_FreeXid(curr_xid); cFYI(1,("CIFS VFS: leaving %s (xid = %d) rc = %d",__func__,curr_xid,(int)rc));}
>>  extern char *build_path_from_dentry(struct dentry *);
>> +extern char *build_path_to_root(struct cifs_sb_info *cifs_sb);
>>  extern char *build_wildcard_path_from_dentry(struct dentry *direntry);
>>  /* extern void renew_parental_timestamps(struct dentry *direntry);*/
>>  extern int SendReceive(const unsigned int /* xid */ , struct cifsSesInfo *,
>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> index 2209be9..dcffaa9 100644
>> --- a/fs/cifs/connect.c
>> +++ b/fs/cifs/connect.c
>> @@ -2181,6 +2181,24 @@ static void setup_cifs_sb(struct smb_vol *pvolume_info,
>>  }
>>  
>>  int
>> +is_path_accessible(int xid, struct cifsTconInfo *tcon,
>> +		struct cifs_sb_info *cifs_sb,  const char *full_path)
>> +{
>> +	int rc;
>> +	FILE_ALL_INFO *pfindData;
>> +	pfindData = kmalloc(sizeof(FILE_ALL_INFO), GFP_KERNEL);
>> +	if (pfindData == NULL)
>> +		return -ENOMEM;
>> +
>> +	rc = CIFSSMBQPathInfo(xid, tcon, full_path, pfindData,
>> +			0 /* not legacy */,
>> +			cifs_sb->local_nls, cifs_sb->mnt_cifs_flags &
>> +			CIFS_MOUNT_MAP_SPECIAL_CHR);
>> +	kfree(pfindData);
>> +	return rc;
>> +}
>> +
>> +int
>>  cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
>>  	   char *mount_data, const char *devname)
>>  {
>> @@ -2190,6 +2208,7 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
>>  	struct cifsSesInfo *pSesInfo = NULL;
>>  	struct cifsTconInfo *tcon = NULL;
>>  	struct TCP_Server_Info *srvTcp = NULL;
>> +	char *full_path;
>>  
>>  	xid = GetXid();
>>  
>> @@ -2426,6 +2445,17 @@ mount_fail_check:
>>  		cifs_sb->rsize = min(cifs_sb->rsize,
>>  			       (tcon->ses->server->maxBuf - MAX_CIFS_HDR_SIZE));
>>  
>> +
>> +	full_path = build_path_to_root(cifs_sb);
>> +	if (full_path != NULL) {
>> +		rc = is_path_accessible(xid, tcon, cifs_sb, full_path);
>> +		kfree(full_path);
>> +		if (rc) {
>> +			cERROR(1, ("Remote DFS root not supported"));
> 
>                         ^^^ aren't you making an assumption about the error here? What if the allocation in is_path_accessable failed? A more helpful error message would be nice.
> 
>> +			goto mount_fail_check;
>> +		}
>> +	}
>> +
>>  	/* volume_info->password is freed above when existing session found
>>  	(in which case it is not needed anymore) but when new sesion is created
>>  	the password ptr is put in the new session structure (in which case the
>> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
>> index bcf7b51..00c6a3f 100644
>> --- a/fs/cifs/inode.c
>> +++ b/fs/cifs/inode.c
>> @@ -621,7 +621,7 @@ static const struct inode_operations cifs_ipc_inode_ops = {
>>  	.lookup = cifs_lookup,
>>  };
>>  
>> -static char *build_path_to_root(struct cifs_sb_info *cifs_sb)
>> +char *build_path_to_root(struct cifs_sb_info *cifs_sb)
>>  {
>>  	int pplen = cifs_sb->prepathlen;
>>  	int dfsplen;
>> -- 
>> 1.6.0.2
>>
> 
> Other than the error message this seems reasonable. I assume that it's
> been tested?
> 

I've tested it in mine test env.
About 'error message', Any suggestions on what should be there?

-- 

Best regards,

-------------------------
Igor Mammedov,
niallain "at" gmail.com






More information about the linux-cifs-client mailing list