On Thu, Mar 25, 2021 at 03:52:28PM +0100, Miklos Szeredi wrote: > On Tue, Mar 23, 2021 at 1:24 AM Eric Biggers <[email protected]> wrote: > > > > +int vfs_miscattr_set(struct dentry *dentry, struct miscattr *ma) > > > +{ > > > + struct inode *inode = d_inode(dentry); > > > + struct miscattr old_ma = {}; > > > + int err; > > > + > > > + if (d_is_special(dentry)) > > > + return -ENOTTY; > > > + > > > + if (!inode->i_op->miscattr_set) > > > + return -ENOIOCTLCMD; > > > + > > > + if (!inode_owner_or_capable(inode)) > > > + return -EPERM; > > > > Shouldn't this be EACCES, not EPERM? > > $ git diff master.. | grep -C1 inode_owner_or_capable | grep > "^-.*\(EPERM\|EACCES\)" | cut -d- -f3 | sort | uniq -c > 12 EACCES; > 4 EPERM; > > So EACCES would win if this was a democracy. However: > > "[EACCES] > Permission denied. An attempt was made to access a file in a way > forbidden by its file access permissions." > > "[EPERM] > Operation not permitted. An attempt was made to perform an operation > limited to processes with appropriate privileges or to the owner of a > file or other resource." > > The EPERM description matches the semantics of > inode_owner_or_capable() exactly. It's a pretty clear choice.
Except that existing implementation (e.g. for ext2) gives -EACCES here... OTOH, EPERM matches the behaviour of chown(2), as well as that of *BSD chflags(2), which is the best match to functionality (setting and clearing immutable/append-only/etc.) So I'd probably go with EPERM, and watched for userland breakage; if something *does* rely upon the historical EACCES here, we might have to restore that.
