linux-kernel - Re: [PATCH v2 6/6] RFC: io_uring getdents: test returning an EOF flag in CQE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZG0qgniV1DzIbbzi@codewreck.org>
Date:   Wed, 24 May 2023 06:05:06 +0900
From:   Dominique Martinet <asmadeus@...ewreck.org>
To:     Christian Brauner <brauner@...nel.org>
Cc:     Alexander Viro <viro@...iv.linux.org.uk>,
        Jens Axboe <axboe@...nel.dk>,
        Pavel Begunkov <asml.silence@...il.com>,
        Stefan Roesch <shr@...com>, Clay Harris <bugs@...ycon.org>,
        Dave Chinner <david@...morbit.com>,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        io-uring@...r.kernel.org
Subject: Re: [PATCH v2 6/6] RFC: io_uring getdents: test returning an EOF
 flag in CQE

Christian Brauner wrote on Tue, May 23, 2023 at 04:30:14PM +0200:
> > index b15ec81c1ed2..f6222b0148ef 100644
> > --- a/io_uring/fs.c
> > +++ b/io_uring/fs.c
> > @@ -322,6 +322,7 @@ int io_getdents(struct io_kiocb *req, unsigned int issue_flags)
> >  {
> >  	struct io_getdents *gd = io_kiocb_to_cmd(req, struct io_getdents);
> >  	unsigned long getdents_flags = 0;
> > +	u32 cqe_flags = 0;
> >  	int ret;
> >  
> >  	if (issue_flags & IO_URING_F_NONBLOCK) {
> > @@ -338,13 +339,16 @@ int io_getdents(struct io_kiocb *req, unsigned int issue_flags)
> >  			goto out;
> >  	}
> >  
> > -	ret = vfs_getdents(req->file, gd->dirent, gd->count, getdents_flags);
> > +	ret = vfs_getdents(req->file, gd->dirent, gd->count, &getdents_flags);
> 
> I don't understand how synchronization and updating of f_pos works here.
> For example, what happens if a concurrent seek happens on the fd while
> io_uring is using vfs_getdents which calls into iterate_dir() and
> updates f_pos?

I don't see how different that is from a user spawning two threads and
calling getdents64 + lseek or two getdents64 in parallel?
(or any two other users of iterate_dir)

As far as I understand you'll either get the old or new pos as
obtained/updated by iterate_dir()?

That iterate_dir probably ought to be using READ_ONCE/WRITE_ONCE or some
atomic read/update wrappers as the shared case only has a read lock
around these, but that's not a new problem; and for all I care
about I'm happy to let users shoot themselves in the foot.
(although I guess that with filesystems not validating the offset as
was pointed out in a previous version comment having non-atomic update
might be a security issue at some point on architectures that don't
guarantee atomic 64bit updates, but if someone manages to abuse it
it's already possible to abuse it with the good old syscalls, so I'd
rather leave that up to someone who understand how atomicity in the
kernel works better than me...)

-- 
Dominique Martinet | Asmadeus