linux-kernel - Re: [PATCH v3b] eventfd: convert to f_op->read

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] [day] [month] [year] [list]

Message-ID: <db9405e8-5960-787f-5a4c-8266b2e456f2@kernel.dk>
Date:   Fri, 1 May 2020 13:08:18 -0600
From:   Jens Axboe <axboe@...nel.dk>
To:     Al Viro <viro@...iv.linux.org.uk>
Cc:     linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3b] eventfd: convert to f_op->read_iter()

On 5/1/20 1:00 PM, Al Viro wrote:
> On Fri, May 01, 2020 at 11:54:01AM -0600, Jens Axboe wrote:
> 
>> @@ -427,8 +424,17 @@ static int do_eventfd(unsigned int count, int flags)
>>  
>>  	fd = anon_inode_getfd("[eventfd]", &eventfd_fops, ctx,
>>  			      O_RDWR | (flags & EFD_SHARED_FCNTL_FLAGS));
>> -	if (fd < 0)
>> +	if (fd < 0) {
>>  		eventfd_free_ctx(ctx);
>> +	} else {
>> +		struct file *file;
>> +
>> +		file = fget(fd);
>> +		if (file) {
>> +			file->f_mode |= FMODE_NOWAIT;
>> +			fput(file);
>> +		}
> 
> No.  The one and only thing you can do to return value of anon_inode_getfd() is to
> return the fscker to userland.  You *CAN* *NOT* assume that descriptor table is
> still pointing to whatever you've just created.
> 
> As soon as it's in descriptor table, it's out of your hands.  And frankly, if you
> are playing with descriptors, you should be very well aware of that.
> 
> Descriptor tables are fundamentally shared objects; they *can* be accessed and
> modified by other threads, right behind your back.
> 
> *IF* you are going to play with ->f_mode, you must use get_unused_fd_flags(),
> anon_inode_getfile(), modify ->f_mode of the result and use fd_install() to
> put it into descriptor table.  With put_unused_fd() as cleanup in case
> of allocation failure.

OK, that makes sense, so we've got f_mode set before the fd_install() and fd
visibility. I wrote that up, will test, and send out a v4... Thanks Al, this
is very helpful.

-- 
Jens Axboe