public inbox for linux-cifs@vger.kernel.org
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: Dorjoy Chowdhury <dorjoychy111@gmail.com>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	 ceph-devel@vger.kernel.org, gfs2@lists.linux.dev,
	linux-nfs@vger.kernel.org, 	linux-cifs@vger.kernel.org,
	v9fs@lists.linux.dev, 	linux-kselftest@vger.kernel.org,
	viro@zeniv.linux.org.uk, brauner@kernel.org, 	jack@suse.cz,
	chuck.lever@oracle.com, alex.aring@gmail.com, arnd@arndb.de,
	 adilger@dilger.ca, mjguzik@gmail.com, smfrench@gmail.com,
	 richard.henderson@linaro.org, mattst88@gmail.com,
	linmag7@gmail.com,  tsbogend@alpha.franken.de,
	James.Bottomley@hansenpartnership.com, deller@gmx.de,
		davem@davemloft.net, andreas@gaisler.com, idryomov@gmail.com,
	amarkuze@redhat.com, 	slava@dubeyko.com, agruenba@redhat.com,
	trondmy@kernel.org, anna@kernel.org,  sfrench@samba.org,
	pc@manguebit.org, ronniesahlberg@gmail.com,
	 sprasad@microsoft.com, tom@talpey.com, bharathsm@microsoft.com,
	shuah@kernel.org, 	miklos@szeredi.hu, hansg@kernel.org
Subject: Re: [PATCH v4 1/4] openat2: new OPENAT2_REGULAR flag support
Date: Sun, 01 Mar 2026 09:47:15 -0500	[thread overview]
Message-ID: <ed5aeaa81ad9b87926fa7ebee0308aeb8df9f0ac.camel@kernel.org> (raw)
In-Reply-To: <CAFfO_h7i86qdKZObdFpWd8Mh+8VXVMFYoGgYBgzomzhGJJFnEQ@mail.gmail.com>

On Sun, 2026-03-01 at 20:16 +0600, Dorjoy Chowdhury wrote:
> On Sun, Mar 1, 2026 at 6:44 PM Jeff Layton <jlayton@kernel.org> wrote:
> > 
> > On Sat, 2026-02-21 at 20:45 +0600, Dorjoy Chowdhury wrote:
> > > This flag indicates the path should be opened if it's a regular file.
> > > This is useful to write secure programs that want to avoid being
> > > tricked into opening device nodes with special semantics while thinking
> > > they operate on regular files. This is a requested feature from the
> > > uapi-group[1].
> > > 
> > > A corresponding error code EFTYPE has been introduced. For example, if
> > > openat2 is called on path /dev/null with OPENAT2_REGULAR in the flag
> > > param, it will return -EFTYPE.
> > > 
> > > When used in combination with O_CREAT, either the regular file is
> > > created, or if the path already exists, it is opened if it's a regular
> > > file. Otherwise, -EFTYPE is returned.
> > > 
> > 
> > It would be good to mention that EFTYPE has precedent in BSD/Darwin.
> > When an error code is already supported in another UNIX-y OS, then it
> > bolsters the case for adding it here.
> > 
> 
> Good suggestion. Yes, I can include this information in the commit
> message during the next posting.
> 
> > Your cover letter mentions that you only tested this on btrfs. At the
> > very least, you should test NFS and SMB. It should be fairly easy to
> > set up mounts over loopback for those cases.
> > 
> 
> I used virtme-ng (which I think reuses the host's filesystem) to run
> the compiled bzImage and ran the openat2 kselftests there to verify
> it's working. Is there a similar way I can test NFS/SMB by adding
> kselftests? Or would I need to setup NFS/SMB inside a full VM distro
> with a modified kernel to test this? I would appreciate any suggestion
> on this.
> 

I imagine virtme would need some configuration to set up for nfs or
cifs, but maybe it's possible. I mostly use kdevops for this sort of
testing.

> > There are some places where it doesn't seem like -EFTYPE will be
> > returned. It looks like it can send back -EISDIR and -ENOTDIR in some
> > cases as well. With a new API like this, I think we ought to strive for
> > consistency.
> > 
> 
> Good point. There was a comment in a previous posting of this patch
> series "The most useful behavior would indicate what was found (e.g.,
> a pipe)."
> (ref: https://lore.kernel.org/linux-fsdevel/vhq3osjqs3nn764wrp2lxp66b4dxpb3n5x3dijhe2yr53qfgy3@tfswbjskc3y6/
> )
> So I thought maybe it would be useful to return -EISDIR where it was
> already doing that. But it is a good point about consistency that we
> won't be doing this for other different types so I guess it's better
> to return -EFTYPE for all the cases anyway as you mention. Any
> thoughts?
> 

There is a case to be made for either. The big question is whether you
can consistently return the same error codes in the same situations.

For instance, you can return -EISDIR on NFS when the target is a
directory, but can you do the same on btrfs or ceph? If not, then we
have a situation where we have to deal with the possibility of two
different error codes.

In general, I think returning EFTYPE for everything is simplest and
therefore best. Sure, EISDIR tells you a bit more about the target, but
that info is probably not that helpful if you were expecting it to be a
regular file.

> 
> > Should this API return -EFTYPE for all cases where it's not S_IFREG? If
> > not, then what other errors are allowed? Bear in mind that you'll need
> > to document this in the manpages too.
> > 
> 
> Are the manpages in the kernel git repository or in a separate
> repository? Do I make separate patch series for that? Sorry I don't
> know about this in detail.
> 

Separate repo and mailing list: https://www.kernel.org/doc/man-pages/

...come to think of it, you should also cc the linux-api mailing list
when you send the next version:

    https://www.kernel.org/doc/man-pages/linux-api-ml.html

This one is fairly straightforward, but once a new API is in a released
kernel, it's hard to change things, so we'll want to make sure we get
this right.

I should also ask you about testcases here. You should add some tests
to fstests for O_REGULAR if you haven't already:

    https://www.kernel.org/doc/man-pages/linux-api-ml.html

> > > When OPENAT2_REGULAR is combined with O_DIRECTORY, -EINVAL is returned
> > > as it doesn't make sense to open a path that is both a directory and a
> > > regular file.
> > > 
> > > [1]: https://uapi-group.org/kernel-features/#ability-to-only-open-regular-files
> > > 
> > > Signed-off-by: Dorjoy Chowdhury <dorjoychy111@gmail.com>
> > > ---
> > >  arch/alpha/include/uapi/asm/errno.h        |  2 ++
> > >  arch/alpha/include/uapi/asm/fcntl.h        |  1 +
> > >  arch/mips/include/uapi/asm/errno.h         |  2 ++
> > >  arch/parisc/include/uapi/asm/errno.h       |  2 ++
> > >  arch/parisc/include/uapi/asm/fcntl.h       |  1 +
> > >  arch/sparc/include/uapi/asm/errno.h        |  2 ++
> > >  arch/sparc/include/uapi/asm/fcntl.h        |  1 +
> > >  fs/ceph/file.c                             |  4 ++++
> > >  fs/gfs2/inode.c                            |  2 ++
> > >  fs/namei.c                                 |  4 ++++
> > >  fs/nfs/dir.c                               |  4 +++-
> > >  fs/open.c                                  |  4 +++-
> > >  fs/smb/client/dir.c                        | 11 ++++++++++-
> > >  include/linux/fcntl.h                      |  2 ++
> > >  include/uapi/asm-generic/errno.h           |  2 ++
> > >  include/uapi/asm-generic/fcntl.h           |  4 ++++
> > >  tools/arch/alpha/include/uapi/asm/errno.h  |  2 ++
> > >  tools/arch/mips/include/uapi/asm/errno.h   |  2 ++
> > >  tools/arch/parisc/include/uapi/asm/errno.h |  2 ++
> > >  tools/arch/sparc/include/uapi/asm/errno.h  |  2 ++
> > >  tools/include/uapi/asm-generic/errno.h     |  2 ++
> > >  21 files changed, 55 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/arch/alpha/include/uapi/asm/errno.h b/arch/alpha/include/uapi/asm/errno.h
> > > index 6791f6508632..1a99f38813c7 100644
> > > --- a/arch/alpha/include/uapi/asm/errno.h
> > > +++ b/arch/alpha/include/uapi/asm/errno.h
> > > @@ -127,4 +127,6 @@
> > > 
> > >  #define EHWPOISON    139     /* Memory page has hardware error */
> > > 
> > > +#define EFTYPE               140     /* Wrong file type for the intended operation */
> > > +
> > >  #endif
> > > diff --git a/arch/alpha/include/uapi/asm/fcntl.h b/arch/alpha/include/uapi/asm/fcntl.h
> > > index 50bdc8e8a271..fe488bf7c18e 100644
> > > --- a/arch/alpha/include/uapi/asm/fcntl.h
> > > +++ b/arch/alpha/include/uapi/asm/fcntl.h
> > > @@ -34,6 +34,7 @@
> > > 
> > >  #define O_PATH               040000000
> > >  #define __O_TMPFILE  0100000000
> > > +#define OPENAT2_REGULAR      0200000000
> > > 
> > >  #define F_GETLK              7
> > >  #define F_SETLK              8
> > > diff --git a/arch/mips/include/uapi/asm/errno.h b/arch/mips/include/uapi/asm/errno.h
> > > index c01ed91b1ef4..1835a50b69ce 100644
> > > --- a/arch/mips/include/uapi/asm/errno.h
> > > +++ b/arch/mips/include/uapi/asm/errno.h
> > > @@ -126,6 +126,8 @@
> > > 
> > >  #define EHWPOISON    168     /* Memory page has hardware error */
> > > 
> > > +#define EFTYPE               169     /* Wrong file type for the intended operation */
> > > +
> > >  #define EDQUOT               1133    /* Quota exceeded */
> > > 
> > > 
> > > diff --git a/arch/parisc/include/uapi/asm/errno.h b/arch/parisc/include/uapi/asm/errno.h
> > > index 8cbc07c1903e..93194fbb0a80 100644
> > > --- a/arch/parisc/include/uapi/asm/errno.h
> > > +++ b/arch/parisc/include/uapi/asm/errno.h
> > > @@ -124,4 +124,6 @@
> > > 
> > >  #define EHWPOISON    257     /* Memory page has hardware error */
> > > 
> > > +#define EFTYPE               258     /* Wrong file type for the intended operation */
> > > +
> > >  #endif
> > > diff --git a/arch/parisc/include/uapi/asm/fcntl.h b/arch/parisc/include/uapi/asm/fcntl.h
> > > index 03dee816cb13..d46812f2f0f4 100644
> > > --- a/arch/parisc/include/uapi/asm/fcntl.h
> > > +++ b/arch/parisc/include/uapi/asm/fcntl.h
> > > @@ -19,6 +19,7 @@
> > > 
> > >  #define O_PATH               020000000
> > >  #define __O_TMPFILE  040000000
> > > +#define OPENAT2_REGULAR      0100000000
> > > 
> > >  #define F_GETLK64    8
> > >  #define F_SETLK64    9
> > > diff --git a/arch/sparc/include/uapi/asm/errno.h b/arch/sparc/include/uapi/asm/errno.h
> > > index 4a41e7835fd5..71940ec9130b 100644
> > > --- a/arch/sparc/include/uapi/asm/errno.h
> > > +++ b/arch/sparc/include/uapi/asm/errno.h
> > > @@ -117,4 +117,6 @@
> > > 
> > >  #define EHWPOISON    135     /* Memory page has hardware error */
> > > 
> > > +#define EFTYPE               136     /* Wrong file type for the intended operation */
> > > +
> > >  #endif
> > > diff --git a/arch/sparc/include/uapi/asm/fcntl.h b/arch/sparc/include/uapi/asm/fcntl.h
> > > index 67dae75e5274..bb6e9fa94bc9 100644
> > > --- a/arch/sparc/include/uapi/asm/fcntl.h
> > > +++ b/arch/sparc/include/uapi/asm/fcntl.h
> > > @@ -37,6 +37,7 @@
> > > 
> > >  #define O_PATH               0x1000000
> > >  #define __O_TMPFILE  0x2000000
> > > +#define OPENAT2_REGULAR      0x4000000
> > > 
> > >  #define F_GETOWN     5       /*  for sockets. */
> > >  #define F_SETOWN     6       /*  for sockets. */
> > > diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> > > index 31b691b2aea2..0a4220f72ada 100644
> > > --- a/fs/ceph/file.c
> > > +++ b/fs/ceph/file.c
> > > @@ -977,6 +977,10 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
> > >                       ceph_init_inode_acls(newino, &as_ctx);
> > >                       file->f_mode |= FMODE_CREATED;
> > >               }
> > > +             if ((flags & OPENAT2_REGULAR) && !d_is_reg(dentry)) {
> > > +                     err = -EFTYPE;
> > > +                     goto out_req;
> > > +             }
> > >               err = finish_open(file, dentry, ceph_open);
> > >       }
> > >  out_req:
> > > diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> > > index 8344040ecaf7..0dc3e4240d9e 100644
> > > --- a/fs/gfs2/inode.c
> > > +++ b/fs/gfs2/inode.c
> > > @@ -749,6 +749,8 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
> > >               if (file) {
> > >                       if (S_ISREG(inode->i_mode))
> > >                               error = finish_open(file, dentry, gfs2_open_common);
> > > +                     else if (file->f_flags & OPENAT2_REGULAR)
> > > +                             error = -EFTYPE;
> > >                       else
> > >                               error = finish_no_open(file, NULL);
> > >               }
> > > diff --git a/fs/namei.c b/fs/namei.c
> > > index 5fe6cac48df8..aa5fb2672881 100644
> > > --- a/fs/namei.c
> > > +++ b/fs/namei.c
> > > @@ -4651,6 +4651,10 @@ static int do_open(struct nameidata *nd,
> > >               if (unlikely(error))
> > >                       return error;
> > >       }
> > > +
> > > +     if ((open_flag & OPENAT2_REGULAR) && !d_is_reg(nd->path.dentry))
> > > +             return -EFTYPE;
> > > +
> > >       if ((nd->flags & LOOKUP_DIRECTORY) && !d_can_lookup(nd->path.dentry))
> > >               return -ENOTDIR;
> > > 
> > > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> > > index b3f5c9461204..ef61db67d06e 100644
> > > --- a/fs/nfs/dir.c
> > > +++ b/fs/nfs/dir.c
> > > @@ -2195,7 +2195,9 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
> > >                       break;
> > >               case -EISDIR:
> > >               case -ENOTDIR:
> > > -                     goto no_open;
> > > +                     if (!(open_flags & OPENAT2_REGULAR))
> > > +                             goto no_open;
> > > +                     break;
> > 
> > Shouldn't this also set the error to -EFTYPE?
> > 
> > >               case -ELOOP:
> > >                       if (!(open_flags & O_NOFOLLOW))
> > >                               goto no_open;
> > > diff --git a/fs/open.c b/fs/open.c
> > > index 91f1139591ab..1524f52a1773 100644
> > > --- a/fs/open.c
> > > +++ b/fs/open.c
> > > @@ -1198,7 +1198,7 @@ inline int build_open_flags(const struct open_how *how, struct open_flags *op)
> > >        * values before calling build_open_flags(), but openat2(2) checks all
> > >        * of its arguments.
> > >        */
> > > -     if (flags & ~VALID_OPEN_FLAGS)
> > > +     if (flags & ~VALID_OPENAT2_FLAGS)
> > >               return -EINVAL;
> > >       if (how->resolve & ~VALID_RESOLVE_FLAGS)
> > >               return -EINVAL;
> > > @@ -1237,6 +1237,8 @@ inline int build_open_flags(const struct open_how *how, struct open_flags *op)
> > >                       return -EINVAL;
> > >               if (!(acc_mode & MAY_WRITE))
> > >                       return -EINVAL;
> > > +     } else if ((flags & O_DIRECTORY) && (flags & OPENAT2_REGULAR)) {
> > > +             return -EINVAL;
> > >       }
> > >       if (flags & O_PATH) {
> > >               /* O_PATH only permits certain other flags to be set. */
> > > diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
> > > index cb10088197d2..d12ed0c87599 100644
> > > --- a/fs/smb/client/dir.c
> > > +++ b/fs/smb/client/dir.c
> > > @@ -236,6 +236,11 @@ static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned
> > >                                * lookup.
> > >                                */
> > >                               CIFSSMBClose(xid, tcon, fid->netfid);
> > > +                             if (oflags & OPENAT2_REGULAR) {
> > > +                                     iput(newinode);
> > > +                                     rc = -EFTYPE;
> > > +                                     goto out;
> > > +                             }
> > >                               goto cifs_create_get_file_info;
> > >                       }
> > >                       /* success, no need to query */
> > > @@ -433,11 +438,15 @@ static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned
> > >               goto out_err;
> > >       }
> > > 
> > > -     if (newinode)
> > > +     if (newinode) {
> > >               if (S_ISDIR(newinode->i_mode)) {
> > >                       rc = -EISDIR;
> > >                       goto out_err;
> > 
> > This logic doesn't look quite right. If you do a create and race with a
> > directory create, then it looks like you'll send back -EISDIR here
> > instead of -EFTYPE?
> > 
> 
> Yes, I thought it would be useful for the caller to know what type of
> file it actually is so I had kept this the same. Let me know if it is
> preferable to return -EFTYPE always.
> 
> Thanks for reviewing!
> 
> Regards,
> Dorjoy

-- 
Jeff Layton <jlayton@kernel.org>

  reply	other threads:[~2026-03-01 14:47 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-21 14:45 [PATCH v4 0/4] OPENAT2_REGULAR flag support in openat2 Dorjoy Chowdhury
2026-02-21 14:45 ` [PATCH v4 1/4] openat2: new OPENAT2_REGULAR flag support Dorjoy Chowdhury
2026-03-01 12:44   ` Jeff Layton
2026-03-01 14:16     ` Dorjoy Chowdhury
2026-03-01 14:47       ` Jeff Layton [this message]
2026-03-01 15:01         ` Dorjoy Chowdhury
2026-03-01 15:10           ` Jeff Layton
2026-03-01 15:15             ` Dorjoy Chowdhury
2026-03-01 15:17               ` Jeff Layton
2026-03-01 15:19                 ` Dorjoy Chowdhury
2026-02-21 14:45 ` [PATCH v4 2/4] kselftest/openat2: test for OPENAT2_REGULAR flag Dorjoy Chowdhury
2026-02-21 14:45 ` [PATCH v4 3/4] sparc/fcntl.h: convert O_* flag macros from hex to octal Dorjoy Chowdhury
2026-02-21 14:45 ` [PATCH v4 4/4] mips/fcntl.h: " Dorjoy Chowdhury
2026-02-21 15:49 ` [PATCH v4 0/4] OPENAT2_REGULAR flag support in openat2 Dorjoy Chowdhury
2026-02-21 23:45   ` Askar Safin
2026-02-22  7:19     ` Dorjoy Chowdhury
2026-03-01 10:16 ` Dorjoy Chowdhury

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=ed5aeaa81ad9b87926fa7ebee0308aeb8df9f0ac.camel@kernel.org \
    --to=jlayton@kernel.org \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=adilger@dilger.ca \
    --cc=agruenba@redhat.com \
    --cc=alex.aring@gmail.com \
    --cc=amarkuze@redhat.com \
    --cc=andreas@gaisler.com \
    --cc=anna@kernel.org \
    --cc=arnd@arndb.de \
    --cc=bharathsm@microsoft.com \
    --cc=brauner@kernel.org \
    --cc=ceph-devel@vger.kernel.org \
    --cc=chuck.lever@oracle.com \
    --cc=davem@davemloft.net \
    --cc=deller@gmx.de \
    --cc=dorjoychy111@gmail.com \
    --cc=gfs2@lists.linux.dev \
    --cc=hansg@kernel.org \
    --cc=idryomov@gmail.com \
    --cc=jack@suse.cz \
    --cc=linmag7@gmail.com \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=mattst88@gmail.com \
    --cc=miklos@szeredi.hu \
    --cc=mjguzik@gmail.com \
    --cc=pc@manguebit.org \
    --cc=richard.henderson@linaro.org \
    --cc=ronniesahlberg@gmail.com \
    --cc=sfrench@samba.org \
    --cc=shuah@kernel.org \
    --cc=slava@dubeyko.com \
    --cc=smfrench@gmail.com \
    --cc=sprasad@microsoft.com \
    --cc=tom@talpey.com \
    --cc=trondmy@kernel.org \
    --cc=tsbogend@alpha.franken.de \
    --cc=v9fs@lists.linux.dev \
    --cc=viro@zeniv.linux.org.uk \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox