LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Stefan Metzmacher <metze@samba.org>
To: Linus Torvalds <torvalds@linux-foundation.org>,
	David Howells <dhowells@redhat.com>
Cc: Aleksa Sarai <cyphar@cyphar.com>,
	Al Viro <viro@zeniv.linux.org.uk>, Ian Kent <raven@themaw.net>,
	Miklos Szeredi <mszeredi@redhat.com>,
	Christian Brauner <christian@brauner.io>,
	Jann Horn <jannh@google.com>,
	"Darrick J. Wong" <darrick.wong@oracle.com>,
	Karel Zak <kzak@redhat.com>,
	jlayton@redhat.com, Linux API <linux-api@vger.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	LSM List <linux-security-module@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 01/14] VFS: Add additional RESOLVE_* flags [ver #18]
Date: Thu, 12 Mar 2020 10:08:05 +0100	[thread overview]
Message-ID: <3d209e29-e73d-23a6-5c6f-0267b1e669b6@samba.org> (raw)
In-Reply-To: <CAHk-=wiaL6zznNtCHKg6+MJuCqDxO=yVfms3qR9A0czjKuSSiA@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 3065 bytes --]

Hi Linus,

>> Okay.  So what's the equivalent of AT_SYMLINK_NOFOLLOW in RESOLVE_* flag
>> terms?
> 
> Nothing.
> 
> openat2() takes two sets of flags. We'll never get rid of
> AT_SYMLINK_NOFOLLOW / O_NOFOLLOW, and we've added RESOLVE_NO_SYMLINKS
> to the new set of flags. It's just a separate namespace.
> 
> We will _not_ be adding a RESOLVE_XYZ flag for O_NOFOLLOW or
> AT_SYMLINK_NOFOLLOW. At least not visible to user space - because as
> people already figured out, that just causes problems with consistency
> issues.
> 
> And yes, the fact that we then have three different user-visible
> namespaces (O_xyz flags for open(), AT_xyz flags for linkat(), and now
> RESOLVE_xyz flags for openat2()) is sad and messy. But it's an
> inherent messiness from just how the world works. We can't get rid of
> it.

For openat2() and other existing syscalls I agree, that it's good to
have just a single bit to control a feature.

The whole discussion was triggered by the introduction of a completely
new fsinfo()
call:

>> The new system call looks like:
>> 
>> 	int ret = fsinfo(int dfd,
>> 			 const char *pathname,
>> 			 const struct fsinfo_params *params,
>> 			 size_t params_size,
>> 			 void *result_buffer,
>> 			 size_t result_buf_size);
>> 
>> The params parameter optionally points to a block of parameters:
>> 
>> 	struct fsinfo_params {
>> 		__u32	resolve_flags;

If I remember correctly with was named at_flags initially.
And I thought it would be great to also have the new RESOLVE_YXZ feature
available for that new path based syscall.

Would you propose to have 'at_flags' and 'resolve_flags' passed in here?
Or is there something even better you would propose for new syscalls?

>> 		__u32	flags;
>> 		__u32	request;
>> 		__u32	Nth;
>> 		__u32	Mth;
>> 	};

> If we need linkat2() and friends, so be it. Do we?

Yes, I'm going to propose something like this, as it would make the life
much easier for Samba to have the new features available on all path
based syscalls.

In addition I'll propose to have a way to specify the source of
removeat and unlinkat also by fd in addition to the the source parent fd
and relative path, the reason are also to detect races of path
recycling. pidfd_open() solved a similar problem for pid recycling.
> Could we have a _fourth_ set of flags that are simply for internal use
> that is a superset of them all? Sure. But no, it's almost certainly
> not worth it. Four is not better than three.

As you pointed our below the LOOKUP_yxz namespace is already in place...
And the discussion was more about an possible single namespace for
completely new syscalls.

> Now, some type-safety in the kernel to make sure that we can't mix
> AT_xyz with O_xyz or RESOLVE_xyz - that might be worth it. Although
> judging by past experience, not enough people run sparse for it to
> really be worth it.

I'm new to all this and maybe too naive, but would a build bot running
sparse on linux-next be able to catch this early enough?

metze



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2020-03-12  9:08 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-09 14:00 [PATCH 00/14] VFS: Filesystem information " David Howells
2020-03-09 14:00 ` [PATCH 01/14] VFS: Add additional RESOLVE_* flags " David Howells
2020-03-09 20:56   ` Stefan Metzmacher
2020-03-09 21:13   ` David Howells
2020-03-10  0:55   ` Aleksa Sarai
2020-03-10  1:14     ` Linus Torvalds
2020-03-10  7:25     ` David Howells
2020-03-11 17:59       ` Linus Torvalds
2020-03-12  9:08         ` Stefan Metzmacher [this message]
2020-03-12 16:24           ` Linus Torvalds
2020-03-12 17:11             ` Stefan Metzmacher
2020-03-12 19:37               ` Al Viro
2020-03-12 21:48               ` Jeremy Allison
2020-03-13  9:59               ` Aleksa Sarai
2020-03-13 10:00                 ` Aleksa Sarai
2020-03-13 16:48                 ` Jeremy Allison
2020-03-13 18:28                 ` Al Viro
2020-03-13 18:35                   ` Jeremy Allison
2020-03-16 14:21                   ` Aleksa Sarai
2020-03-16 14:20                     ` Aleksa Sarai
2020-03-12 19:25             ` Al Viro
2020-03-12 16:56           ` David Howells
2020-03-12 18:09             ` Linus Torvalds
2020-03-13  9:53               ` Aleksa Sarai
2020-03-13  9:50         ` Aleksa Sarai
2020-03-09 14:01 ` [PATCH 02/14] fsinfo: Add fsinfo() syscall to query filesystem information " David Howells
2020-03-10  9:31   ` Christian Brauner
2020-03-10  9:32     ` [PATCH v19 01/14] fsinfo: Add fsinfo() syscall to query filesystem information Christian Brauner
2020-03-10  9:32       ` [PATCH v19 14/14] arch: wire up fsinfo syscall Christian Brauner
2020-03-09 14:01 ` [PATCH 03/14] fsinfo: Provide a bitmap of supported features [ver #18] David Howells
2020-03-09 14:01 ` [PATCH 04/14] fsinfo: Allow retrieval of superblock devname, options and stats " David Howells
2020-03-09 14:01 ` [PATCH 05/14] fsinfo: Allow fsinfo() to look up a mount object by ID " David Howells
2020-03-09 14:01 ` [PATCH 06/14] fsinfo: Add a uniquifier ID to struct mount " David Howells
2020-03-09 14:01 ` [PATCH 07/14] fsinfo: Allow mount information to be queried " David Howells
2020-03-10  9:04   ` Miklos Szeredi
2020-03-09 14:02 ` [PATCH 08/14] fsinfo: Allow the mount topology propogation flags to be retrieved " David Howells
2020-03-10  8:42   ` Christian Brauner
2020-03-09 14:02 ` [PATCH 09/14] fsinfo: Provide notification overrun handling support " David Howells
2020-03-09 14:02 ` [PATCH 10/14] fsinfo: sample: Mount listing program " David Howells
2020-03-09 14:02 ` [PATCH 11/14] fsinfo: Add API documentation " David Howells
2020-03-09 14:02 ` [PATCH 12/14] fsinfo: Add support for AFS " David Howells
2020-03-09 14:02 ` [PATCH 13/14] fsinfo: Example support for Ext4 " David Howells
2020-03-09 14:02 ` [PATCH 14/14] fsinfo: Example support for NFS " David Howells
2020-03-09 17:50 ` [PATCH 00/14] VFS: Filesystem information " Jeff Layton
2020-03-09 19:22   ` Andres Freund
2020-03-09 22:49     ` Jeff Layton
2020-03-10  0:18       ` Andres Freund
2020-03-09 20:02 ` Miklos Szeredi
2020-03-09 22:52 ` David Howells
2020-03-10  9:18   ` Miklos Szeredi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3d209e29-e73d-23a6-5c6f-0267b1e669b6@samba.org \
    --to=metze@samba.org \
    --cc=christian@brauner.io \
    --cc=cyphar@cyphar.com \
    --cc=darrick.wong@oracle.com \
    --cc=dhowells@redhat.com \
    --cc=jannh@google.com \
    --cc=jlayton@redhat.com \
    --cc=kzak@redhat.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mszeredi@redhat.com \
    --cc=raven@themaw.net \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --subject='Re: [PATCH 01/14] VFS: Add additional RESOLVE_* flags [ver #18]' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).