public inbox for linux-fsdevel@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,
	 linux-api@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 v6 1/4] openat2: new OPENAT2_REGULAR flag support
Date: Wed, 01 Apr 2026 15:02:47 -0400	[thread overview]
Message-ID: <4385168f2147efb8131d5fe4209e88d2d15a60bf.camel@kernel.org> (raw)
In-Reply-To: <CAFfO_h75dF2s83VNtUaNuRmto1NVVcxo7kN6eAtNtN3ME8mPiQ@mail.gmail.com>

On Mon, 2026-03-30 at 21:07 +0600, Dorjoy Chowdhury wrote:
> On Mon, Mar 30, 2026 at 5:49 PM Jeff Layton <jlayton@kernel.org> wrote:
> > 
> > On Sat, 2026-03-28 at 23:22 +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. EFTYPE is already used in BSD systems
> > > like FreeBSD, macOS.
> > > 
> > > 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.
> > > 
> > > 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/fcntl.c                                 |  4 ++--
> > >  fs/gfs2/inode.c                            |  6 ++++++
> > >  fs/namei.c                                 |  4 ++++
> > >  fs/nfs/dir.c                               |  4 ++++
> > >  fs/open.c                                  |  8 +++++---
> > >  fs/smb/client/dir.c                        | 14 +++++++++++++-
> > >  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 ++
> > >  22 files changed, 67 insertions(+), 6 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 66bbf6d517a9..6d8d4c7765e6 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;
> > > +             }
> > 
> > ^^^
> > This doesn't look quite right. Here's a larger chunk of the code:
> > 
> > -------------------------8<--------------------------
> >         if (d_in_lookup(dentry)) {
> >                 dn = ceph_finish_lookup(req, dentry, err);
> >                 if (IS_ERR(dn))
> >                         err = PTR_ERR(dn);
> >         } else {
> >                 /* we were given a hashed negative dentry */
> >                 dn = NULL;
> >         }
> >         if (err)
> >                 goto out_req;
> >         if (dn || d_really_is_negative(dentry) || d_is_symlink(dentry)) {
> >                 /* make vfs retry on splice, ENOENT, or symlink */
> >                 doutc(cl, "finish_no_open on dn %p\n", dn);
> >                 err = finish_no_open(file, dn);
> >         } else {
> >                 if (IS_ENCRYPTED(dir) &&
> >                     !fscrypt_has_permitted_context(dir, d_inode(dentry))) {
> >                         pr_warn_client(cl,
> >                                 "Inconsistent encryption context (parent %llx:%llx child %llx:%llx)\n",
> >                                 ceph_vinop(dir), ceph_vinop(d_inode(dentry)));
> >                         goto out_req;
> >                 }
> > 
> >                 doutc(cl, "finish_open on dn %p\n", dn);
> >                 if (req->r_op == CEPH_MDS_OP_CREATE && req->r_reply_info.has_create_ino) {
> >                         struct inode *newino = d_inode(dentry);
> > 
> >                         cache_file_layout(dir, newino);
> >                         ceph_init_inode_acls(newino, &as_ctx);
> >                         file->f_mode |= FMODE_CREATED;
> >                 }
> >                 err = finish_open(file, dentry, ceph_open);
> >         }
> > -------------------------8<--------------------------
> > 
> > It looks like this won't handle it correctly if the pathwalk terminates
> > on a symlink (re: d_is_symlink() case). You should either set up a test
> > ceph cluster on your own, or reach out to the ceph community and ask
> > them to test this.
> > 
> 
> Thanks for reviewing. The d_is_symlink() case seems to be calling
> finish_no_open so shouldn't this be okay?
> 

My mistake -- you're correct. I keep forgetting that finish_no_open()
will handle this case regardless of what else happens.

> > >               err = finish_open(file, dentry, ceph_open);
> > >       }
> > >  out_req:
> > > diff --git a/fs/fcntl.c b/fs/fcntl.c
> > > index beab8080badf..240bb511557a 100644
> > > --- a/fs/fcntl.c
> > > +++ b/fs/fcntl.c
> > > @@ -1169,9 +1169,9 @@ static int __init fcntl_init(void)
> > >        * Exceptions: O_NONBLOCK is a two bit define on parisc; O_NDELAY
> > >        * is defined as O_NONBLOCK on some platforms and not on others.
> > >        */
> > > -     BUILD_BUG_ON(20 - 1 /* for O_RDONLY being 0 */ !=
> > > +     BUILD_BUG_ON(21 - 1 /* for O_RDONLY being 0 */ !=
> > >               HWEIGHT32(
> > > -                     (VALID_OPEN_FLAGS & ~(O_NONBLOCK | O_NDELAY)) |
> > > +                     (VALID_OPENAT2_FLAGS & ~(O_NONBLOCK | O_NDELAY)) |
> > >                       __FMODE_EXEC));
> > > 
> > >       fasync_cache = kmem_cache_create("fasync_cache",
> > > diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> > > index 8344040ecaf7..4604e2e8a9cc 100644
> > > --- a/fs/gfs2/inode.c
> > > +++ b/fs/gfs2/inode.c
> > > @@ -738,6 +738,12 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
> > >       inode = gfs2_dir_search(dir, &dentry->d_name, !S_ISREG(mode) || excl);
> > >       error = PTR_ERR(inode);
> > >       if (!IS_ERR(inode)) {
> > > +             if (file && (file->f_flags & OPENAT2_REGULAR) && !S_ISREG(inode->i_mode)) {
> > 
> > Isn't OPENAT2_REGULAR getting masked off in ->f_flags now?
> > 
> Yes, I thought the masking off was happening after this codepath got
> executed. Maybe it's better anyway to pass another flags param to this
> function and forward the flags from the gfs2_atomic_open function and
> in other call sites pass 0 ? What do you think?
> 

Also my mistake. That happens in do_dentry_open() which happens in
finish_open(), so you should be OK here.

Reviewed-by: Jeff Layton <jlayton@kernel.org>

  reply	other threads:[~2026-04-01 19:02 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-28 17:22 [PATCH v6 0/4] OPENAT2_REGULAR flag support for openat2 Dorjoy Chowdhury
2026-03-28 17:22 ` [PATCH v6 1/4] openat2: new OPENAT2_REGULAR flag support Dorjoy Chowdhury
2026-03-30 11:49   ` Jeff Layton
2026-03-30 15:07     ` Dorjoy Chowdhury
2026-04-01 19:02       ` Jeff Layton [this message]
2026-04-04 15:17         ` Dorjoy Chowdhury
2026-04-05 23:27           ` Jeff Layton
2026-04-06 15:30             ` Dorjoy Chowdhury
2026-03-28 17:22 ` [PATCH v6 2/4] kselftest/openat2: test for OPENAT2_REGULAR flag Dorjoy Chowdhury
2026-03-28 17:22 ` [PATCH v6 3/4] sparc/fcntl.h: convert O_* flag macros from hex to octal Dorjoy Chowdhury
2026-03-28 17:22 ` [PATCH v6 4/4] mips/fcntl.h: " 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=4385168f2147efb8131d5fe4209e88d2d15a60bf.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-api@vger.kernel.org \
    --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