| Message ID | 20210330055957.3684579-2-dkadashev@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | io_uring: add mkdirat support | expand |
On Tue, Mar 30, 2021 at 12:59:56PM +0700, Dmitry Kadashev wrote: > Pass in the struct filename pointers instead of the user string, and > update the three callers to do the same. This is heavily based on > commit dbea8d345177 ("fs: make do_renameat2() take struct filename"). > > This behaves like do_unlinkat() and do_renameat2(). > > Cc: Al Viro <viro@zeniv.linux.org.uk> > Signed-off-by: Dmitry Kadashev <dkadashev@gmail.com> > --- > fs/internal.h | 1 + > fs/namei.c | 25 +++++++++++++++++++------ > 2 files changed, 20 insertions(+), 6 deletions(-) The only thing that is a bit unpleasant here is that this change breaks the consistency between the creation helpers: do_mkdirat() do_symlinkat() do_linkat() do_mknodat() All but of them currently take const char __user *pathname and call user_path_create() with that do_mkdirat() change that's no longer true. One of the major benefits over the recent years in this code is naming and type consistency. And since it's just matter of time until io_uring will also gain support for do_{symlinkat,linkat,mknodat} I would think switching all of them to take a struct filename and then have all do_* helpers call getname() might just be nicer in the long run. Christian
On Tue, Mar 30, 2021 at 2:17 PM Christian Brauner <christian.brauner@ubuntu.com> wrote: > The only thing that is a bit unpleasant here is that this change > breaks the consistency between the creation helpers: > > do_mkdirat() > do_symlinkat() > do_linkat() > do_mknodat() > > All but of them currently take > const char __user *pathname > and call > user_path_create() > with that do_mkdirat() change that's no longer true. One of the major > benefits over the recent years in this code is naming and type consistency. > And since it's just matter of time until io_uring will also gain support > for do_{symlinkat,linkat,mknodat} I would think switching all of them to > take a struct filename > and then have all do_* helpers call getname() might just be nicer in the > long run. OK, I can change the rest of them in the next version if no one objects.
On Tue, Mar 30, 2021 at 2:17 PM Christian Brauner <christian.brauner@ubuntu.com> wrote: > The only thing that is a bit unpleasant here is that this change > breaks the consistency between the creation helpers: > > do_mkdirat() > do_symlinkat() > do_linkat() > do_mknodat() > > All but of them currently take > const char __user *pathname > and call > user_path_create() > with that do_mkdirat() change that's no longer true. One of the major > benefits over the recent years in this code is naming and type consistency. > And since it's just matter of time until io_uring will also gain support > for do_{symlinkat,linkat,mknodat} I would think switching all of them to > take a struct filename > and then have all do_* helpers call getname() might just be nicer in the > long run. So, I've finally got some time to look into this. do_mknodat() and do_symlinkat() are easy. But do_linkat() is more complicated, I could use some hints as to what's the reasonable way to implement the change. The problem is linkat() requires CAP_DAC_READ_SEARCH capability if AT_EMPTY_PATH flag is passed. Right now do_linkat checks the capability before calling getname_flags (essentially). If do_linkat is changed to accept struct filename then there is no bulletproof way to force CAP_DAC_READ_SEARCH presence (e.g. if for whatever reason AT_EMPTY_PATH is not in flags passed to do_linkat). Also, it means that the caller is responsible to process AT_EMPTY_PATH in the first place, which means logic duplication. Any ideas what's the best way to approach this? Thanks.
On Thu, Apr 8, 2021 at 3:45 PM Dmitry Kadashev <dkadashev@gmail.com> wrote: > > On Tue, Mar 30, 2021 at 2:17 PM Christian Brauner > <christian.brauner@ubuntu.com> wrote: > > The only thing that is a bit unpleasant here is that this change > > breaks the consistency between the creation helpers: > > > > do_mkdirat() > > do_symlinkat() > > do_linkat() > > do_mknodat() > > > > All but of them currently take > > const char __user *pathname > > and call > > user_path_create() > > with that do_mkdirat() change that's no longer true. One of the major > > benefits over the recent years in this code is naming and type consistency. > > And since it's just matter of time until io_uring will also gain support > > for do_{symlinkat,linkat,mknodat} I would think switching all of them to > > take a struct filename > > and then have all do_* helpers call getname() might just be nicer in the > > long run. > > So, I've finally got some time to look into this. do_mknodat() and > do_symlinkat() are easy. But do_linkat() is more complicated, I could use some > hints as to what's the reasonable way to implement the change. > > The problem is linkat() requires CAP_DAC_READ_SEARCH capability if AT_EMPTY_PATH > flag is passed. Right now do_linkat checks the capability before calling > getname_flags (essentially). If do_linkat is changed to accept struct filename > then there is no bulletproof way to force CAP_DAC_READ_SEARCH presence (e.g. if > for whatever reason AT_EMPTY_PATH is not in flags passed to do_linkat). Also, it > means that the caller is responsible to process AT_EMPTY_PATH in the first > place, which means logic duplication. > > Any ideas what's the best way to approach this? Ping. If someone can see how we can avoid making do_linkat() callers ensure the process has CAP_DAC_READ_SEARCH capability if AT_EMPTY_PATH was passed then the hints would be really appreciated. The best I could come up with is something like getname_linkat(), which could be used by the do_linkat callers, but this sounds error prone and ugly. Jens, do you want to keep the mkdir change out of 5.13 because of this?
On Thu, Apr 15, 2021 at 02:14:17PM +0700, Dmitry Kadashev wrote: > On Thu, Apr 8, 2021 at 3:45 PM Dmitry Kadashev <dkadashev@gmail.com> wrote: > > > > On Tue, Mar 30, 2021 at 2:17 PM Christian Brauner > > <christian.brauner@ubuntu.com> wrote: > > > The only thing that is a bit unpleasant here is that this change > > > breaks the consistency between the creation helpers: > > > > > > do_mkdirat() > > > do_symlinkat() > > > do_linkat() > > > do_mknodat() > > > > > > All but of them currently take > > > const char __user *pathname > > > and call > > > user_path_create() > > > with that do_mkdirat() change that's no longer true. One of the major > > > benefits over the recent years in this code is naming and type consistency. > > > And since it's just matter of time until io_uring will also gain support > > > for do_{symlinkat,linkat,mknodat} I would think switching all of them to > > > take a struct filename > > > and then have all do_* helpers call getname() might just be nicer in the > > > long run. > > > > So, I've finally got some time to look into this. do_mknodat() and > > do_symlinkat() are easy. But do_linkat() is more complicated, I could use some > > hints as to what's the reasonable way to implement the change. > > > > The problem is linkat() requires CAP_DAC_READ_SEARCH capability if AT_EMPTY_PATH > > flag is passed. Right now do_linkat checks the capability before calling > > getname_flags (essentially). If do_linkat is changed to accept struct filename > > then there is no bulletproof way to force CAP_DAC_READ_SEARCH presence (e.g. if > > for whatever reason AT_EMPTY_PATH is not in flags passed to do_linkat). Also, it > > means that the caller is responsible to process AT_EMPTY_PATH in the first > > place, which means logic duplication. > > > > Any ideas what's the best way to approach this? > > Ping. If someone can see how we can avoid making do_linkat() callers > ensure the process has CAP_DAC_READ_SEARCH capability if AT_EMPTY_PATH > was passed then the hints would be really appreciated. Would something like this help? From 7adeec2fe4a954e4e4b8a158a4d9fe705b82b978 Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brauner@ubuntu.com> Date: Thu, 15 Apr 2021 12:03:42 +0200 Subject: [PATCH] namei: add getname_uflags() There are a couple of places where we already open-code the (flags & AT_EMPTY_PATH) check and io_uring will likely add another one in the future. Let's just add a simple helper getname_uflags() that handles this directly and use it. getname_flags() itself doesn't need access to lookup flags other than LOOKUP_EMPTY so this is basically just a boolean already so be honest about it. Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> --- fs/exec.c | 10 ++-------- fs/fsopen.c | 6 +++--- fs/namei.c | 6 +++--- include/linux/fs.h | 4 ++++ 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index 18594f11c31f..53c633f69f4a 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -2069,10 +2069,7 @@ SYSCALL_DEFINE5(execveat, const char __user *const __user *, envp, int, flags) { - int lookup_flags = (flags & AT_EMPTY_PATH) ? LOOKUP_EMPTY : 0; - - return do_execveat(fd, - getname_flags(filename, lookup_flags, NULL), + return do_execveat(fd, getname_uflags(filename, flags), argv, envp, flags); } @@ -2090,10 +2087,7 @@ COMPAT_SYSCALL_DEFINE5(execveat, int, fd, const compat_uptr_t __user *, envp, int, flags) { - int lookup_flags = (flags & AT_EMPTY_PATH) ? LOOKUP_EMPTY : 0; - - return compat_do_execveat(fd, - getname_flags(filename, lookup_flags, NULL), + return compat_do_execveat(fd, getname_uflags(filename, flags), argv, envp, flags); } #endif diff --git a/fs/fsopen.c b/fs/fsopen.c index 27a890aa493a..00906abaf466 100644 --- a/fs/fsopen.c +++ b/fs/fsopen.c @@ -321,7 +321,7 @@ SYSCALL_DEFINE5(fsconfig, struct fs_context *fc; struct fd f; int ret; - int lookup_flags = 0; + bool lookup_empty = false; struct fs_parameter param = { .type = fs_value_is_undefined, @@ -411,11 +411,11 @@ SYSCALL_DEFINE5(fsconfig, } break; case FSCONFIG_SET_PATH_EMPTY: - lookup_flags = LOOKUP_EMPTY; + lookup_empty = true; fallthrough; case FSCONFIG_SET_PATH: param.type = fs_value_is_filename; - param.name = getname_flags(_value, lookup_flags, NULL); + param.name = getname_flags(_value, lookup_empty, NULL); if (IS_ERR(param.name)) { ret = PTR_ERR(param.name); goto out_key; diff --git a/fs/namei.c b/fs/namei.c index 216f16e74351..7694f6bcd711 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -125,7 +125,7 @@ #define EMBEDDED_NAME_MAX (PATH_MAX - offsetof(struct filename, iname)) struct filename * -getname_flags(const char __user *filename, int flags, int *empty) +getname_flags(const char __user *filename, bool lookup_empty, int *empty) { struct filename *result; char *kname; @@ -191,7 +191,7 @@ getname_flags(const char __user *filename, int flags, int *empty) if (unlikely(!len)) { if (empty) *empty = 1; - if (!(flags & LOOKUP_EMPTY)) { + if (lookup_empty) { putname(result); return ERR_PTR(-ENOENT); } @@ -206,7 +206,7 @@ getname_flags(const char __user *filename, int flags, int *empty) struct filename * getname(const char __user * filename) { - return getname_flags(filename, 0, NULL); + return getname_flags(filename, false, NULL); } struct filename * diff --git a/include/linux/fs.h b/include/linux/fs.h index ec8f3ddf4a6a..6dbd629ece04 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2644,6 +2644,10 @@ static inline struct file *file_clone_open(struct file *file) extern int filp_close(struct file *, fl_owner_t id); extern struct filename *getname_flags(const char __user *, int, int *); +extern struct filename *getname_uflags(const char __user *filename, int uflags) +{ + return getname_flags(filename, (flags & AT_EMPTY_PATH), NULL); +} extern struct filename *getname(const char __user *); extern struct filename *getname_kernel(const char *); extern void putname(struct filename *name);
On Thu, Apr 15, 2021 at 12:08:20PM +0200, Christian Brauner wrote: > On Thu, Apr 15, 2021 at 02:14:17PM +0700, Dmitry Kadashev wrote: > > On Thu, Apr 8, 2021 at 3:45 PM Dmitry Kadashev <dkadashev@gmail.com> wrote: > > > > > > On Tue, Mar 30, 2021 at 2:17 PM Christian Brauner > > > <christian.brauner@ubuntu.com> wrote: > > > > The only thing that is a bit unpleasant here is that this change > > > > breaks the consistency between the creation helpers: > > > > > > > > do_mkdirat() > > > > do_symlinkat() > > > > do_linkat() > > > > do_mknodat() > > > > > > > > All but of them currently take > > > > const char __user *pathname > > > > and call > > > > user_path_create() > > > > with that do_mkdirat() change that's no longer true. One of the major > > > > benefits over the recent years in this code is naming and type consistency. > > > > And since it's just matter of time until io_uring will also gain support > > > > for do_{symlinkat,linkat,mknodat} I would think switching all of them to > > > > take a struct filename > > > > and then have all do_* helpers call getname() might just be nicer in the > > > > long run. > > > > > > So, I've finally got some time to look into this. do_mknodat() and > > > do_symlinkat() are easy. But do_linkat() is more complicated, I could use some > > > hints as to what's the reasonable way to implement the change. > > > > > > The problem is linkat() requires CAP_DAC_READ_SEARCH capability if AT_EMPTY_PATH > > > flag is passed. Right now do_linkat checks the capability before calling > > > getname_flags (essentially). If do_linkat is changed to accept struct filename > > > then there is no bulletproof way to force CAP_DAC_READ_SEARCH presence (e.g. if > > > for whatever reason AT_EMPTY_PATH is not in flags passed to do_linkat). Also, it > > > means that the caller is responsible to process AT_EMPTY_PATH in the first > > > place, which means logic duplication. > > > > > > Any ideas what's the best way to approach this? > > > > Ping. If someone can see how we can avoid making do_linkat() callers > > ensure the process has CAP_DAC_READ_SEARCH capability if AT_EMPTY_PATH > > was passed then the hints would be really appreciated. > > Would something like this help? Because I always forget this: _Completely_ untested. (And sorry for the late reply. Just a lot of mail.) > > From 7adeec2fe4a954e4e4b8a158a4d9fe705b82b978 Mon Sep 17 00:00:00 2001 > From: Christian Brauner <christian.brauner@ubuntu.com> > Date: Thu, 15 Apr 2021 12:03:42 +0200 > Subject: [PATCH] namei: add getname_uflags() > > There are a couple of places where we already open-code the (flags & > AT_EMPTY_PATH) check and io_uring will likely add another one in the future. > Let's just add a simple helper getname_uflags() that handles this directly and > use it. > getname_flags() itself doesn't need access to lookup flags other than > LOOKUP_EMPTY so this is basically just a boolean already so be honest about it. > > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> > --- > fs/exec.c | 10 ++-------- > fs/fsopen.c | 6 +++--- > fs/namei.c | 6 +++--- > include/linux/fs.h | 4 ++++ > 4 files changed, 12 insertions(+), 14 deletions(-) > > diff --git a/fs/exec.c b/fs/exec.c > index 18594f11c31f..53c633f69f4a 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -2069,10 +2069,7 @@ SYSCALL_DEFINE5(execveat, > const char __user *const __user *, envp, > int, flags) > { > - int lookup_flags = (flags & AT_EMPTY_PATH) ? LOOKUP_EMPTY : 0; > - > - return do_execveat(fd, > - getname_flags(filename, lookup_flags, NULL), > + return do_execveat(fd, getname_uflags(filename, flags), > argv, envp, flags); > } > > @@ -2090,10 +2087,7 @@ COMPAT_SYSCALL_DEFINE5(execveat, int, fd, > const compat_uptr_t __user *, envp, > int, flags) > { > - int lookup_flags = (flags & AT_EMPTY_PATH) ? LOOKUP_EMPTY : 0; > - > - return compat_do_execveat(fd, > - getname_flags(filename, lookup_flags, NULL), > + return compat_do_execveat(fd, getname_uflags(filename, flags), > argv, envp, flags); > } > #endif > diff --git a/fs/fsopen.c b/fs/fsopen.c > index 27a890aa493a..00906abaf466 100644 > --- a/fs/fsopen.c > +++ b/fs/fsopen.c > @@ -321,7 +321,7 @@ SYSCALL_DEFINE5(fsconfig, > struct fs_context *fc; > struct fd f; > int ret; > - int lookup_flags = 0; > + bool lookup_empty = false; > > struct fs_parameter param = { > .type = fs_value_is_undefined, > @@ -411,11 +411,11 @@ SYSCALL_DEFINE5(fsconfig, > } > break; > case FSCONFIG_SET_PATH_EMPTY: > - lookup_flags = LOOKUP_EMPTY; > + lookup_empty = true; > fallthrough; > case FSCONFIG_SET_PATH: > param.type = fs_value_is_filename; > - param.name = getname_flags(_value, lookup_flags, NULL); > + param.name = getname_flags(_value, lookup_empty, NULL); > if (IS_ERR(param.name)) { > ret = PTR_ERR(param.name); > goto out_key; > diff --git a/fs/namei.c b/fs/namei.c > index 216f16e74351..7694f6bcd711 100644 > --- a/fs/namei.c > +++ b/fs/namei.c > @@ -125,7 +125,7 @@ > #define EMBEDDED_NAME_MAX (PATH_MAX - offsetof(struct filename, iname)) > > struct filename * > -getname_flags(const char __user *filename, int flags, int *empty) > +getname_flags(const char __user *filename, bool lookup_empty, int *empty) > { > struct filename *result; > char *kname; > @@ -191,7 +191,7 @@ getname_flags(const char __user *filename, int flags, int *empty) > if (unlikely(!len)) { > if (empty) > *empty = 1; > - if (!(flags & LOOKUP_EMPTY)) { > + if (lookup_empty) { > putname(result); > return ERR_PTR(-ENOENT); > } > @@ -206,7 +206,7 @@ getname_flags(const char __user *filename, int flags, int *empty) > struct filename * > getname(const char __user * filename) > { > - return getname_flags(filename, 0, NULL); > + return getname_flags(filename, false, NULL); > } > > struct filename * > diff --git a/include/linux/fs.h b/include/linux/fs.h > index ec8f3ddf4a6a..6dbd629ece04 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -2644,6 +2644,10 @@ static inline struct file *file_clone_open(struct file *file) > extern int filp_close(struct file *, fl_owner_t id); > > extern struct filename *getname_flags(const char __user *, int, int *); > +extern struct filename *getname_uflags(const char __user *filename, int uflags) > +{ > + return getname_flags(filename, (flags & AT_EMPTY_PATH), NULL); > +} > extern struct filename *getname(const char __user *); > extern struct filename *getname_kernel(const char *); > extern void putname(struct filename *name); > -- > 2.27.0 >
On Thu, Apr 15, 2021 at 5:09 PM Christian Brauner <christian.brauner@ubuntu.com> wrote: > > On Thu, Apr 15, 2021 at 12:08:20PM +0200, Christian Brauner wrote: > > Would something like this help? Thanks for the reply, Christian! But it's not the AT_EMPTY_PATH / LOOKUP_EMPTY part that is tricky, it's the fact that do_linkat() allows AT_EMPTY_PATH only if the process has CAP_DAC_READ_SEARCH capability. But AT_EMPTY_PATH is processed during getname(), so if do_linkat() accepts struct filename* then there is no bullet-proof way to force the capability. We could do something like this: do_linkat(oldfd, getname_uflags(oldname, flags), newfd, getname(newname), flags); I.e. call getname_uflags() without checking the capability and rely on the fact that do_linkat() will do the checking. But this is fragile if somehow someone passes different flags to getname_uflags and do_linkat. And there is no way (that I know of) for do_linkat to actually check that AT_EMPTY_PATH was not used if it gets struct filename. Or am I creating extra problems and the thing above is OK? -- Dmitry Kadashev
On Thu, Apr 15, 2021 at 05:41:07PM +0700, Dmitry Kadashev wrote: > On Thu, Apr 15, 2021 at 5:09 PM Christian Brauner > <christian.brauner@ubuntu.com> wrote: > > > > On Thu, Apr 15, 2021 at 12:08:20PM +0200, Christian Brauner wrote: > > > Would something like this help? > > Thanks for the reply, Christian! > > But it's not the AT_EMPTY_PATH / LOOKUP_EMPTY part that is tricky, it's > the fact that do_linkat() allows AT_EMPTY_PATH only if the process has > CAP_DAC_READ_SEARCH capability. But AT_EMPTY_PATH is processed during > getname(), so if do_linkat() accepts struct filename* then there is no > bullet-proof way to force the capability. > > We could do something like this: > > do_linkat(oldfd, getname_uflags(oldname, flags), newfd, > getname(newname), flags); > > I.e. call getname_uflags() without checking the capability and rely on > the fact that do_linkat() will do the checking. But this is fragile if > somehow someone passes different flags to getname_uflags and do_linkat. > And there is no way (that I know of) for do_linkat to actually check > that AT_EMPTY_PATH was not used if it gets struct filename. > > Or am I creating extra problems and the thing above is OK? Hm, I get your point but if you e.g. look at fs/exec.c we already do have that problem today: SYSCALL_DEFINE5(execveat, int, fd, const char __user *, filename, const char __user *const __user *, argv, const char __user *const __user *, envp, int, flags) { int lookup_flags = (flags & AT_EMPTY_PATH) ? LOOKUP_EMPTY : 0; return do_execveat(fd, getname_flags(filename, lookup_flags, NULL), argv, envp, flags); } The new simple flag helper would simplify things because right now it pretends that it cares about multiple flags where it actually just cares about whether or not empty pathnames are allowed and it forces callers to translate between flags too. (Note, just my opinion this might get shot to hell.) Christian
On Thu, Apr 15, 2021 at 9:09 PM Christian Brauner <christian.brauner@ubuntu.com> wrote: > Hm, I get your point but if you e.g. look at fs/exec.c we already do > have that problem today: > > SYSCALL_DEFINE5(execveat, > int, fd, const char __user *, filename, > const char __user *const __user *, argv, > const char __user *const __user *, envp, > int, flags) > { > int lookup_flags = (flags & AT_EMPTY_PATH) ? LOOKUP_EMPTY : 0; > > return do_execveat(fd, > getname_flags(filename, lookup_flags, NULL), > argv, envp, flags); > } > > The new simple flag helper would simplify things because right now it > pretends that it cares about multiple flags where it actually just cares > about whether or not empty pathnames are allowed and it forces callers > to translate between flags too. Hi Christian, Sorry for the long silence, I got overwhelmed by the primary job and life stuff. I've finally carved out some time to work on this. I left out the "make getname_flags accept a single boolean instead of flags" bit to make the change smaller. If you think it's something that definitely should be in this patch set then let me know, I'll put it back in. I'm still somewhat concerned about the separation of the capability check and the actual logic to get the name, but I guess I'll just post what I have and collect comments. I'll send the v4 soon.
On Thu, May 13, 2021 at 02:45:39PM +0700, Dmitry Kadashev wrote: > On Thu, Apr 15, 2021 at 9:09 PM Christian Brauner > <christian.brauner@ubuntu.com> wrote: > > Hm, I get your point but if you e.g. look at fs/exec.c we already do > > have that problem today: > > > > SYSCALL_DEFINE5(execveat, > > int, fd, const char __user *, filename, > > const char __user *const __user *, argv, > > const char __user *const __user *, envp, > > int, flags) > > { > > int lookup_flags = (flags & AT_EMPTY_PATH) ? LOOKUP_EMPTY : 0; > > > > return do_execveat(fd, > > getname_flags(filename, lookup_flags, NULL), > > argv, envp, flags); > > } > > > > The new simple flag helper would simplify things because right now it > > pretends that it cares about multiple flags where it actually just cares > > about whether or not empty pathnames are allowed and it forces callers > > to translate between flags too. > > Hi Christian, > > Sorry for the long silence, I got overwhelmed by the primary job and life > stuff. I've finally carved out some time to work on this. I left out the No problem at all! Yeah, I can relate. :) > "make getname_flags accept a single boolean instead of flags" bit to > make the change smaller. If you think it's something that definitely > should be in this patch set then let me know, I'll put it back in. I'm > still somewhat concerned about the separation of the capability check > and the actual logic to get the name, but I guess I'll just post what I > have and collect comments. Sounds good! Christian
diff --git a/fs/internal.h b/fs/internal.h index 6aeae7ef3380..848e165ef0f1 100644 --- a/fs/internal.h +++ b/fs/internal.h @@ -77,6 +77,7 @@ long do_unlinkat(int dfd, struct filename *name); int may_linkat(struct user_namespace *mnt_userns, struct path *link); int do_renameat2(int olddfd, struct filename *oldname, int newdfd, struct filename *newname, unsigned int flags); +long do_mkdirat(int dfd, struct filename *name, umode_t mode); /* * namespace.c diff --git a/fs/namei.c b/fs/namei.c index 216f16e74351..bfdc3128bf8d 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -3560,7 +3560,7 @@ struct file *do_file_open_root(struct dentry *dentry, struct vfsmount *mnt, return file; } -static struct dentry *filename_create(int dfd, struct filename *name, +static struct dentry *__filename_create(int dfd, struct filename *name, struct path *path, unsigned int lookup_flags) { struct dentry *dentry = ERR_PTR(-EEXIST); @@ -3616,7 +3616,6 @@ static struct dentry *filename_create(int dfd, struct filename *name, error = err2; goto fail; } - putname(name); return dentry; fail: dput(dentry); @@ -3631,6 +3630,16 @@ static struct dentry *filename_create(int dfd, struct filename *name, return dentry; } +static inline struct dentry *filename_create(int dfd, struct filename *name, + struct path *path, unsigned int lookup_flags) +{ + struct dentry *res = __filename_create(dfd, name, path, lookup_flags); + + if (!IS_ERR(res)) + putname(name); + return res; +} + struct dentry *kern_path_create(int dfd, const char *pathname, struct path *path, unsigned int lookup_flags) { @@ -3821,15 +3830,18 @@ int vfs_mkdir(struct user_namespace *mnt_userns, struct inode *dir, } EXPORT_SYMBOL(vfs_mkdir); -static long do_mkdirat(int dfd, const char __user *pathname, umode_t mode) +long do_mkdirat(int dfd, struct filename *name, umode_t mode) { struct dentry *dentry; struct path path; int error; unsigned int lookup_flags = LOOKUP_DIRECTORY; + if (IS_ERR(name)) + return PTR_ERR(name); + retry: - dentry = user_path_create(dfd, pathname, &path, lookup_flags); + dentry = __filename_create(dfd, name, &path, lookup_flags); if (IS_ERR(dentry)) return PTR_ERR(dentry); @@ -3847,17 +3859,18 @@ static long do_mkdirat(int dfd, const char __user *pathname, umode_t mode) lookup_flags |= LOOKUP_REVAL; goto retry; } + putname(name); return error; } SYSCALL_DEFINE3(mkdirat, int, dfd, const char __user *, pathname, umode_t, mode) { - return do_mkdirat(dfd, pathname, mode); + return do_mkdirat(dfd, getname(pathname), mode); } SYSCALL_DEFINE2(mkdir, const char __user *, pathname, umode_t, mode) { - return do_mkdirat(AT_FDCWD, pathname, mode); + return do_mkdirat(AT_FDCWD, getname(pathname), mode); } /**
Pass in the struct filename pointers instead of the user string, and update the three callers to do the same. This is heavily based on commit dbea8d345177 ("fs: make do_renameat2() take struct filename"). This behaves like do_unlinkat() and do_renameat2(). Cc: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: Dmitry Kadashev <dkadashev@gmail.com> --- fs/internal.h | 1 + fs/namei.c | 25 +++++++++++++++++++------ 2 files changed, 20 insertions(+), 6 deletions(-)