Re: [PATCH] vfs: missing inode operation should return a consistent error code

[Jan Kara]

On Sun 31-05-26 10:08:47, Jeff Layton wrote:
> On Sun, 2026-05-31 at 12:49 +0200, Jori Koolstra wrote:
> > Currently several different error codes are used in the VFS for
> > situations where the underlying filesystem does not support the
> > requested inode operation (such as mkdir, tmpfile, create, etc.)
> > Examples: create returns EACCES, mkdir EPERM, tmpfile EOPNOTSUPP,
> > fileattr_get ENOIOCTLCMD.
> > 
> > We should provide a sensible unified error code for these situations.
> > EOPNOTSUPP is already used for this both in the kernel (when lacking
> > tmpfile support) and in userland (e.g. glibc).[1] Restricting EOPNOTSUPP
> > to socket operations as POSIX suggests is not the current reality and
> > this was recently changed in the man page as well.[2]
> > 
> > vfs_fileattr_get|set return ENOIOCTLCMD, but this cannot be changed
> > since EOPNOTSUPP is already used to by underlying filesystems to indicate
> > that a flag is not supported. The change to EOPNOTSUPP was reverted by
> > 4dd5b5ac089b ("Revert "fs: make vfs_fileattr_[get|set] return
> > -EOPNOTSUPP"")
> > 
> > [1]: https://lore.kernel.org/all/20260528-abnimmt-befreien-perspektive-a7930659fb40@brauner/
> > [2]: https://lore.kernel.org/linux-fsdevel/ahd3SmZZqnzP0-O2@devuan/T/#t
> > 
> > Signed-off-by: Jori Koolstra <jkoolstra@xxxxxxxxx>
> > ---
> >  fs/namei.c                       | 18 +++++++++---------
> >  include/uapi/asm-generic/errno.h |  2 +-
> >  2 files changed, 10 insertions(+), 10 deletions(-)
> > 
> > diff --git a/fs/namei.c b/fs/namei.c
> > index c7fac83c9a85..813419c340ad 100644
> > --- a/fs/namei.c
> > +++ b/fs/namei.c
> > @@ -4192,7 +4192,7 @@ int vfs_create(struct mnt_idmap *idmap, struct dentry *dentry, umode_t mode,
> >  		return error;
> >  
> >  	if (!dir->i_op->create)
> > -		return -EACCES;	/* shouldn't it be ENOSYS? */
> > +		return -EOPNOTSUPP;
> >  
> >  	mode = vfs_prepare_mode(idmap, dir, mode, S_IALLUGO, S_IFREG);
> >  	error = security_inode_create(dir, dentry, mode);
> > @@ -4504,7 +4504,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
> >  		file->f_mode |= FMODE_CREATED;
> >  		audit_inode_child(dir_inode, dentry, AUDIT_TYPE_CHILD_CREATE);
> >  		if (!dir_inode->i_op->create) {
> > -			error = -EACCES;
> > +			error = -EOPNOTSUPP;
> >  			goto out_dput;
> >  		}
> >  
> > @@ -5102,7 +5102,7 @@ int vfs_mknod(struct mnt_idmap *idmap, struct inode *dir,
> >  		return -EPERM;
> >  
> >  	if (!dir->i_op->mknod)
> > -		return -EPERM;
> > +		return -EOPNOTSUPP;
> >  
> >  	mode = vfs_prepare_mode(idmap, dir, mode, mode, mode);
> >  	error = devcgroup_inode_mknod(mode, dev);
> > @@ -5241,7 +5241,7 @@ struct dentry *vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
> >  	if (error)
> >  		goto err;
> >  
> > -	error = -EPERM;
> > +	error = -EOPNOTSUPP;
> >  	if (!dir->i_op->mkdir)
> >  		goto err;
> >  
> > @@ -5345,7 +5345,7 @@ int vfs_rmdir(struct mnt_idmap *idmap, struct inode *dir,
> >  		return error;
> >  
> >  	if (!dir->i_op->rmdir)
> > -		return -EPERM;
> > +		return -EOPNOTSUPP;
> >  
> >  	dget(dentry);
> >  	inode_lock(dentry->d_inode);
> > @@ -5479,7 +5479,7 @@ int vfs_unlink(struct mnt_idmap *idmap, struct inode *dir,
> >  		return error;
> >  
> >  	if (!dir->i_op->unlink)
> > -		return -EPERM;
> > +		return -EOPNOTSUPP;
> >  
> >  	inode_lock(target);
> >  	if (IS_SWAPFILE(target))
> > @@ -5630,7 +5630,7 @@ int vfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
> >  		return error;
> >  
> >  	if (!dir->i_op->symlink)
> > -		return -EPERM;
> > +		return -EOPNOTSUPP;
> >  
> >  	error = security_inode_symlink(dir, dentry, oldname);
> >  	if (error)
> > @@ -5752,7 +5752,7 @@ int vfs_link(struct dentry *old_dentry, struct mnt_idmap *idmap,
> >  	if (HAS_UNMAPPED_ID(idmap, inode))
> >  		return -EPERM;
> >  	if (!dir->i_op->link)
> > -		return -EPERM;
> > +		return -EOPNOTSUPP;
> >  	if (S_ISDIR(inode->i_mode))
> >  		return -EPERM;
> >  
> > @@ -5961,7 +5961,7 @@ int vfs_rename(struct renamedata *rd)
> >  		return error;
> >  
> >  	if (!old_dir->i_op->rename)
> > -		return -EPERM;
> > +		return -EOPNOTSUPP;
> >  
> >  	/*
> >  	 * If we are going to change the parent - check write permissions,
> > diff --git a/include/uapi/asm-generic/errno.h b/include/uapi/asm-generic/errno.h
> > index 92e7ae493ee3..7b5b71ae1b12 100644
> > --- a/include/uapi/asm-generic/errno.h
> > +++ b/include/uapi/asm-generic/errno.h
> > @@ -76,7 +76,7 @@
> >  #define	ENOPROTOOPT	92	/* Protocol not available */
> >  #define	EPROTONOSUPPORT	93	/* Protocol not supported */
> >  #define	ESOCKTNOSUPPORT	94	/* Socket type not supported */
> > -#define	EOPNOTSUPP	95	/* Operation not supported on transport endpoint */
> > +#define	EOPNOTSUPP	95	/* Operation not supported */
> >  #define	EPFNOSUPPORT	96	/* Protocol family not supported */
> >  #define	EAFNOSUPPORT	97	/* Address family not supported by protocol */
> >  #define	EADDRINUSE	98	/* Address already in use */
> > 
> > base-commit: 670b77dfebe7257adc0defbc48a4c43cfdf6c8f6
> 
> This seems ill-advised. The problem is that most of this behavior has
> been well established for years, and not all userland software (and
> even some internal callers like nfsd), will react well when you go
> changing behavior like this.
> 
> As a case in point, the POSIX spec doesn't list EOPNOTSUPP as a valid
> error return for open():
> 
>    https://pubs.opengroup.org/onlinepubs/9690949399/functions/open.html
> 
> The manpage for open() says only:
> 
>        EOPNOTSUPP
>               The filesystem containing pathname does not support O_TMPFILE.
> 
> What is the poor userland developer to make of this when open() starts
> returning EOPNOTSUPP without O_TMPFILE being specified?

So I wouldn't care as much about POSIX, we largely ignore it anyway in the
kernel. But I tend to agree that changing the error code we returned for
several decades just for the sake of "cleanliness" isn't IMHO a good enough
reason to risk breaking userspace or causing confusion.

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR