[linux-cifs-client] [PATCH] [CIFS] Prevent OOPs when mounting
with remote prefixpath
Jeff Layton
jlayton at redhat.com
Wed Feb 4 02:13:07 GMT 2009
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>
More information about the linux-cifs-client
mailing list