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

[Jeff Layton]

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?

-- 
Jeff Layton <jlayton at redhat.com>