[Bug 1817789] Re: misleading error message for SELinux denials
I changed the error message, as indicated. ** Changed in: mksh Status: Opinion => Fix Released -- You received this bug notification because you are a member of mksh Mailing List, which is subscribed to mksh. Matching subscriptions: mkshlist-to-mksh-bugmail https://bugs.launchpad.net/bugs/1817789 Title: misleading error message for SELinux denials Status in mksh: Fix Released Bug description: Given a stat(2) failure caused by an SELinux denial (rather than a stat(2) success and an access(2) failure, as with a regular `chmod a-x` failure), mksh reports "not found" rather than the more correct "Permission denied". Expected: * Permission Denied error message Actual: $ sh -c /system/bin/vold sh: /system/bin/vold: not found "not found" error message. here's the behind-the-scenes SELinux denial: 02-25 22:37:11.023 4571 4571 W sh : type=1400 audit(0.0:347): avc: denied { getattr } for path="/system/bin/vold" dev="dm-0" ino=717 scontext=u:r:shell:s0 tcontext=u:object_r:vold_exec:s0 tclass=file permissive=0 here's what strace says happened: newfstatat(AT_FDCWD, "/system/bin/vold", 0x7ffcc3ef20, 0) = -1 EACCES (Permission denied) write(2, "/system/bin/sh: /system/bin/vold"..., 44/system/bin/sh: /system/bin/vold: not found ) = 44 versus the normal `chmod a-x` case where stat succeeds but access fails: newfstatat(AT_FDCWD, "/data/local/tmp/date2", {st_mode=S_IFREG|0644, st_size=482560, ...}, 0) = 0 faccessat(AT_FDCWD, "/data/local/tmp/date2", X_OK) = -1 EACCES (Permission denied) write(2, "sh: /data/local/tmp/date2: can't"..., 60sh: /data/local/tmp/date2: can't execute: Permission denied ) = 60 this patch fixes the issue: ``` diff --git a/src/exec.c b/src/exec.c index 8330174..3f6d876 100644 --- a/src/exec.c +++ b/src/exec.c @@ -1279,8 +1279,8 @@ search_access(const char *fn, int mode) struct stat sb; if (stat(fn, &sb) < 0) - /* file does not exist */ - return (ENOENT); + /* file may or may not exist: check errno */ + return errno; /* LINTED use of access */ if (access(fn, mode) < 0) { /* file exists, but we can't access it */ ``` i don't know if you want to elaborate further in the comment along the lines of "...for example, an SELinux denial may mean that we get EACCES here, or if the file doesn't exist and we're allowed to know that, we'll get ENOENT". result with patch: $ sh -c /system/bin/vold sh: /system/bin/vold: can't execute: Permission denied To manage notifications about this bug go to: https://bugs.launchpad.net/mksh/+bug/1817789/+subscriptions
[Bug 1817789] Re: misleading error message for SELinux denials
To be clear, the current implementation of using stat(), reading the permissions, then later exec()ing is subject to the same race conditions described in the access() man page. Just because stat() doesn't include these warnings in the man page shouldn't be interpreted that the current usage is race condition free. In fact, if the code wants to be race condition free, the appropriate thing to do is to skip the access(X_OK) check *and* the stat() check, and just do the exec(). This is the solution I proposed in comment #10 bullet #5. Since you're raising security concerns, I assume you want to be race condition free? As an alternative solution to deal with buggy or incomplete access() implementations, we could also consider making the "stat()" call conditional on the operating system. For Linux, in particular, access(X_OK) is known to be robust, so we can omit the stat() check on that operating system. I proposed a similar solution in https://bugs.launchpad.net/mksh/+bug/1817959 comment #6. -- You received this bug notification because you are a member of mksh Mailing List, which is subscribed to mksh. Matching subscriptions: mkshlist-to-mksh-bugmail https://bugs.launchpad.net/bugs/1817789 Title: misleading error message for SELinux denials Status in mksh: Opinion Bug description: Given a stat(2) failure caused by an SELinux denial (rather than a stat(2) success and an access(2) failure, as with a regular `chmod a-x` failure), mksh reports "not found" rather than the more correct "Permission denied". Expected: * Permission Denied error message Actual: $ sh -c /system/bin/vold sh: /system/bin/vold: not found "not found" error message. here's the behind-the-scenes SELinux denial: 02-25 22:37:11.023 4571 4571 W sh : type=1400 audit(0.0:347): avc: denied { getattr } for path="/system/bin/vold" dev="dm-0" ino=717 scontext=u:r:shell:s0 tcontext=u:object_r:vold_exec:s0 tclass=file permissive=0 here's what strace says happened: newfstatat(AT_FDCWD, "/system/bin/vold", 0x7ffcc3ef20, 0) = -1 EACCES (Permission denied) write(2, "/system/bin/sh: /system/bin/vold"..., 44/system/bin/sh: /system/bin/vold: not found ) = 44 versus the normal `chmod a-x` case where stat succeeds but access fails: newfstatat(AT_FDCWD, "/data/local/tmp/date2", {st_mode=S_IFREG|0644, st_size=482560, ...}, 0) = 0 faccessat(AT_FDCWD, "/data/local/tmp/date2", X_OK) = -1 EACCES (Permission denied) write(2, "sh: /data/local/tmp/date2: can't"..., 60sh: /data/local/tmp/date2: can't execute: Permission denied ) = 60 this patch fixes the issue: ``` diff --git a/src/exec.c b/src/exec.c index 8330174..3f6d876 100644 --- a/src/exec.c +++ b/src/exec.c @@ -1279,8 +1279,8 @@ search_access(const char *fn, int mode) struct stat sb; if (stat(fn, &sb) < 0) - /* file does not exist */ - return (ENOENT); + /* file may or may not exist: check errno */ + return errno; /* LINTED use of access */ if (access(fn, mode) < 0) { /* file exists, but we can't access it */ ``` i don't know if you want to elaborate further in the comment along the lines of "...for example, an SELinux denial may mean that we get EACCES here, or if the file doesn't exist and we're allowed to know that, we'll get ENOENT". result with patch: $ sh -c /system/bin/vold sh: /system/bin/vold: can't execute: Permission denied To manage notifications about this bug go to: https://bugs.launchpad.net/mksh/+bug/1817789/+subscriptions
[Bug 1817789] Re: misleading error message for SELinux denials
I vaguely recall something about access() succeeding for file foo when foo.exe exists on some platforms, or the other way round, and that the modes are also not always correct. The manpage supports the latter… Even if a process has appropriate privileges and indicates success for X_OK, the file may not actually have execute permission bits set. Like- wise for R_OK and W_OK. … but uses stronger language: CAVEATS access() is a potential security hole and should never be used. Ceterum censeo, a non-broken stat() is mandatory. -- You received this bug notification because you are a member of mksh Mailing List, which is subscribed to mksh. Matching subscriptions: mkshlist-to-mksh-bugmail https://bugs.launchpad.net/bugs/1817789 Title: misleading error message for SELinux denials Status in mksh: Opinion Bug description: Given a stat(2) failure caused by an SELinux denial (rather than a stat(2) success and an access(2) failure, as with a regular `chmod a-x` failure), mksh reports "not found" rather than the more correct "Permission denied". Expected: * Permission Denied error message Actual: $ sh -c /system/bin/vold sh: /system/bin/vold: not found "not found" error message. here's the behind-the-scenes SELinux denial: 02-25 22:37:11.023 4571 4571 W sh : type=1400 audit(0.0:347): avc: denied { getattr } for path="/system/bin/vold" dev="dm-0" ino=717 scontext=u:r:shell:s0 tcontext=u:object_r:vold_exec:s0 tclass=file permissive=0 here's what strace says happened: newfstatat(AT_FDCWD, "/system/bin/vold", 0x7ffcc3ef20, 0) = -1 EACCES (Permission denied) write(2, "/system/bin/sh: /system/bin/vold"..., 44/system/bin/sh: /system/bin/vold: not found ) = 44 versus the normal `chmod a-x` case where stat succeeds but access fails: newfstatat(AT_FDCWD, "/data/local/tmp/date2", {st_mode=S_IFREG|0644, st_size=482560, ...}, 0) = 0 faccessat(AT_FDCWD, "/data/local/tmp/date2", X_OK) = -1 EACCES (Permission denied) write(2, "sh: /data/local/tmp/date2: can't"..., 60sh: /data/local/tmp/date2: can't execute: Permission denied ) = 60 this patch fixes the issue: ``` diff --git a/src/exec.c b/src/exec.c index 8330174..3f6d876 100644 --- a/src/exec.c +++ b/src/exec.c @@ -1279,8 +1279,8 @@ search_access(const char *fn, int mode) struct stat sb; if (stat(fn, &sb) < 0) - /* file does not exist */ - return (ENOENT); + /* file may or may not exist: check errno */ + return errno; /* LINTED use of access */ if (access(fn, mode) < 0) { /* file exists, but we can't access it */ ``` i don't know if you want to elaborate further in the comment along the lines of "...for example, an SELinux denial may mean that we get EACCES here, or if the file doesn't exist and we're allowed to know that, we'll get ENOENT". result with patch: $ sh -c /system/bin/vold sh: /system/bin/vold: can't execute: Permission denied To manage notifications about this bug go to: https://bugs.launchpad.net/mksh/+bug/1817789/+subscriptions
[Bug 1817789] Re: misleading error message for SELinux denials
“5) A failure of the stat() system call is not an accurate indicator of the exec()-ability of the file. The only way to determine if a file is executable is to execute it.” This is not generally true. We need to check for +x, and there was something with strange operating systems, *and* the shell does not only use the exec*() family syscalls but also “executes” a script by doing stuff itself. I’m willing to compromise on the error message right now. Everything else needs more thought, and (probably) tomorrow will see a new mksh version. -- You received this bug notification because you are a member of mksh Mailing List, which is subscribed to mksh. Matching subscriptions: mkshlist-to-mksh-bugmail https://bugs.launchpad.net/bugs/1817789 Title: misleading error message for SELinux denials Status in mksh: Opinion Bug description: Given a stat(2) failure caused by an SELinux denial (rather than a stat(2) success and an access(2) failure, as with a regular `chmod a-x` failure), mksh reports "not found" rather than the more correct "Permission denied". Expected: * Permission Denied error message Actual: $ sh -c /system/bin/vold sh: /system/bin/vold: not found "not found" error message. here's the behind-the-scenes SELinux denial: 02-25 22:37:11.023 4571 4571 W sh : type=1400 audit(0.0:347): avc: denied { getattr } for path="/system/bin/vold" dev="dm-0" ino=717 scontext=u:r:shell:s0 tcontext=u:object_r:vold_exec:s0 tclass=file permissive=0 here's what strace says happened: newfstatat(AT_FDCWD, "/system/bin/vold", 0x7ffcc3ef20, 0) = -1 EACCES (Permission denied) write(2, "/system/bin/sh: /system/bin/vold"..., 44/system/bin/sh: /system/bin/vold: not found ) = 44 versus the normal `chmod a-x` case where stat succeeds but access fails: newfstatat(AT_FDCWD, "/data/local/tmp/date2", {st_mode=S_IFREG|0644, st_size=482560, ...}, 0) = 0 faccessat(AT_FDCWD, "/data/local/tmp/date2", X_OK) = -1 EACCES (Permission denied) write(2, "sh: /data/local/tmp/date2: can't"..., 60sh: /data/local/tmp/date2: can't execute: Permission denied ) = 60 this patch fixes the issue: ``` diff --git a/src/exec.c b/src/exec.c index 8330174..3f6d876 100644 --- a/src/exec.c +++ b/src/exec.c @@ -1279,8 +1279,8 @@ search_access(const char *fn, int mode) struct stat sb; if (stat(fn, &sb) < 0) - /* file does not exist */ - return (ENOENT); + /* file may or may not exist: check errno */ + return errno; /* LINTED use of access */ if (access(fn, mode) < 0) { /* file exists, but we can't access it */ ``` i don't know if you want to elaborate further in the comment along the lines of "...for example, an SELinux denial may mean that we get EACCES here, or if the file doesn't exist and we're allowed to know that, we'll get ENOENT". result with patch: $ sh -c /system/bin/vold sh: /system/bin/vold: can't execute: Permission denied To manage notifications about this bug go to: https://bugs.launchpad.net/mksh/+bug/1817789/+subscriptions
[Bug 1817789] Re: misleading error message for SELinux denials
Are you referring to Posix 1003.1 section "C.2.8.2 Exit Status for Commands"? Historical shells make the distinction between ‘‘utility not found’’ and ‘‘utility found but cannot execute’’ in their error messages. By specifying two seldomly used exit status values for these cases, 127 and 126 respectively, this gives an application the opportunity to make use of this distinction without having to parse an error message that would probably change from locale to locale. The command, env, nohup, and xargs utilities in the Shell and Utilities volume of IEEE Std 1003.1-2001 have also been specified to use this convention. (I may be viewing an out of date version... my apologies if so) A few comments: 1) Based on the above, POSIX also seems to require different error messages for "not found" and "found but cannot execute". The entire reason this bug exists is because mksh is failing to return different error messages when the executable exists but the stat() fails. The proposal from comment #8 seems to acknowledge this non-compliance, yet not address it. 2) The stat() is unnecessary for distinguishing between a 126 and 127 error status. Instead, a much simpler way is to just execve() the file, return 127 iff errno==ENOENT. I believe this is all Posix requires. 3) Manually checking permission bits is not an accurate indicator of whether the file can be executed or not, primarily due to LSMs. 4) Calling stat() unnecessarily inhibits certain use cases, specifically comment #7. 5) A failure of the stat() system call is not an accurate indicator of the exec()-ability of the file. The only way to determine if a file is executable is to execute it. -- You received this bug notification because you are a member of mksh Mailing List, which is subscribed to mksh. Matching subscriptions: mkshlist-to-mksh-bugmail https://bugs.launchpad.net/bugs/1817789 Title: misleading error message for SELinux denials Status in mksh: Opinion Bug description: Given a stat(2) failure caused by an SELinux denial (rather than a stat(2) success and an access(2) failure, as with a regular `chmod a-x` failure), mksh reports "not found" rather than the more correct "Permission denied". Expected: * Permission Denied error message Actual: $ sh -c /system/bin/vold sh: /system/bin/vold: not found "not found" error message. here's the behind-the-scenes SELinux denial: 02-25 22:37:11.023 4571 4571 W sh : type=1400 audit(0.0:347): avc: denied { getattr } for path="/system/bin/vold" dev="dm-0" ino=717 scontext=u:r:shell:s0 tcontext=u:object_r:vold_exec:s0 tclass=file permissive=0 here's what strace says happened: newfstatat(AT_FDCWD, "/system/bin/vold", 0x7ffcc3ef20, 0) = -1 EACCES (Permission denied) write(2, "/system/bin/sh: /system/bin/vold"..., 44/system/bin/sh: /system/bin/vold: not found ) = 44 versus the normal `chmod a-x` case where stat succeeds but access fails: newfstatat(AT_FDCWD, "/data/local/tmp/date2", {st_mode=S_IFREG|0644, st_size=482560, ...}, 0) = 0 faccessat(AT_FDCWD, "/data/local/tmp/date2", X_OK) = -1 EACCES (Permission denied) write(2, "sh: /data/local/tmp/date2: can't"..., 60sh: /data/local/tmp/date2: can't execute: Permission denied ) = 60 this patch fixes the issue: ``` diff --git a/src/exec.c b/src/exec.c index 8330174..3f6d876 100644 --- a/src/exec.c +++ b/src/exec.c @@ -1279,8 +1279,8 @@ search_access(const char *fn, int mode) struct stat sb; if (stat(fn, &sb) < 0) - /* file does not exist */ - return (ENOENT); + /* file may or may not exist: check errno */ + return errno; /* LINTED use of access */ if (access(fn, mode) < 0) { /* file exists, but we can't access it */ ``` i don't know if you want to elaborate further in the comment along the lines of "...for example, an SELinux denial may mean that we get EACCES here, or if the file doesn't exist and we're allowed to know that, we'll get ENOENT". result with patch: $ sh -c /system/bin/vold sh: /system/bin/vold: can't execute: Permission denied To manage notifications about this bug go to: https://bugs.launchpad.net/mksh/+bug/1817789/+subscriptions
[Bug 1817789] Re: misleading error message for SELinux denials
stat() must always succeed, because we must check that the executable bits are set before accepting the file as command (if not for anything else, then because POSIX was changed to demand distinguishing $? between 126 and 127 for these cases). -- You received this bug notification because you are a member of mksh Mailing List, which is subscribed to mksh. Matching subscriptions: mkshlist-to-mksh-bugmail https://bugs.launchpad.net/bugs/1817789 Title: misleading error message for SELinux denials Status in mksh: Opinion Bug description: Given a stat(2) failure caused by an SELinux denial (rather than a stat(2) success and an access(2) failure, as with a regular `chmod a-x` failure), mksh reports "not found" rather than the more correct "Permission denied". Expected: * Permission Denied error message Actual: $ sh -c /system/bin/vold sh: /system/bin/vold: not found "not found" error message. here's the behind-the-scenes SELinux denial: 02-25 22:37:11.023 4571 4571 W sh : type=1400 audit(0.0:347): avc: denied { getattr } for path="/system/bin/vold" dev="dm-0" ino=717 scontext=u:r:shell:s0 tcontext=u:object_r:vold_exec:s0 tclass=file permissive=0 here's what strace says happened: newfstatat(AT_FDCWD, "/system/bin/vold", 0x7ffcc3ef20, 0) = -1 EACCES (Permission denied) write(2, "/system/bin/sh: /system/bin/vold"..., 44/system/bin/sh: /system/bin/vold: not found ) = 44 versus the normal `chmod a-x` case where stat succeeds but access fails: newfstatat(AT_FDCWD, "/data/local/tmp/date2", {st_mode=S_IFREG|0644, st_size=482560, ...}, 0) = 0 faccessat(AT_FDCWD, "/data/local/tmp/date2", X_OK) = -1 EACCES (Permission denied) write(2, "sh: /data/local/tmp/date2: can't"..., 60sh: /data/local/tmp/date2: can't execute: Permission denied ) = 60 this patch fixes the issue: ``` diff --git a/src/exec.c b/src/exec.c index 8330174..3f6d876 100644 --- a/src/exec.c +++ b/src/exec.c @@ -1279,8 +1279,8 @@ search_access(const char *fn, int mode) struct stat sb; if (stat(fn, &sb) < 0) - /* file does not exist */ - return (ENOENT); + /* file may or may not exist: check errno */ + return errno; /* LINTED use of access */ if (access(fn, mode) < 0) { /* file exists, but we can't access it */ ``` i don't know if you want to elaborate further in the comment along the lines of "...for example, an SELinux denial may mean that we get EACCES here, or if the file doesn't exist and we're allowed to know that, we'll get ENOENT". result with patch: $ sh -c /system/bin/vold sh: /system/bin/vold: can't execute: Permission denied To manage notifications about this bug go to: https://bugs.launchpad.net/mksh/+bug/1817789/+subscriptions
[Bug 1817789] Re: misleading error message for SELinux denials
What do you think of “not found or inaccessible”? (I hope I spelt that right.) -- You received this bug notification because you are a member of mksh Mailing List, which is subscribed to mksh. Matching subscriptions: mkshlist-to-mksh-bugmail https://bugs.launchpad.net/bugs/1817789 Title: misleading error message for SELinux denials Status in mksh: Opinion Bug description: Given a stat(2) failure caused by an SELinux denial (rather than a stat(2) success and an access(2) failure, as with a regular `chmod a-x` failure), mksh reports "not found" rather than the more correct "Permission denied". Expected: * Permission Denied error message Actual: $ sh -c /system/bin/vold sh: /system/bin/vold: not found "not found" error message. here's the behind-the-scenes SELinux denial: 02-25 22:37:11.023 4571 4571 W sh : type=1400 audit(0.0:347): avc: denied { getattr } for path="/system/bin/vold" dev="dm-0" ino=717 scontext=u:r:shell:s0 tcontext=u:object_r:vold_exec:s0 tclass=file permissive=0 here's what strace says happened: newfstatat(AT_FDCWD, "/system/bin/vold", 0x7ffcc3ef20, 0) = -1 EACCES (Permission denied) write(2, "/system/bin/sh: /system/bin/vold"..., 44/system/bin/sh: /system/bin/vold: not found ) = 44 versus the normal `chmod a-x` case where stat succeeds but access fails: newfstatat(AT_FDCWD, "/data/local/tmp/date2", {st_mode=S_IFREG|0644, st_size=482560, ...}, 0) = 0 faccessat(AT_FDCWD, "/data/local/tmp/date2", X_OK) = -1 EACCES (Permission denied) write(2, "sh: /data/local/tmp/date2: can't"..., 60sh: /data/local/tmp/date2: can't execute: Permission denied ) = 60 this patch fixes the issue: ``` diff --git a/src/exec.c b/src/exec.c index 8330174..3f6d876 100644 --- a/src/exec.c +++ b/src/exec.c @@ -1279,8 +1279,8 @@ search_access(const char *fn, int mode) struct stat sb; if (stat(fn, &sb) < 0) - /* file does not exist */ - return (ENOENT); + /* file may or may not exist: check errno */ + return errno; /* LINTED use of access */ if (access(fn, mode) < 0) { /* file exists, but we can't access it */ ``` i don't know if you want to elaborate further in the comment along the lines of "...for example, an SELinux denial may mean that we get EACCES here, or if the file doesn't exist and we're allowed to know that, we'll get ENOENT". result with patch: $ sh -c /system/bin/vold sh: /system/bin/vold: can't execute: Permission denied To manage notifications about this bug go to: https://bugs.launchpad.net/mksh/+bug/1817789/+subscriptions
[Bug 1817789] Re: misleading error message for SELinux denials
Additionally, this behavior also causes problems where the security policy writer, for whatever reason, wants to allow a file to be executed but disallow stat() operations. This could occur, for example, in high sensitivity environments where leaking metadata (size, last update time, etc) about the file being executed could reveal other activity on the system. By assuming that stat() must always succeed before the file can be executed, such environments cannot create unstat()able executables and have mksh run the executable. -- You received this bug notification because you are a member of mksh Mailing List, which is subscribed to mksh. Matching subscriptions: mkshlist-to-mksh-bugmail https://bugs.launchpad.net/bugs/1817789 Title: misleading error message for SELinux denials Status in mksh: Opinion Bug description: Given a stat(2) failure caused by an SELinux denial (rather than a stat(2) success and an access(2) failure, as with a regular `chmod a-x` failure), mksh reports "not found" rather than the more correct "Permission denied". Expected: * Permission Denied error message Actual: $ sh -c /system/bin/vold sh: /system/bin/vold: not found "not found" error message. here's the behind-the-scenes SELinux denial: 02-25 22:37:11.023 4571 4571 W sh : type=1400 audit(0.0:347): avc: denied { getattr } for path="/system/bin/vold" dev="dm-0" ino=717 scontext=u:r:shell:s0 tcontext=u:object_r:vold_exec:s0 tclass=file permissive=0 here's what strace says happened: newfstatat(AT_FDCWD, "/system/bin/vold", 0x7ffcc3ef20, 0) = -1 EACCES (Permission denied) write(2, "/system/bin/sh: /system/bin/vold"..., 44/system/bin/sh: /system/bin/vold: not found ) = 44 versus the normal `chmod a-x` case where stat succeeds but access fails: newfstatat(AT_FDCWD, "/data/local/tmp/date2", {st_mode=S_IFREG|0644, st_size=482560, ...}, 0) = 0 faccessat(AT_FDCWD, "/data/local/tmp/date2", X_OK) = -1 EACCES (Permission denied) write(2, "sh: /data/local/tmp/date2: can't"..., 60sh: /data/local/tmp/date2: can't execute: Permission denied ) = 60 this patch fixes the issue: ``` diff --git a/src/exec.c b/src/exec.c index 8330174..3f6d876 100644 --- a/src/exec.c +++ b/src/exec.c @@ -1279,8 +1279,8 @@ search_access(const char *fn, int mode) struct stat sb; if (stat(fn, &sb) < 0) - /* file does not exist */ - return (ENOENT); + /* file may or may not exist: check errno */ + return errno; /* LINTED use of access */ if (access(fn, mode) < 0) { /* file exists, but we can't access it */ ``` i don't know if you want to elaborate further in the comment along the lines of "...for example, an SELinux denial may mean that we get EACCES here, or if the file doesn't exist and we're allowed to know that, we'll get ENOENT". result with patch: $ sh -c /system/bin/vold sh: /system/bin/vold: can't execute: Permission denied To manage notifications about this bug go to: https://bugs.launchpad.net/mksh/+bug/1817789/+subscriptions
[Bug 1817789] Re: misleading error message for SELinux denials
In the SELinux case that Elliott pointed to in the initial bug report, mksh can also "see" the file (eg, stat() returns EACCES, indicating the file exists but security policy disallows stat() operations). Yet "not found" is emitted by mksh vs (the IMHO more correct) "Permission denied". The mksh code assumes any stat() failure is due to the file not existing vs other causes. Unfortunately, this error condition can only be replicated using one of the available Linux Mandatory Access Control systems such as SELinux, Smack, AppArmor, or Tomoyo. Similarly, in the following scenerio, mksh can "see" the asdf command, but still returns "not found", when the command clearly exists but is malformed. nnk@nnk0:/tmp$ mkdir d nnk@nnk0:/tmp$ ln -s ../asdf d/asdf nnk@nnk0:/tmp$ ln -s d/asdf asdf nnk@nnk0:/tmp$ ls -la d/asdf asdf lrwxrwxrwx 1 nnk nnk 6 Feb 27 09:27 asdf -> d/asdf lrwxrwxrwx 1 nnk nnk 7 Feb 27 09:27 d/asdf -> ../asdf nnk@nnk0:/tmp$ mksh -c /tmp/asdf mksh: /tmp/asdf: not found Bash provides a more accurate error message in this case: nnk@nnk0:/tmp$ bash -c /tmp/asdf bash: /tmp/asdf: Too many levels of symbolic links This is particularly problematic for interactive shells, where the lack of accurate error messages inhibits end user understanding of the error conditions. nnk@nnk0:/tmp$ mksh $ /tmp/asdf mksh: /tmp/asdf: not found $ ls -la /tmp/asdf lrwxrwxrwx 1 nnk nnk 6 Feb 27 09:27 /tmp/asdf -> d/asdf $ mkdir d2 $ touch d2/a2 $ chmod 000 ./d2 $ /tmp/d2/a2 mksh: /tmp/d2/a2: not found Can you elaborate on the statement that "passing through the errno may introduce other problems"? What other problems are you concerned about? The statement feels unactionable. IMHO, changing "not found" to a more generic string, without other changes, would not improve end user understandability. Thank you for your continued dialog on this issue. -- You received this bug notification because you are a member of mksh Mailing List, which is subscribed to mksh. Matching subscriptions: mkshlist-to-mksh-bugmail https://bugs.launchpad.net/bugs/1817789 Title: misleading error message for SELinux denials Status in mksh: Opinion Bug description: Given a stat(2) failure caused by an SELinux denial (rather than a stat(2) success and an access(2) failure, as with a regular `chmod a-x` failure), mksh reports "not found" rather than the more correct "Permission denied". Expected: * Permission Denied error message Actual: $ sh -c /system/bin/vold sh: /system/bin/vold: not found "not found" error message. here's the behind-the-scenes SELinux denial: 02-25 22:37:11.023 4571 4571 W sh : type=1400 audit(0.0:347): avc: denied { getattr } for path="/system/bin/vold" dev="dm-0" ino=717 scontext=u:r:shell:s0 tcontext=u:object_r:vold_exec:s0 tclass=file permissive=0 here's what strace says happened: newfstatat(AT_FDCWD, "/system/bin/vold", 0x7ffcc3ef20, 0) = -1 EACCES (Permission denied) write(2, "/system/bin/sh: /system/bin/vold"..., 44/system/bin/sh: /system/bin/vold: not found ) = 44 versus the normal `chmod a-x` case where stat succeeds but access fails: newfstatat(AT_FDCWD, "/data/local/tmp/date2", {st_mode=S_IFREG|0644, st_size=482560, ...}, 0) = 0 faccessat(AT_FDCWD, "/data/local/tmp/date2", X_OK) = -1 EACCES (Permission denied) write(2, "sh: /data/local/tmp/date2: can't"..., 60sh: /data/local/tmp/date2: can't execute: Permission denied ) = 60 this patch fixes the issue: ``` diff --git a/src/exec.c b/src/exec.c index 8330174..3f6d876 100644 --- a/src/exec.c +++ b/src/exec.c @@ -1279,8 +1279,8 @@ search_access(const char *fn, int mode) struct stat sb; if (stat(fn, &sb) < 0) - /* file does not exist */ - return (ENOENT); + /* file may or may not exist: check errno */ + return errno; /* LINTED use of access */ if (access(fn, mode) < 0) { /* file exists, but we can't access it */ ``` i don't know if you want to elaborate further in the comment along the lines of "...for example, an SELinux denial may mean that we get EACCES here, or if the file doesn't exist and we're allowed to know that, we'll get ENOENT". result with patch: $ sh -c /system/bin/vold sh: /system/bin/vold: can't execute: Permission denied To manage notifications about this bug go to: https://bugs.launchpad.net/mksh/+bug/1817789/+subscriptions
[Bug 1817789] Re: misleading error message for SELinux denials
Hmm. In the case Nick posted, it can “see” the file (find it) but not read it. In the other case, an intermediate directory is not accessible. This has been so since pdksh times, and I think some code later on checks for ENOENT, so just passing through the errno may introduce other problems. The “not found” is a string in the shell anyway, not an errno. Perhaps changing it to something more encompassing would help? -- You received this bug notification because you are a member of mksh Mailing List, which is subscribed to mksh. Matching subscriptions: mkshlist-to-mksh-bugmail https://bugs.launchpad.net/bugs/1817789 Title: misleading error message for SELinux denials Status in mksh: Opinion Bug description: Given a stat(2) failure caused by an SELinux denial (rather than a stat(2) success and an access(2) failure, as with a regular `chmod a-x` failure), mksh reports "not found" rather than the more correct "Permission denied". Expected: * Permission Denied error message Actual: $ sh -c /system/bin/vold sh: /system/bin/vold: not found "not found" error message. here's the behind-the-scenes SELinux denial: 02-25 22:37:11.023 4571 4571 W sh : type=1400 audit(0.0:347): avc: denied { getattr } for path="/system/bin/vold" dev="dm-0" ino=717 scontext=u:r:shell:s0 tcontext=u:object_r:vold_exec:s0 tclass=file permissive=0 here's what strace says happened: newfstatat(AT_FDCWD, "/system/bin/vold", 0x7ffcc3ef20, 0) = -1 EACCES (Permission denied) write(2, "/system/bin/sh: /system/bin/vold"..., 44/system/bin/sh: /system/bin/vold: not found ) = 44 versus the normal `chmod a-x` case where stat succeeds but access fails: newfstatat(AT_FDCWD, "/data/local/tmp/date2", {st_mode=S_IFREG|0644, st_size=482560, ...}, 0) = 0 faccessat(AT_FDCWD, "/data/local/tmp/date2", X_OK) = -1 EACCES (Permission denied) write(2, "sh: /data/local/tmp/date2: can't"..., 60sh: /data/local/tmp/date2: can't execute: Permission denied ) = 60 this patch fixes the issue: ``` diff --git a/src/exec.c b/src/exec.c index 8330174..3f6d876 100644 --- a/src/exec.c +++ b/src/exec.c @@ -1279,8 +1279,8 @@ search_access(const char *fn, int mode) struct stat sb; if (stat(fn, &sb) < 0) - /* file does not exist */ - return (ENOENT); + /* file may or may not exist: check errno */ + return errno; /* LINTED use of access */ if (access(fn, mode) < 0) { /* file exists, but we can't access it */ ``` i don't know if you want to elaborate further in the comment along the lines of "...for example, an SELinux denial may mean that we get EACCES here, or if the file doesn't exist and we're allowed to know that, we'll get ENOENT". result with patch: $ sh -c /system/bin/vold sh: /system/bin/vold: can't execute: Permission denied To manage notifications about this bug go to: https://bugs.launchpad.net/mksh/+bug/1817789/+subscriptions
[Bug 1817789] Re: misleading error message for SELinux denials
By that argument, would you argue that returning "Permission Denied" below is incorrect? nnk@nnk0:~$ cd /tmp nnk@nnk0:/tmp$ touch ./asdf nnk@nnk0:/tmp$ mksh -c ./asdf mksh: ./asdf: can't execute: Permission denied I don't understand how "mksh -c /tmp/d/f" differs from "mksh -c ./asdf" by the logic above. (FWIW, "Permission Denied" for both of these cases seems more correct to me) -- You received this bug notification because you are a member of mksh Mailing List, which is subscribed to mksh. Matching subscriptions: mkshlist-to-mksh-bugmail https://bugs.launchpad.net/bugs/1817789 Title: misleading error message for SELinux denials Status in mksh: Opinion Bug description: Given a stat(2) failure caused by an SELinux denial (rather than a stat(2) success and an access(2) failure, as with a regular `chmod a-x` failure), mksh reports "not found" rather than the more correct "Permission denied". Expected: * Permission Denied error message Actual: $ sh -c /system/bin/vold sh: /system/bin/vold: not found "not found" error message. here's the behind-the-scenes SELinux denial: 02-25 22:37:11.023 4571 4571 W sh : type=1400 audit(0.0:347): avc: denied { getattr } for path="/system/bin/vold" dev="dm-0" ino=717 scontext=u:r:shell:s0 tcontext=u:object_r:vold_exec:s0 tclass=file permissive=0 here's what strace says happened: newfstatat(AT_FDCWD, "/system/bin/vold", 0x7ffcc3ef20, 0) = -1 EACCES (Permission denied) write(2, "/system/bin/sh: /system/bin/vold"..., 44/system/bin/sh: /system/bin/vold: not found ) = 44 versus the normal `chmod a-x` case where stat succeeds but access fails: newfstatat(AT_FDCWD, "/data/local/tmp/date2", {st_mode=S_IFREG|0644, st_size=482560, ...}, 0) = 0 faccessat(AT_FDCWD, "/data/local/tmp/date2", X_OK) = -1 EACCES (Permission denied) write(2, "sh: /data/local/tmp/date2: can't"..., 60sh: /data/local/tmp/date2: can't execute: Permission denied ) = 60 this patch fixes the issue: ``` diff --git a/src/exec.c b/src/exec.c index 8330174..3f6d876 100644 --- a/src/exec.c +++ b/src/exec.c @@ -1279,8 +1279,8 @@ search_access(const char *fn, int mode) struct stat sb; if (stat(fn, &sb) < 0) - /* file does not exist */ - return (ENOENT); + /* file may or may not exist: check errno */ + return errno; /* LINTED use of access */ if (access(fn, mode) < 0) { /* file exists, but we can't access it */ ``` i don't know if you want to elaborate further in the comment along the lines of "...for example, an SELinux denial may mean that we get EACCES here, or if the file doesn't exist and we're allowed to know that, we'll get ENOENT". result with patch: $ sh -c /system/bin/vold sh: /system/bin/vold: can't execute: Permission denied To manage notifications about this bug go to: https://bugs.launchpad.net/mksh/+bug/1817789/+subscriptions
[Bug 1817789] Re: misleading error message for SELinux denials
No. When you do mksh -c /tmp/d/f you ask mksh to find it as a command. The “not found” error message there is more general; for example, it also happens if the interpreter is not found: print '#!/nonexistent-int' >/tmp/ff chmod 755 /tmp/ff mksh -c /tmp/ff (This even results in “No such file or directory”.) When you ask mksh to read it as a script file, all is well: mksh /tmp/d/f (This results in “Permission denied”.) ** Changed in: mksh Importance: Undecided => Wishlist ** Changed in: mksh Status: New => Opinion -- You received this bug notification because you are a member of mksh Mailing List, which is subscribed to mksh. Matching subscriptions: mkshlist-to-mksh-bugmail https://bugs.launchpad.net/bugs/1817789 Title: misleading error message for SELinux denials Status in mksh: Opinion Bug description: Given a stat(2) failure caused by an SELinux denial (rather than a stat(2) success and an access(2) failure, as with a regular `chmod a-x` failure), mksh reports "not found" rather than the more correct "Permission denied". Expected: * Permission Denied error message Actual: $ sh -c /system/bin/vold sh: /system/bin/vold: not found "not found" error message. here's the behind-the-scenes SELinux denial: 02-25 22:37:11.023 4571 4571 W sh : type=1400 audit(0.0:347): avc: denied { getattr } for path="/system/bin/vold" dev="dm-0" ino=717 scontext=u:r:shell:s0 tcontext=u:object_r:vold_exec:s0 tclass=file permissive=0 here's what strace says happened: newfstatat(AT_FDCWD, "/system/bin/vold", 0x7ffcc3ef20, 0) = -1 EACCES (Permission denied) write(2, "/system/bin/sh: /system/bin/vold"..., 44/system/bin/sh: /system/bin/vold: not found ) = 44 versus the normal `chmod a-x` case where stat succeeds but access fails: newfstatat(AT_FDCWD, "/data/local/tmp/date2", {st_mode=S_IFREG|0644, st_size=482560, ...}, 0) = 0 faccessat(AT_FDCWD, "/data/local/tmp/date2", X_OK) = -1 EACCES (Permission denied) write(2, "sh: /data/local/tmp/date2: can't"..., 60sh: /data/local/tmp/date2: can't execute: Permission denied ) = 60 this patch fixes the issue: ``` diff --git a/src/exec.c b/src/exec.c index 8330174..3f6d876 100644 --- a/src/exec.c +++ b/src/exec.c @@ -1279,8 +1279,8 @@ search_access(const char *fn, int mode) struct stat sb; if (stat(fn, &sb) < 0) - /* file does not exist */ - return (ENOENT); + /* file may or may not exist: check errno */ + return errno; /* LINTED use of access */ if (access(fn, mode) < 0) { /* file exists, but we can't access it */ ``` i don't know if you want to elaborate further in the comment along the lines of "...for example, an SELinux denial may mean that we get EACCES here, or if the file doesn't exist and we're allowed to know that, we'll get ENOENT". result with patch: $ sh -c /system/bin/vold sh: /system/bin/vold: can't execute: Permission denied To manage notifications about this bug go to: https://bugs.launchpad.net/mksh/+bug/1817789/+subscriptions