Re: [PATCH] vfs: move fdput() to right place in ksys_sync_file_range()

[Al Viro <viro@xxxxxxxxxxxxxxxxxx>]

On Wed, May 11, 2022 at 09:43:46PM +0000, Al Viro wrote:

> 3) ovl_aio_put() is hard to follow (and some of the callers are poking
> where they shouldn't), no idea if it's correct.  struct fd is manually
> constructed there, anyway.

Speaking of poking in the internals:

SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
                u32, min_complete, u32, flags, const void __user *, argp,
                size_t, argsz)
{
...
        struct fd f;
...
        if (flags & IORING_ENTER_REGISTERED_RING) {
                struct io_uring_task *tctx = current->io_uring;

                if (!tctx || fd >= IO_RINGFD_REG_MAX)
                        return -EINVAL;
                fd = array_index_nospec(fd, IO_RINGFD_REG_MAX);
                f.file = tctx->registered_rings[fd];
                if (unlikely(!f.file))
                        return -EBADF;
        } else {
                f = fdget(fd);
                if (unlikely(!f.file))
                        return -EBADF;
        }
...
a bunch of accesses to f.file
...
        if (!(flags & IORING_ENTER_REGISTERED_RING))
                fdput(f);

Note that f.flags is left uninitialized in the first case; it doesn't
break since we have fdput(f) (which does look at f.flags) done only
in the case where we don't have IORING_ENTER_REGISTERED_RING in flags
and since flags remains unchanged since the first if.  But it would
be just as easy to set f.flags to 0 and use fdput() in both cases...

Jens, do you have any objections against the following?  Easier to
follow that way...

Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
---
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 91de361ea9ab..b61ae18ef10a 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -10760,14 +10760,14 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
 			return -EINVAL;
 		fd = array_index_nospec(fd, IO_RINGFD_REG_MAX);
 		f.file = tctx->registered_rings[fd];
-		if (unlikely(!f.file))
-			return -EBADF;
+		f.flags = 0;
 	} else {
 		f = fdget(fd);
-		if (unlikely(!f.file))
-			return -EBADF;
 	}
 
+	if (unlikely(!f.file))
+		return -EBADF;
+
 	ret = -EOPNOTSUPP;
 	if (unlikely(f.file->f_op != &io_uring_fops))
 		goto out_fput;
@@ -10840,8 +10840,7 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
 out:
 	percpu_ref_put(&ctx->refs);
 out_fput:
-	if (!(flags & IORING_ENTER_REGISTERED_RING))
-		fdput(f);
+	fdput(f);
 	return submitted ? submitted : ret;
 }