On Mon, Sep 4, 2023 at 9:29 AM Christian Brauner brauner@kernel.org wrote:
On Fri, Sep 01, 2023 at 11:34:32AM -0700, Kees Cook wrote:
On Fri, Sep 01, 2023 at 04:50:53PM +0200, Michał Cłapiński wrote:
On Fri, Sep 1, 2023 at 2:56 PM Christian Brauner brauner@kernel.org wrote:
On Thu, Aug 31, 2023 at 10:36:46PM +0200, Michal Clapinski wrote:
Add a way to check if an fd points to the memfd's original open fd (the one created by memfd_create). Useful because only the original open fd can be both writable and executable.
Signed-off-by: Michal Clapinski mclapinski@google.com
fs/fcntl.c | 3 +++ include/uapi/linux/fcntl.h | 9 +++++++++ 2 files changed, 12 insertions(+)
diff --git a/fs/fcntl.c b/fs/fcntl.c index e871009f6c88..301527e07a4d 100644 --- a/fs/fcntl.c +++ b/fs/fcntl.c @@ -419,6 +419,9 @@ static long do_fcntl(int fd, unsigned int cmd, unsigned long arg, case F_SET_RW_HINT: err = fcntl_rw_hint(filp, cmd, arg); break;
case F_CHECK_ORIGINAL_MEMFD:err = !(filp->f_mode & FMODE_WRITER);break;Honestly, make this an ioctl on memfds. This is so specific that it really doesn't belong into fcntl().
I've never touched ioctls but if I'm correct, I can't just add it to memfd. I would have to add it to the underlying fs, so hugetlbfs and shmem (which I think can be defined as ramfs so also there). File sealing fcntl is already memfd specific. Are you sure ioctl will be a better idea?
fcntl() should be generic. Frankly, the sealing stuff should've gone into an ioctl as well and only upgraded to a fcntl() once multiple fd types support it.
But ioctl is good for stuff related to the underlying fs, which this isn't. I'm worried if I rewrite it as an ioctl and put it in 3 different places, the maintainers of shmem, hugetlbfs and ramfs will tell me to rewrite it as an fcntl. If a new filesystem pops up that can be used as the backend for memfd, the ioctl will also have to be added there.
Does this check "mean" anything for other files? Because if it's generically useful (and got renamed) it maybe would be right for fcntl...
For regular files it just means that the file has gotten write access to the underlying fs and we use this flag to release the necessary refcounts/protections once the file is closed.
If this check has any meaning beyond that than it only has meaning for memfd. I'm also not sure why this checks FMODE_WRITER and not FMODE_WRITE as FMODE_WRITER is almost an entirely internal thing that only very specific codepaths need to know about.
If you reopen the memfd via /proc/<pid>/fd/ with O_RDWR, both file objects (the original and the reopened one) have FMODE_WRITE, so knowing if the flag is set gives me nothing. FMODE_WRITER is the only difference between the original fd and the reopened one. This flag also dictates whether `inode->i_writecount` will be decremented on close (in `put_file_access`) which influences exec()ability of the other fd. It surprised me too that this flag theoretically means "write access to underlying fs" but it's used to determine whether to decrement i_writecount.