Re: [RFC PATCH v5 1/2] vfs: add O_CREAT|O_DIRECTORY to open*(2)

[Jori Koolstra]

> Op 27-05-2026 09:27 CEST schreef NeilBrown <neilb@xxxxxxxxxxx>:
>
> > @@ -1268,6 +1269,8 @@ inline int build_open_flags(const struct open_how *how, struct open_flags *op)
> >  	op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
> >  
> >  	if (flags & O_CREAT) {
> > +		if ((flags & O_DIRECTORY) && (acc_mode & MAY_WRITE))
> > +			return -EISDIR;
> 
> It seems odd that the MAY_WRITE test is only performed for O_CREAT.
> Shouldn't any open of O_DIRECTORY|O_WRONLY fail, and shouldn't the one
> test catch both create and non-create cases?
> 

I get your point, it does look odd. But AFAIS O_DIRECTORY|O_WRONLY is currently only blocked
after path resolution and final lookup. So that was the behavior I tried to keep. On the
other hand, right now O_DIRECTORY|O_CREAT is already blocked in build_open_flags().

But if that is true, and you have atomic_open() defined for your filesystem, then you
can lookup and open a directory with O_DIRECTORY|O_WRONLY, while it fails with EISDIR later
(if the fs atomic_open() does not error on O_DIRECTORY|O_WRONLY, which I would guess it
mostly does). It will then fput_close() the file, so there is no leak, but I don't think
that is wanted behavior.

@Christian, what do you think?

> 
> >  		op->intent |= LOOKUP_CREATE;
> >  		if (flags & O_EXCL) {
> >  			op->intent |= LOOKUP_EXCL;
> > diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
> > index e4295a5b55b3..ec8c54c91261 100644
> > --- a/fs/smb/client/dir.c
> > +++ b/fs/smb/client/dir.c
> > @@ -526,6 +526,9 @@ int cifs_atomic_open(struct inode *dir, struct dentry *direntry,
> >  	if (unlikely(cifs_forced_shutdown(cifs_sb)))
> >  		return smb_EIO(smb_eio_trace_forced_shutdown);
> >  
> > +	if ((oflags & O_MKDIR_MASK) == O_MKDIR_MASK)
> > +		return -EINVAL;
> > +
> >  	/*
> >  	 * Posix open is only called (at lookup time) for file create now. For
> >  	 * opens (rather than creates), because we do not know if it is a file
> > diff --git a/fs/vboxsf/dir.c b/fs/vboxsf/dir.c
> > index 42bedc4ec7af..aef5ca6730be 100644
> > --- a/fs/vboxsf/dir.c
> > +++ b/fs/vboxsf/dir.c
> > @@ -318,6 +318,9 @@ static int vboxsf_dir_atomic_open(struct inode *parent, struct dentry *dentry,
> >  	u64 handle;
> >  	int err;
> >  
> > +	if ((flags & O_MKDIR_MASK) == O_MKDIR_MASK)
> > +		return -EINVAL;
> > +
> >  	if (d_in_lookup(dentry)) {
> >  		struct dentry *res = vboxsf_dir_lookup(parent, dentry, 0);
> >  		if (res || d_really_is_positive(dentry))
> > diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
> > index a332e79b3207..e31f3a57f07c 100644
> > --- a/include/linux/fcntl.h
> > +++ b/include/linux/fcntl.h
> > @@ -12,6 +12,8 @@
> >  	 FASYNC	| O_DIRECT | O_LARGEFILE | O_DIRECTORY | O_NOFOLLOW | \
> >  	 O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE)
> >  
> > +#define O_MKDIR_MASK	(O_CREAT | O_DIRECTORY)
> > +
> >  /* List of all valid flags for the how->resolve argument: */
> >  #define VALID_RESOLVE_FLAGS \
> >  	(RESOLVE_NO_XDEV | RESOLVE_NO_MAGICLINKS | RESOLVE_NO_SYMLINKS | \
> > -- 
> > 2.54.0
> > 
> > 
> > 
> 
> Thanks,
> NeilBrown

There is another point, I maybe should have mentioned in the cover letter: I have not attempted
to handle dangling symlinks for O_MKDIR. Not because I think they are a great idea (as Aleksa
has mentioned, but I am not very familiar with the dragons it entails), but I wanted to discuss
what behavior we want in this case. Do we say that we never do a mkdir after following a lookup
last symlink? I don't think that state is even recorded right now.

Best,
Jori.