| Message ID | 20220428065354.27605-1-lersek@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | selinux_restorecon(3), setfiles(8): skip relabeling errors | expand |
A couple of comments: I'm not clear from the patch series what the difference is between an error which ignored (and counted) and an error that would actually stop setfiles immediately. With setfiles -C, will all errors now be counted and cause setfiles to exit with 1, or will some errors still be fatal (exit with 255)? Why on earth is setfiles originally calling exit(-1) at all?! I didn't even know that was allowed. I wrote a test program and this does indeed cause the exit status to be 255 (because the status is &-ed with 0xff). Never seen a program before calling exit(-1). Rich.
On 04/28/22 11:22, Richard W.M. Jones wrote: > > A couple of comments: > > I'm not clear from the patch series what the difference is between an > error which ignored (and counted) and an error that would actually > stop setfiles immediately. The difference is: with the new flag, we count and ignore exactly those errors that would abort the processing (the file tree walk) in case SELINUX_RESTORECON_ABORT_ON_ERROR were specified. This is based on Petr's comments in RHBZ#1794518. The whole idea is: stick with non-aborting, but *also* don't fail the whole function in case you had aborted with ABORT_ON_ERROR. Because right now, even though we don't abort, we still fail the whole function. > With setfiles -C, will all errors now be > counted and cause setfiles to exit with 1, or will some errors still > be fatal (exit with 255)? The latter, definitely. > Why on earth is setfiles originally calling exit(-1) at all?! I > didn't even know that was allowed. I wrote a test program and this > does indeed cause the exit status to be 255 (because the status is > &-ed with 0xff). Never seen a program before calling exit(-1). I noticed that immediately, but that's not something I want to get into... ;) Laszlo
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1794518 The first three patches in the series are cleanups. The fourth patch introduces a new feature (and API) to libselinux, and the fifth patch exposes the feature through a new setfiles(8) command line option. In libguestfs, we invoke setfiles(8) on masses of directory trees (in a large amount of virtual machines). We don't care if relabeling fails on a few individual files, but we still want to catch fatal errors. Currently these error conditions are impossible to tell apart, because selinux_restorecon[_parallel()], even though it is capable of continuing through relabeling errors, ultimately reports even the case of *only* relabeling errors, with return value (-1). Setfiles(8) then has no choice but to exit with status 255 in this case as well. The fourth patch introduces a new selinux_restorecon[_parallel()] flag, namely SELINUX_RESTORECON_COUNT_ERRORS, which counts, but otherwise ignores, relabeling errors encountered during the file tree walk. In case the function succeeds, the skipped error count can be fetched with a new API, selinux_restorecon_get_skipped_errors(). This relies on static library data, which is inspired by existent APIs such as selinux_restorecon_set_sehandle(3) and selinux_restorecon_set_exclude_list(3) -- those do the same (albeit before calling selinux_restorecon[_parallel()], not after). The fifth patch wires the new flag to the new setfiles(8) option "-C". Please CC me on all replies to the series; I'm not subscribed to the SELinux mailing list. ("CONTRIBUTING.md" does not say that subscribing is a requirement.) Cc: "Richard W.M. Jones" <rjones@redhat.com> Cc: Petr Lautrbach <plautrba@redhat.com> Thanks, Laszlo Laszlo Ersek (5): setfiles: fix up inconsistent indentation setfiles: remove useless assignment and comment (after RHBZ#1926386) setfiles: remove useless "iamrestorecon" checks in option parsing selinux_restorecon: introduce SELINUX_RESTORECON_COUNT_ERRORS setfiles: introduce the -C option for distinguishing file tree walk errors libselinux/include/selinux/restorecon.h | 15 ++++++++ libselinux/man/man3/selinux_restorecon.3 | 22 +++++++++++- libselinux/man/man3/selinux_restorecon_get_skipped_errors.3 | 28 +++++++++++++++ libselinux/src/libselinux.map | 5 +++ libselinux/src/selinux_restorecon.c | 34 +++++++++++++++--- policycoreutils/setfiles/restore.c | 8 +++-- policycoreutils/setfiles/restore.h | 4 ++- policycoreutils/setfiles/setfiles.8 | 22 ++++++++++++ policycoreutils/setfiles/setfiles.c | 36 +++++++++----------- 9 files changed, 145 insertions(+), 29 deletions(-) create mode 100644 libselinux/man/man3/selinux_restorecon_get_skipped_errors.3 base-commit: 2a167d1156578fc29541f6fb60af65452f431aae