ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
34.37k stars 2.51k forks source link

std.os.access doesn't handle EPERM errno #19162

Open mrschyte opened 7 months ago

mrschyte commented 7 months ago

Zig Version

0.11.0

Steps to Reproduce and Observed Behavior

1) Create file with the immutable attribute set: $ touch test && sudo chattr +i test 2) Call std.os.access("test", std.os.W_OK); 3) Zig program dumps stack trace with: unexpected errno: 1

Some man pages list this errno as a possible return value (https://man7.org/linux/man-pages/man2/access.2.html), while others don't (https://linux.die.net/man/2/access); which could be the reason why zig currently doesn't handle this case.

Expected Behavior

Instead of returning unexpectedErrno, the std.os.accessZ function should return an error.PermissionDenied error when encountering EPERM.

cactusbento commented 7 months ago

I took a cursory look through some man pages for access(2) and found that OpenBSD and man7 lists EPERM as possible errors. Debian and Arch Linux don't list EPERM.

In both OpenBSD and man7, EPERM is related to the immutable flag of a file (set by chflags or ioctl_iflags).

Maybe error.FileImmutable should be added to os.AccessError. Although, since EPERM is a permission error, error.PermissionDenied could be sufficient.

mrschyte commented 7 months ago

Adding error.FileImmutable seems like a sensible approach if nothing else can trigger the EPERM errno. Looking at the linux kernel sources, this seems to be the case.

The do_faccessat function invokes inode_permission which returns the permission error https://github.com/torvalds/linux/blob/90d35da658da8cff0d4ecbb5113f5fac9d00eb72/fs/open.c#L504C8-L504C24

https://github.com/torvalds/linux/blob/90d35da658da8cff0d4ecbb5113f5fac9d00eb72/fs/namei.c#L517

Maybe we could check where else inode_permission is called from to discover other places where a FileImmutable error should be returned.

garrisonhh commented 7 months ago

AccessError.PermissionDenied corresponds to EACCESS:

EACCES The requested access would be denied to the file, or search permission is denied for one of the directories in the path prefix of pathname. (See also path_resolution(7).

I think this is distinct enough from the semantics of EPERM to demand a separate error

garrisonhh commented 7 months ago

Actually, looking at functions like mmap, EACCES corresponds to MMapError.AccessDenied while EPERM corresponds to MMAPError.PermissionDenied. It would be more consistent to make AccessError match the set precedent