On Wednesday 12 February 2025 19:19:00 Paulo Alcantara wrote: > Pali Rohár <pali@xxxxxxxxxx> writes: > > > On Wednesday 12 February 2025 17:49:31 Paulo Alcantara wrote: > >> Steve, > >> > >> The commit 438e2116d7bd ("cifs: Change translation of > >> STATUS_PRIVILEGE_NOT_HELD to -EPERM") regressed getcifsacl(1) because it > >> expects -EIO to be returned from getxattr(2) when the client can't read > >> system.cifs_ntsd_full attribute and then fall back to system.cifs_acl > >> attribute. Either -EIO or -EPERM is wrong for getxattr(2), but that's a > >> different problem, though. > >> > >> Reproduced against samba-4.22 server. > > > > That is bad. I can prepare a fix for cifs.ko getxattr syscall to > > translate -EPERM to -EIO. This will ensure that getcifsacl will work as > > before as it would still see -EIO error. > > Sounds good. > > > But as discussed before, we need to distinguish between > > privilege/permission error and other generic errors (access/io). > > So I think that we need 438e2116d7bd commit. > > OK. > > > Based on linux-fsdevel discussion it is a good idea to distinguish > > between errors by mapping status codes to appropriate posix errno, and > > then updating linux syscall manpages. > > Either way, we shouldn't be leaking -EIO or -EPERM to userland from > getxattr(2). By looking at the man pages, -ENODATA seems to be the > appropriate error to return instead. It looks like there are missing error codes for getxattr. Because any path based syscall can return -EACCES if trying to open path to which calling process does not have access. And EACCES is not mentioned nor documented in getxattr(2). Same applies for listxattr(2). Now I have tried listxattr() and it really returns EACCES for /root/file called by nobody. -EIO is generic I/O error. And I think that this error code could be returned by any I/O syscall when unknown I/O error occurs. Returning -ENODATA for generic or unknown I/O error is a bad idea because ENODATA (= ENOATTR) has already specific meaning when attribute does not exists at all (or process does not have access to it). For me it makes sense to return -EIO and -EPERM by those syscalls. But for getxattr() we cannot do it due that backward compatibility needed by getcifsacl application.