linux-kernel - Re: [RFC PATCH] vfs: shutdown lease notifications on file close

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]

Date:   Fri, 13 Oct 2017 11:00:33 -0700
From:   Dan Williams <dan.j.williams@...el.com>
To:     Al Viro <viro@...iv.linux.org.uk>
Cc:     "J. Bruce Fields" <bfields@...ldses.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Jeff Layton <jlayton@...chiereds.net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH] vfs: shutdown lease notifications on file close

On Fri, Oct 13, 2017 at 10:43 AM, Dan Williams <dan.j.williams@...el.com> wrote:
> On Fri, Oct 13, 2017 at 10:01 AM, Al Viro <viro@...iv.linux.org.uk> wrote:
>> On Fri, Oct 13, 2017 at 08:56:10AM -0700, Dan Williams wrote:
>>> While implementing MAP_DIRECT, an mmap flag that arranges for an
>>> FL_LAYOUT lease to be established, Al noted:
>>>
>>>     You are not even guaranteed that descriptor will remain be still
>>>     open by the time you pass it down to your helper, nevermind the
>>>     moment when event actually happens...
>>>
>>> The first problem can be solved with an fd{get,put} at mmap
>>> {entry,exit}.
>>
>> Huh?  fdget() does *NOT* guarantee that descriptor won't get closed.  What
>> it does is guarantee that struct file won't get closed under you, which
>> is nowhere near the same thing.  And while we are at it, it certainly
>> _is_ called by mmap()...
>>
>>> The second problem appears to be a general issue.
>>>
>>> Leases follow the lifetime of the inode, so it is possible for a lease
>>> to be broken after the file is closed. When that happens userspace may
>>> get a notification on a stale fd. Of course it is not recommended that a
>>> process close a file descriptor with an active lease, but if it does we
>>> should assume that the notification is not needed either. Walk leases at
>>> close time and invalidate any pending fasync instances.
>>
>> What the hell is special about close(2) and not, e.g. dup2(2)?  Or execve(2)
>> triggering close-on-exec, etc...  Besides, you are changing a user-visible
>> behaviour here.  Suppose your process forks and the child closes all
>> descriptors; should that stop SIGIO delivery to the parent?
>>
>> Let's step back for a minute; could you describe how the userland is supposed
>> to use that thing?
>
> MAP_DIRECT is a meant as a way to safely pass DAX mappings of a file
> to the RDMA sub-system...

Al, before you spend any more time thinking about this let me close
with the RDMA folks on a notification scheme that works for them.