Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

error.PermissionDenied vs error.AccessDenied #16782

Open
squeek502 opened this issue Aug 12, 2023 · 5 comments · May be fixed by #23007
Open

error.PermissionDenied vs error.AccessDenied #16782

squeek502 opened this issue Aug 12, 2023 · 5 comments · May be fixed by #23007
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@squeek502
Copy link
Collaborator

squeek502 commented Aug 12, 2023

These two error names are common through the Zig codebase, but they seem to be used pretty much interchangeably and don't seem to ever be used to convey unique information, so it seems like one canonical version should be chosen and used everywhere (or a distinction should be made about when to use one over another).

Some evidence for why I think it might make sense to merge them:

In terms of which is more common:

  • There are 50 instances of return error.PermissionDenied
  • There are 144 instances of return error.AccessDenied
@nektro
Copy link
Contributor

nektro commented Aug 12, 2023

mismatch seems inherited

EPERM 1 Operation not permitted
EACCES 13 Permission denied

@jacobly0 jacobly0 added standard library This issue involves writing Zig code for the standard library. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. labels Aug 12, 2023
@jacobly0 jacobly0 added this to the 0.13.0 milestone Aug 12, 2023
@squeek502
Copy link
Collaborator Author

There is no explicit error set that I can find that includes both PermissionDenied and AccessDenied

I was wrong. There's at least one:

zig/lib/std/os.zig

Lines 5591 to 5610 in e078324

pub const FutimensError = error{
/// times is NULL, or both tv_nsec values are UTIME_NOW, and either:
/// * the effective user ID of the caller does not match the owner
/// of the file, the caller does not have write access to the
/// file, and the caller is not privileged (Linux: does not have
/// either the CAP_FOWNER or the CAP_DAC_OVERRIDE capability);
/// or,
/// * the file is marked immutable (see chattr(1)).
AccessDenied,
/// The caller attempted to change one or both timestamps to a value
/// other than the current time, or to change one of the timestamps
/// to the current time while leaving the other timestamp unchanged,
/// (i.e., times is not NULL, neither tv_nsec field is UTIME_NOW,
/// and neither tv_nsec field is UTIME_OMIT) and either:
/// * the caller's effective user ID does not match the owner of
/// file, and the caller is not privileged (Linux: does not have
/// the CAP_FOWNER capability); or,
/// * the file is marked append-only or immutable (see chattr(1)).
PermissionDenied,

@ok-ryoko
Copy link
Contributor

ok-ryoko commented Feb 24, 2024

I’m writing Zig bindings to a C library for a Linux system administration application—this is the lens through which I’m viewing this issue.

I want to keep C code and errno out of the main application code, so I’ve defined a single error set type to encompass the errors that the C library functions can return out of band via errno. In my case, errno can be set to EACCES but not EPERM, EPERM but not EACCES, or either EACCES or EPERM depending on the function that was called.

EACCES and EPERM come from the POSIX.1-2001 specification, which has this to say:

[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 implication is that EACCES is specific to the permissions on a given file and EPERM indicates a generic deficiency of privilege that may not have anything to do with files, e.g., calling setuid(2) to become an arbitrary user without CAP_SETUID. I could make the argument that a failure to access a file due to insufficient permissions is an instance of an operation that isn’t permitted and that there is therefore no need to distinguish between the two cases.

However, the distinction above turns out to be meaningful for the purpose of troubleshooting my (privileged) application. Thus, I’d like to be able to represent both constants in my error set type. How should I do that? Knowing that, at an upper layer of my application, I will be uniting my library-specific error set type with syscall-specific error set types in std.os, I would rather use the same representations that the standard library does for EACCES and EPERM so as not to end up with redundant errors. Currently, error.AccessDenied and error.PermissionDenied, respectively, serve this role.

If there were only one representation for both EACCES and EPERM in the standard library, then I would want to introduce my own representation for one of EACCES and EPERM to maintain the distinction. However, I may not choose the same representation as the author of another module that is also checking errno. If I should happen to include their module as a dependency, I may find myself writing boilerplate code to unify multiple representations of what is canonically one thing.

Allow me to give a more concrete example where compressing the distinction away can be undesirable: the bpf(2) syscall. Since bpf(2) is a Linux-specific syscall, there’s no corresponding API in std.os—users wanting to leverage an idiomatic Zig interface to std.os.linux.bpf that returns an error union type will need to write their own. They will then encounter the following situation: Not only can bpf(2) set errno to EACCES and EPERM, it uses EACCES to communicate a specific error that has nothing to do with file access permissions:

EACCES For BPF_PROG_LOAD, even though all program instructions are valid, the program has been rejected because it was deemed unsafe. This may be because it may have accessed a disallowed memory region or an uninitialized stack/register or because the function constraints don’t match the actual types or because there was a misaligned memory access. In this case, it is recommended to call bpf() again with log_level = 1 and examine log_buf for the specific reason provided by the verifier.

Meanwhile, EPERM is used in the expected manner—to convey the absence of privileges needed to call bpf(2) in the first place:

EPERM The call was made without sufficient privilege (without the CAP_SYS_ADMIN capability).

I believe that the standard library should equip users with the constructs they need to handle situations such as these should they deem it meaningful for their use cases.

If anything, my suggestion is to rename PermissionDenied to OperationNotPermitted to help dispel conflation with AccessDenied. The current nomenclature of AccessDenied and PermissionDenied confuses me to the same degree as the traditional perror(3) output (“Permission denied” and “Operation not permitted”, respectively).

Nomenclature notwithstanding, I agree that an effort should be made to ensure a consistent mapping from E.ACCES to error.AccessDenied and from E.PERM to error.PermissionDenied across the standard library for the reasons given above.

@garrisonhh
Copy link
Contributor

Thanks for the excellent analysis, all! It seems to me that what this discussion boils down to is that std.os needs to choose whether to consistently respect the errno itself or its perror description in translating errnos to zig errors.

I would argue that using the errno to decide error names would be more intuitive for anyone familiar with developing for *nix, would make porting code to zig easier, and minimize the future role of std.os maintainers. I second error.AccessDenied for EACCES and error.PermissionDenied for EPERM simply because they are already widely used.

@rootbeer
Copy link
Contributor

I spent a little time trying to clean this up (my fix for #22733 got out of hand). As noted above by @ok-ryoko, POSIX defines EACCES and EPERM for disjoint sets of failures (EACCES is generally for file-mode "user/group/other rwx" bits disallowing an operation, and EPERM is for other reasons like immutable files, or SELinux or capabilities, etc.) To me, this means the Zig std.posix.* code should return error.AccessDenied for EACCES and error.PermissionDenied for EPERM. This is a relatively direct set of changes to the current code. An alternative change would be for Zig to declare the distinction is not worth maintaining and always map EPERM to error.AccessDenied (or always map EACCES to error.PermissionDenied).

(As a use case for thinking about this, consider a directory with a regular file, another file you're not allowed to write (due to file permissions), and another file you're not allowed to write because its immutable -- opening the first file for writing should succeed, the second returns EACCES and the third (on Linux) returns EPERM.)

There is a bit of Zig Windows-only code that maps the Windows ACCESS_DENIED error code. That often gets mapped to error.PermissionDenied, which seems wrong to me. But it depends on what the Zig's definition of PermissionDenied is. To me, it seems like that on Windows a syscall that fails with the Windows ACCESS_DENIED should map to a Zig error.AccessDenied. Again this is relatively straight-forward to make consistent (whichever way). Importantly, Windows does not seem to make the ACCES/PERM distinction that POSIX does.

The harder problem I ran into is how this impacts Zig's std.fs.* APIs. It seems reasonable to me that Zig defines the std.fs.* APIs to return consistent error codes, independent of the host platform. Which to me implies error.AccessDenied gets returned consistently, and this layer maps any error.PermissionDenied errors into error.AccessDenied. Alternatively, we could say these APIs may return error.PermissionDenied, but that would only happen in practice on POSIX platforms (yet the compiler would require every caller on any platform to handle this error). Doing the actual code changes to catch one error and map it into another is straightforward, but this badly breaks all the "inherited" error sets that the std.fs.* APIs depend on. The error sets at std.fs are often just a renamed std.posix error set. E.g., std.fs.File.OpenError is defined as pub const OpenError = error{...} || posix.OpenError || ...; As far as I can tell the only way to remove an error (e.g., error.PermissionDenied in this case) is to copy+paste+edit the posix.OpenError list. Forking all those error lists seems a bit error prone, but it is probably the correct thing to do if the std.fs.* errors are merely similar to the underlying POSIX errors, and not identical.

Fundamentally, its not clear to me how tightly Zig error codes map to platform errno numbers. If its 1:1 then std.posix.* should return 1:1 mappings from errno to error (and Windows-only APIs should return Windows-ish errors?), right? But then higher level Zig APIs like std.fs.* and std.os.* probably need distinct errors for their cross-platform, slightly higher-level semantics? (That is, the std.fs API error sets should not just extend POSIX error sets.) Alternatively, Zig could declare that, in this case, the EPERM/EACCES distinction isn't worth keeping and effectively define error.AccessDenied (or error.OperationNotPermitted as suggested above) as the single high-level error code that the std.fs.* and std.os.* APIs use, and std.posix should be responsible for hiding the details.

rootbeer added a commit to rootbeer/zig that referenced this issue Feb 19, 2025
…ionDenied

AccessDenied is returned if the file mode bit (user/group/other rwx bits)
disallow access.  PermissionDenied is returned if something else denies
access (immutable bit, SELinux, capabilities, etc).  This somewhat subtle
distinction is a POSIX thing.

This PR is effecitvely an update of PR ziglang#19193.  See ziglang#16782 for the deeper
problems with the AccessDenied/PermissionDenied duopoly.

Tested locally with an immutable file.

Fixes ziglang#22733 and ziglang#19162.
rootbeer added a commit to rootbeer/zig that referenced this issue Feb 20, 2025
… PermissionDenied

`EACCES` is returned if the file mode bit (i.e., user/group/other rwx
bits) disallow access (mapped to `error.AccessDenied`).  `EPERM` is
returned if something else denies access (immutable bit, SELinux,
capabilities, etc) and is mapped to `error.PermissionDenied`.  This
somewhat subtle no-access distinction is part of POSIX.

This PR is effecitvely an update/simplification of PR ziglang#19193.  See ziglang#16782
for the deeper problems with the AccessDenied/PermissionDenied duopoly.

Tested locally with an immutable file.

Fixes ziglang#22733 and ziglang#19162.
rootbeer added a commit to rootbeer/zig that referenced this issue Feb 24, 2025
This PR consistently maps .ACCES into AccessDenied and .PERM into
PermissionDenied.  AccessDenied is returned if the file mode bit
(user/group/other rwx bits) disallow access (errno was `EACCES`).
PermissionDenied is returned if something else denies access (errno was
`EPERM`) (immutable bit, SELinux, capabilities, etc).  This somewhat
subtle distinction is a POSIX thing.

Most of the change is updating std.posix Error Sets to contain both
errors, and then propagating the pair up through caller Error Sets.

Fixes ziglang#16782
rootbeer added a commit to rootbeer/zig that referenced this issue Feb 24, 2025
This PR consistently maps .ACCES into AccessDenied and .PERM into
PermissionDenied.  AccessDenied is returned if the file mode bit
(user/group/other rwx bits) disallow access (errno was `EACCES`).
PermissionDenied is returned if something else denies access (errno was
`EPERM`) (immutable bit, SELinux, capabilities, etc).  This somewhat
subtle distinction is a POSIX thing.

Most of the change is updating std.posix Error Sets to contain both
errors, and then propagating the pair up through caller Error Sets.

Fixes ziglang#16782
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants