casaroli opened a new pull request, #21461:
URL: https://github.com/apache/nuttx/pull/21461
## Summary
This PR contains the commit of #21454 and has to rebase when that PR merges.
Review the last two commits.
The builtin binary format looks up only the last part of a path.
`exec("/nonexistent/hello")` starts the builtin `hello`, and so does
`exec("/tmp/hello")` for a file that is not a program. POSIX wants ENOENT and
ENOEXEC.
This PR adds `CONFIG_BINFMT_BUILTIN_STRICT_PATH`, default n. With the
option, a name with a `/` matches a builtin only if it names a file in a
mounted BINFS. A bare name still matches. Without the option nothing changes.
## Impact
Default off: no behaviour change, and the new code is not compiled. The
option depends on `CONFIG_BUILTIN`, so it cannot be set in a kernel build.
Without `CONFIG_FS_BINFS`, a name with a `/` never matches a builtin.
The default stays because code depends on the old match. With the option and
no BINFS at `/bin`, these fail:
- Run on qemu-armv8a:nsh: `posix_spawn("/bin/sh")` gives ENOENT.
`popen("echo x")` without `CONFIG_LIBC_ENVPATH` uses `/bin/sh` and fails with
ENOENT. A `#!/bin/sh` script gives ENOENT.
- Only read, not run: `execl("/bin/sh")` in apps `interpreters/bas`, the
shell `/bin/sh` of apps `netutils/dropbear`, the default of
`CONFIG_ADBD_SHELL_SERVICE_PATH` in apps `system/adb`, the shell `/bin/nsh`
that `getpwnam()` reports for root (`ROOT_SHELL`), and `popen()` of a command
that is a builtin (it starts with no shell).
Not changed by this PR: these callers and apps. A BINFS mounted at `/bin`
should avoid the `/bin/sh` failures, but this was not run.
Documentation: `Documentation/os/filesystem/binfs.rst` names the option, in
its own commit.
## Testing
Test program on qemu-armv8a:nsh (flat, HVF) with `FS_BINFS PIPES
SCHED_WAITPID`, as a builtin. The `ret` value is the result of `posix_spawn()`.
| Case | master | option off | option on |
|---|---|---|---|
| `/nonexistent/execbuiltin` | ret 0, the builtin runs | ret 0, the builtin
runs | ret 2 (ENOENT) |
| `/tmp/execbuiltin`, a text file | ret 0, the builtin runs | ret 0, the
builtin runs | ret 8 (ENOEXEC) |
| `execbuiltin`, a bare name | runs | runs | runs |
| `/ebbin/execbuiltin` in a mounted BINFS | runs | runs | runs |
| `./execbuiltin` with the cwd in the BINFS | runs | runs | runs |
| `/ebbin/nosuchprogram` | ret 2 | ret 2 | ret 2 |
| `/ebbin/sub/execbuiltin` | ret 0, the builtin runs | ret 0, the builtin
runs | ret 2 |
| a directory with the name of a builtin | ret 0, the builtin runs | ret 0,
the builtin runs | ret 8 |
| ostest exit status | 0 | 0 | 0 |
The option was also run without `FS_BINFS`, and with `ELF` and
`BINFMT_SCRIPT`: missing, text file and bare name as above, ostest 0.
The option off and option on columns ran on this head. The other runs and
the builds ran before the last change of #21454, which only changes
`load_absmodule()`.
Build only, with `EXTRAFLAGS="-Wno-cpp -Werror"`, no compiler warnings:
option off and on: qemu-armv8a:nsh (flat), mps2-an500:knsh (protected),
rv-virt:citest (flat); option on without `FS_BINFS`: qemu-armv8a:nsh,
mps2-an500:knsh; qemu-armv8a:knsh (kernel, the option cannot be set). The
change has no arch code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
