Laszlo Ersek
2022-May-11 12:23 UTC
[Libguestfs] [libguestfs PATCH 2/2] daemon/selinux-relabel: tolerate relabeling errors
Option "-C" of setfiles(8) causes setfiles(8) to exit with status 1
rather
than status 255 if it encounters relabeling errors, but no other (fatal)
error. Pass "-C" to setfiles(8) in "selinux-relabel",
because we don't
want the "selinux-relabel" API to fail if setfiles(8) only encounters
relabeling errors.
(NB even without "-C", setfiles(8) continues traversing the directory
tree(s) and relabeling files across relabeling errors, so this change is
specifically about the exit status.)
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1794518
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
daemon/selinux-relabel.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/daemon/selinux-relabel.c b/daemon/selinux-relabel.c
index a34287fe27cb..976cffe37261 100644
--- a/daemon/selinux-relabel.c
+++ b/daemon/selinux-relabel.c
@@ -59,11 +59,13 @@ do_selinux_relabel (const char *specfile, const char *path,
int force)
{
static int flag_m = -1;
+ static int flag_C = -1;
const char *argv[MAX_ARGS];
CLEANUP_FREE char *s_dev = NULL, *s_proc = NULL, *s_selinux = NULL,
*s_sys = NULL, *s_specfile = NULL, *s_path = NULL;
CLEANUP_FREE char *err = NULL;
size_t i = 0;
+ int setfiles_status;
s_dev = sysroot_path ("/dev");
if (!s_dev) {
@@ -107,6 +109,13 @@ do_selinux_relabel (const char *specfile, const char *path,
if (setfiles_has_option (&flag_m, 'm'))
ADD_ARG (argv, i, "-m");
+ /* Not only do we want setfiles to trudge through individual relabeling
+ * errors, we also want the setfiles exit status to differentiate a fatal
+ * error from "relabeling errors only". See RHBZ#1794518.
+ */
+ if (setfiles_has_option (&flag_C, 'C'))
+ ADD_ARG (argv, i, "-C");
+
/* Relabelling in a chroot. */
if (STRNEQ (sysroot, "/")) {
ADD_ARG (argv, i, "-r");
@@ -124,10 +133,10 @@ do_selinux_relabel (const char *specfile, const char
*path,
ADD_ARG (argv, i, s_path);
ADD_ARG (argv, i, NULL);
- if (commandv (NULL, &err, argv) == -1) {
- reply_with_error ("%s", err);
- return -1;
- }
+ setfiles_status = commandrv (NULL, &err, argv);
+ if ((setfiles_status == 0) || (setfiles_status == 1 && flag_C))
+ return 0;
- return 0;
+ reply_with_error ("%s", err);
+ return -1;
}
--
2.19.1.3.g30247aa5d201
Richard W.M. Jones
2022-May-11 12:31 UTC
[Libguestfs] [libguestfs PATCH 2/2] daemon/selinux-relabel: tolerate relabeling errors
On Wed, May 11, 2022 at 02:23:45PM +0200, Laszlo Ersek wrote:> Option "-C" of setfiles(8) causes setfiles(8) to exit with status 1 rather > than status 255 if it encounters relabeling errors, but no other (fatal) > error. Pass "-C" to setfiles(8) in "selinux-relabel", because we don't > want the "selinux-relabel" API to fail if setfiles(8) only encounters > relabeling errors. > > (NB even without "-C", setfiles(8) continues traversing the directory > tree(s) and relabeling files across relabeling errors, so this change is > specifically about the exit status.) > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1794518 > Signed-off-by: Laszlo Ersek <lersek at redhat.com> > --- > daemon/selinux-relabel.c | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/daemon/selinux-relabel.c b/daemon/selinux-relabel.c > index a34287fe27cb..976cffe37261 100644 > --- a/daemon/selinux-relabel.c > +++ b/daemon/selinux-relabel.c > @@ -59,11 +59,13 @@ do_selinux_relabel (const char *specfile, const char *path, > int force) > { > static int flag_m = -1; > + static int flag_C = -1; > const char *argv[MAX_ARGS]; > CLEANUP_FREE char *s_dev = NULL, *s_proc = NULL, *s_selinux = NULL, > *s_sys = NULL, *s_specfile = NULL, *s_path = NULL; > CLEANUP_FREE char *err = NULL; > size_t i = 0; > + int setfiles_status; > > s_dev = sysroot_path ("/dev"); > if (!s_dev) { > @@ -107,6 +109,13 @@ do_selinux_relabel (const char *specfile, const char *path, > if (setfiles_has_option (&flag_m, 'm')) > ADD_ARG (argv, i, "-m"); > > + /* Not only do we want setfiles to trudge through individual relabeling > + * errors, we also want the setfiles exit status to differentiate a fatal > + * error from "relabeling errors only". See RHBZ#1794518. > + */ > + if (setfiles_has_option (&flag_C, 'C')) > + ADD_ARG (argv, i, "-C"); > + > /* Relabelling in a chroot. */ > if (STRNEQ (sysroot, "/")) { > ADD_ARG (argv, i, "-r"); > @@ -124,10 +133,10 @@ do_selinux_relabel (const char *specfile, const char *path, > ADD_ARG (argv, i, s_path); > ADD_ARG (argv, i, NULL); > > - if (commandv (NULL, &err, argv) == -1) { > - reply_with_error ("%s", err); > - return -1; > - } > + setfiles_status = commandrv (NULL, &err, argv); > + if ((setfiles_status == 0) || (setfiles_status == 1 && flag_C)) > + return 0; > > - return 0; > + reply_with_error ("%s", err); > + return -1; > }I've been following this one for a while, thanks for all the work especially in dark corners of the SELinux code. For the series: Reviewed-by: Richard W.M. Jones <rjones at redhat.com> Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v