[Libguestfs] [libguestfs PATCH 2/2] daemon/selinux-relabel: tolerate relabeling errors

[Richard W.M. Jones]

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