Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v4] eventfd: convert to f_op->read_iter()
From:   Jens Axboe <axboe@kernel.dk>
To:     Al Viro <viro@zeniv.linux.org.uk>
Cc:     linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
References: <6b29f015-bd7c-0601-cf94-2c077285b933@kernel.dk>
 <20200501231231.GR23230@ZenIV.linux.org.uk>
 <03867cf3-d5e7-cc29-37d2-1a417a58af45@kernel.dk>
 <20200503134622.GS23230@ZenIV.linux.org.uk>
 <435c171c-37aa-8f7d-c506-d1e8f07f4bc7@kernel.dk>
Message-ID: <c8129f11-38a8-64e6-be8e-b485d2b7fb26@kernel.dk>
Date:   Sun, 3 May 2020 10:50:17 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.7.0
MIME-Version: 1.0
In-Reply-To: <435c171c-37aa-8f7d-c506-d1e8f07f4bc7@kernel.dk>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 5/3/20 8:42 AM, Jens Axboe wrote:
> On 5/3/20 7:46 AM, Al Viro wrote:
>> On Fri, May 01, 2020 at 05:54:09PM -0600, Jens Axboe wrote:
>>> On 5/1/20 5:12 PM, Al Viro wrote:
>>>> On Fri, May 01, 2020 at 01:11:09PM -0600, Jens Axboe wrote:
>>>>> +	flags &= EFD_SHARED_FCNTL_FLAGS;
>>>>> +	flags |= O_RDWR;
>>>>> +	fd = get_unused_fd_flags(flags);
>>>>>  	if (fd < 0)
>>>>> -		eventfd_free_ctx(ctx);
>>>>> +		goto err;
>>>>> +
>>>>> +	file = anon_inode_getfile("[eventfd]", &eventfd_fops, ctx, flags);
>>>>> +	if (IS_ERR(file)) {
>>>>> +		put_unused_fd(fd);
>>>>> +		fd = PTR_ERR(file);
>>>>> +		goto err;
>>>>> +	}
>>>>>  
>>>>> +	file->f_mode |= FMODE_NOWAIT;
>>>>> +	fd_install(fd, file);
>>>>> +	return fd;
>>>>> +err:
>>>>> +	eventfd_free_ctx(ctx);
>>>>>  	return fd;
>>>>>  }
>>>>
>>>> Looks sane...  I can take it via vfs.git, or leave it for you if you
>>>> have other stuff in the same area...
>>>
>>> Would be great if you can queue it up in vfs.git, thanks! Don't have
>>> anything else that'd conflict with this.
>>
>> Applied; BTW, what happens if
>>         ctx->id = ida_simple_get(&eventfd_ida, 0, 0, GFP_KERNEL);
>> fails?  Quitely succeed with BS value (-ENOSPC/-ENOMEM) shown by
>> eventfd_show_fdinfo()?
> 
> Huh yeah that's odd, not sure how I missed that when touching code
> near it. Want a followup patch to fix that issue?

This should do the trick. Ideally we'd change the order of these, and
shove this fix into 5.7, but not sure it's super important since this
bug is pretty old. Would make stable backports easier, though...
Let me know how you want to handle it, as it'll impact the one you
have already queued up.


diff --git a/fs/eventfd.c b/fs/eventfd.c
index 20f0fd4d56e1..96081efdd0c9 100644
--- a/fs/eventfd.c
+++ b/fs/eventfd.c
@@ -422,6 +422,10 @@ static int do_eventfd(unsigned int count, int flags)
 	ctx->count = count;
 	ctx->flags = flags;
 	ctx->id = ida_simple_get(&eventfd_ida, 0, 0, GFP_KERNEL);
+	if (ctx->id < 0) {
+		fd = ctx->id;
+		goto err;
+	}
 
 	flags &= EFD_SHARED_FCNTL_FLAGS;
 	flags |= O_RDWR;

-- 
Jens Axboe