| Message ID | 20200714161203.31879-1-yanfei.xu@windriver.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | userfaultfd: avoid the duplicated release for userfaultfd_ctx | expand |
Add maintainer Alexander Viro :) On 7/15/20 12:12 AM, yanfei.xu@windriver.com wrote: > From: Yanfei Xu <yanfei.xu@windriver.com> > > when get_unused_fd_flags gets failure, userfaultfd_ctx_cachep will > be freed by userfaultfd_fops's release function which is the > userfaultfd_release. So we could return directly after fput(). > > userfaultfd_release()->userfaultfd_ctx_put(ctx) > > Fixes: d08ac70b1e0d (Wire UFFD up to SELinux) > Reported-by: syzbot+75867c44841cb6373570@syzkaller.appspotmail.com > Signed-off-by: Yanfei Xu <yanfei.xu@windriver.com> > --- > fs/userfaultfd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c > index 3a4d6ac5a81a..e98317c15530 100644 > --- a/fs/userfaultfd.c > +++ b/fs/userfaultfd.c > @@ -2049,7 +2049,7 @@ SYSCALL_DEFINE1(userfaultfd, int, flags) > fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC); > if (fd < 0) { > fput(file); > - goto out; > + return fd; > } > > ctx->owner = file_inode(file); >
ping Al Viro Could you please help to review this patch? Thanks a lot. Yanfei On 7/15/20 12:12 AM, yanfei.xu@windriver.com wrote: > From: Yanfei Xu <yanfei.xu@windriver.com> > > when get_unused_fd_flags gets failure, userfaultfd_ctx_cachep will > be freed by userfaultfd_fops's release function which is the > userfaultfd_release. So we could return directly after fput(). > > userfaultfd_release()->userfaultfd_ctx_put(ctx) > > Fixes: d08ac70b1e0d (Wire UFFD up to SELinux) > Reported-by: syzbot+75867c44841cb6373570@syzkaller.appspotmail.com > Signed-off-by: Yanfei Xu <yanfei.xu@windriver.com> > --- > fs/userfaultfd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c > index 3a4d6ac5a81a..e98317c15530 100644 > --- a/fs/userfaultfd.c > +++ b/fs/userfaultfd.c > @@ -2049,7 +2049,7 @@ SYSCALL_DEFINE1(userfaultfd, int, flags) > fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC); > if (fd < 0) { > fput(file); > - goto out; > + return fd; > } > > ctx->owner = file_inode(file); >
On Sun, Jul 19, 2020 at 09:58:34PM +0800, Xu, Yanfei wrote: > ping Al Viro > > Could you please help to review this patch? Thanks a lot. That's -next, right? As for the patch itself... Frankly, Daniel's patch looks seriously wrong. * why has O_CLOEXEC been quietly smuggled in? It's a userland ABI change, for fsck sake... * the double-put you've spotted * the whole out: thing - just make it if (IS_ERR(file)) { userfaultfd_ctx_put(ctx); return PTR_ERR(file); } and be done with that.
On 7/20/20 12:57 AM, Al Viro wrote: > On Sun, Jul 19, 2020 at 09:58:34PM +0800, Xu, Yanfei wrote: >> ping Al Viro >> >> Could you please help to review this patch? Thanks a lot. > > That's -next, right? As for the patch itself... Frankly, Yes, it's -next. > Daniel's patch looks seriously wrong. Get it. Regards, Yanfei > * why has O_CLOEXEC been quietly smuggled in? It's > a userland ABI change, for fsck sake... > * the double-put you've spotted > * the whole out: thing - just make it > if (IS_ERR(file)) { > userfaultfd_ctx_put(ctx); > return PTR_ERR(file); > } > and be done with that. >
On Sun, Jul 19, 2020 at 6:34 PM Xu, Yanfei <yanfei.xu@windriver.com> wrote: > > > > On 7/20/20 12:57 AM, Al Viro wrote: > > On Sun, Jul 19, 2020 at 09:58:34PM +0800, Xu, Yanfei wrote: > >> ping Al Viro > >> > >> Could you please help to review this patch? Thanks a lot. > > > > That's -next, right? As for the patch itself... Frankly, > Yes, it's -next. > > Daniel's patch looks seriously wrong. > Get it. > > Regards, > Yanfei > > * why has O_CLOEXEC been quietly smuggled in? It's > > a userland ABI change, for fsck sake... > > * the double-put you've spotted > > * the whole out: thing - just make it > > if (IS_ERR(file)) { > > userfaultfd_ctx_put(ctx); > > return PTR_ERR(file); > > } > > and be done with that. > > Adding Lokesh to take a look.
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index 3a4d6ac5a81a..e98317c15530 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -2049,7 +2049,7 @@ SYSCALL_DEFINE1(userfaultfd, int, flags) fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC); if (fd < 0) { fput(file); - goto out; + return fd; } ctx->owner = file_inode(file);