| Message ID | 20230329202406.15762-1-ddiss@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cifs: fix DFS traversal oops without CONFIG_CIFS_DFS_UPCALL | expand |
reviewed-by me On Thu, 30 Mar 2023 at 06:23, David Disseldorp <ddiss@suse.de> wrote: > > When compiled with CONFIG_CIFS_DFS_UPCALL disabled, cifs_dfs_d_automount > NULL. cifs.ko logic for mapping CIFS_FATTR_DFS_REFERRAL attributes to > S_AUTOMOUNT and corresponding dentry flags is retained regardless of > CONFIG_CIFS_DFS_UPCALL, leading to a NULL pointer dereference in > VFS follow_automount() when traversing a DFS referral link: > BUG: kernel NULL pointer dereference, address: 0000000000000000 > ... > Call Trace: > <TASK> > __traverse_mounts+0xb5/0x220 > ? cifs_revalidate_mapping+0x65/0xc0 [cifs] > step_into+0x195/0x610 > ? lookup_fast+0xe2/0xf0 > path_lookupat+0x64/0x140 > filename_lookup+0xc2/0x140 > ? __create_object+0x299/0x380 > ? kmem_cache_alloc+0x119/0x220 > ? user_path_at_empty+0x31/0x50 > user_path_at_empty+0x31/0x50 > __x64_sys_chdir+0x2a/0xd0 > ? exit_to_user_mode_prepare+0xca/0x100 > do_syscall_64+0x42/0x90 > entry_SYSCALL_64_after_hwframe+0x72/0xdc > > This fix adds an inline cifs_dfs_d_automount() {return -EREMOTE} handler > when CONFIG_CIFS_DFS_UPCALL is disabled. An alternative would be to > avoid flagging S_AUTOMOUNT, etc. without CONFIG_CIFS_DFS_UPCALL. This > approach was chosen as it provides more control over the error path. > > Signed-off-by: David Disseldorp <ddiss@suse.de> > --- > fs/cifs/cifsfs.h | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h > index 71fe0a0a7992..415176b2cf32 100644 > --- a/fs/cifs/cifsfs.h > +++ b/fs/cifs/cifsfs.h > @@ -124,7 +124,10 @@ extern const struct dentry_operations cifs_ci_dentry_ops; > #ifdef CONFIG_CIFS_DFS_UPCALL > extern struct vfsmount *cifs_dfs_d_automount(struct path *path); > #else > -#define cifs_dfs_d_automount NULL > +static inline struct vfsmount *cifs_dfs_d_automount(struct path *path) > +{ > + return ERR_PTR(-EREMOTE); > +} > #endif > > /* Functions related to symlinks */ > -- > 2.35.3 >
merged into cifs-2.6.git for-next and added Paulo's Reviewed-by On Wed, Mar 29, 2023 at 3:23 PM David Disseldorp <ddiss@suse.de> wrote: > > When compiled with CONFIG_CIFS_DFS_UPCALL disabled, cifs_dfs_d_automount > NULL. cifs.ko logic for mapping CIFS_FATTR_DFS_REFERRAL attributes to > S_AUTOMOUNT and corresponding dentry flags is retained regardless of > CONFIG_CIFS_DFS_UPCALL, leading to a NULL pointer dereference in > VFS follow_automount() when traversing a DFS referral link: > BUG: kernel NULL pointer dereference, address: 0000000000000000 > ... > Call Trace: > <TASK> > __traverse_mounts+0xb5/0x220 > ? cifs_revalidate_mapping+0x65/0xc0 [cifs] > step_into+0x195/0x610 > ? lookup_fast+0xe2/0xf0 > path_lookupat+0x64/0x140 > filename_lookup+0xc2/0x140 > ? __create_object+0x299/0x380 > ? kmem_cache_alloc+0x119/0x220 > ? user_path_at_empty+0x31/0x50 > user_path_at_empty+0x31/0x50 > __x64_sys_chdir+0x2a/0xd0 > ? exit_to_user_mode_prepare+0xca/0x100 > do_syscall_64+0x42/0x90 > entry_SYSCALL_64_after_hwframe+0x72/0xdc > > This fix adds an inline cifs_dfs_d_automount() {return -EREMOTE} handler > when CONFIG_CIFS_DFS_UPCALL is disabled. An alternative would be to > avoid flagging S_AUTOMOUNT, etc. without CONFIG_CIFS_DFS_UPCALL. This > approach was chosen as it provides more control over the error path. > > Signed-off-by: David Disseldorp <ddiss@suse.de> > --- > fs/cifs/cifsfs.h | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h > index 71fe0a0a7992..415176b2cf32 100644 > --- a/fs/cifs/cifsfs.h > +++ b/fs/cifs/cifsfs.h > @@ -124,7 +124,10 @@ extern const struct dentry_operations cifs_ci_dentry_ops; > #ifdef CONFIG_CIFS_DFS_UPCALL > extern struct vfsmount *cifs_dfs_d_automount(struct path *path); > #else > -#define cifs_dfs_d_automount NULL > +static inline struct vfsmount *cifs_dfs_d_automount(struct path *path) > +{ > + return ERR_PTR(-EREMOTE); > +} > #endif > > /* Functions related to symlinks */ > -- > 2.35.3 >
Hi Steve, On Wed, 29 Mar 2023 18:20:33 -0500, Steve French wrote: > merged into cifs-2.6.git for-next and added Paulo's Reviewed-by Thanks, although I'm only aware of Ronnie's review (in this thread). > On Wed, Mar 29, 2023 at 3:23 PM David Disseldorp <ddiss@suse.de> wrote: > > > When compiled with CONFIG_CIFS_DFS_UPCALL disabled, cifs_dfs_d_automount > > NULL. cifs.ko logic for mapping CIFS_FATTR_DFS_REFERRAL attributes to ^^ If you're fixing the reviewed-by, then please also add the missing "is" between "cifs_dfs_d_automount NULL". Cheers, David
Fixed description (and updated to include Ronnie's RB) On Thu, Mar 30, 2023 at 4:17 PM David Disseldorp <ddiss@suse.de> wrote: > > Hi Steve, > > On Wed, 29 Mar 2023 18:20:33 -0500, Steve French wrote: > > > merged into cifs-2.6.git for-next and added Paulo's Reviewed-by > > Thanks, although I'm only aware of Ronnie's review (in this thread). > > > On Wed, Mar 29, 2023 at 3:23 PM David Disseldorp <ddiss@suse.de> wrote: > > > > > When compiled with CONFIG_CIFS_DFS_UPCALL disabled, cifs_dfs_d_automount > > > NULL. cifs.ko logic for mapping CIFS_FATTR_DFS_REFERRAL attributes to > ^^ > If you're fixing the reviewed-by, then please also add the missing > "is" between "cifs_dfs_d_automount NULL". > > Cheers, David
diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h index 71fe0a0a7992..415176b2cf32 100644 --- a/fs/cifs/cifsfs.h +++ b/fs/cifs/cifsfs.h @@ -124,7 +124,10 @@ extern const struct dentry_operations cifs_ci_dentry_ops; #ifdef CONFIG_CIFS_DFS_UPCALL extern struct vfsmount *cifs_dfs_d_automount(struct path *path); #else -#define cifs_dfs_d_automount NULL +static inline struct vfsmount *cifs_dfs_d_automount(struct path *path) +{ + return ERR_PTR(-EREMOTE); +} #endif /* Functions related to symlinks */
When compiled with CONFIG_CIFS_DFS_UPCALL disabled, cifs_dfs_d_automount NULL. cifs.ko logic for mapping CIFS_FATTR_DFS_REFERRAL attributes to S_AUTOMOUNT and corresponding dentry flags is retained regardless of CONFIG_CIFS_DFS_UPCALL, leading to a NULL pointer dereference in VFS follow_automount() when traversing a DFS referral link: BUG: kernel NULL pointer dereference, address: 0000000000000000 ... Call Trace: <TASK> __traverse_mounts+0xb5/0x220 ? cifs_revalidate_mapping+0x65/0xc0 [cifs] step_into+0x195/0x610 ? lookup_fast+0xe2/0xf0 path_lookupat+0x64/0x140 filename_lookup+0xc2/0x140 ? __create_object+0x299/0x380 ? kmem_cache_alloc+0x119/0x220 ? user_path_at_empty+0x31/0x50 user_path_at_empty+0x31/0x50 __x64_sys_chdir+0x2a/0xd0 ? exit_to_user_mode_prepare+0xca/0x100 do_syscall_64+0x42/0x90 entry_SYSCALL_64_after_hwframe+0x72/0xdc This fix adds an inline cifs_dfs_d_automount() {return -EREMOTE} handler when CONFIG_CIFS_DFS_UPCALL is disabled. An alternative would be to avoid flagging S_AUTOMOUNT, etc. without CONFIG_CIFS_DFS_UPCALL. This approach was chosen as it provides more control over the error path. Signed-off-by: David Disseldorp <ddiss@suse.de> --- fs/cifs/cifsfs.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)