[PATCH] setsebool: report errors from commit phase

[PATCH] setsebool: report errors from commit phase

[Topi Miettinen]

In case there are errors when committing changes to booleans, the
errors may not be reported to user except by nonzero exit status. With
"setsebool -V" it's possible to see errors from commit phase, but
otherwise the unfixed command is silent:

  # setsebool -V -P secure_mode_insmod=off
libsemanage.semanage_install_final_tmp: Could not copy 
/var/lib/selinux/final/default/contexts/files/file_contexts to 
/etc/selinux/default/contexts/files/file_contexts. (Read-only file system).
libsemanage.semanage_install_final_tmp: Could not copy 
/var/lib/selinux/final/default/contexts/files/file_contexts to 
/etc/selinux/default/contexts/files/file_contexts. (Read-only file system).

Fixed version alerts the user about problems even without -V:
  # setsebool -P secure_mode_insmod=off
Failed to commit changes to booleans: Read-only file system

Signed-off-by: Topi Miettinen <toiwoton@gmail.com>
---
  policycoreutils/setsebool/setsebool.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/policycoreutils/setsebool/setsebool.c 
b/policycoreutils/setsebool/setsebool.c
index 9d8abfac..60da5df1 100644
--- a/policycoreutils/setsebool/setsebool.c
+++ b/policycoreutils/setsebool/setsebool.c
@@ -200,8 +200,10 @@ static int semanage_set_boolean_list(size_t boolcnt,

         if (no_reload)
                 semanage_set_reload(handle, 0);
-       if (semanage_commit(handle) < 0)
+       if (semanage_commit(handle) < 0) {
+               fprintf(stderr, "Failed to commit changes to booleans: 
%m\n");
                 goto err;
+       }

         semanage_disconnect(handle);
         semanage_handle_destroy(handle);
-- 
2.26.2

Re: [PATCH] setsebool: report errors from commit phase

[Nicolas Iooss]

On Sun, Apr 26, 2020 at 5:21 PM Topi Miettinen <toiwoton@gmail.com> wrote:
>
> In case there are errors when committing changes to booleans, the
> errors may not be reported to user except by nonzero exit status. With
> "setsebool -V" it's possible to see errors from commit phase, but
> otherwise the unfixed command is silent:
>
>   # setsebool -V -P secure_mode_insmod=off
> libsemanage.semanage_install_final_tmp: Could not copy
> /var/lib/selinux/final/default/contexts/files/file_contexts to
> /etc/selinux/default/contexts/files/file_contexts. (Read-only file system).
> libsemanage.semanage_install_final_tmp: Could not copy
> /var/lib/selinux/final/default/contexts/files/file_contexts to
> /etc/selinux/default/contexts/files/file_contexts. (Read-only file system).
>
> Fixed version alerts the user about problems even without -V:
>   # setsebool -P secure_mode_insmod=off
> Failed to commit changes to booleans: Read-only file system
>
> Signed-off-by: Topi Miettinen <toiwoton@gmail.com>

Looks good to me. The patch below has been mangled (tabs have been
replaced by spaces) but I took the patch from your Pull Request
(https://github.com/SELinuxProject/selinux/pull/227.patch) and it
applied cleanly.

Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>

If nobody raises an objection, I will merge the patch tomorrow.

Thanks,
Nicolas

> ---
>   policycoreutils/setsebool/setsebool.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/policycoreutils/setsebool/setsebool.c
> b/policycoreutils/setsebool/setsebool.c
> index 9d8abfac..60da5df1 100644
> --- a/policycoreutils/setsebool/setsebool.c
> +++ b/policycoreutils/setsebool/setsebool.c
> @@ -200,8 +200,10 @@ static int semanage_set_boolean_list(size_t boolcnt,
>
>          if (no_reload)
>                  semanage_set_reload(handle, 0);
> -       if (semanage_commit(handle) < 0)
> +       if (semanage_commit(handle) < 0) {
> +               fprintf(stderr, "Failed to commit changes to booleans:
> %m\n");
>                  goto err;
> +       }
>
>          semanage_disconnect(handle);
>          semanage_handle_destroy(handle);
> --
> 2.26.2

Re: [PATCH] setsebool: report errors from commit phase

[Nicolas Iooss]

On Sun, Apr 26, 2020 at 8:09 PM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> On Sun, Apr 26, 2020 at 5:21 PM Topi Miettinen <toiwoton@gmail.com> wrote:
> >
> > In case there are errors when committing changes to booleans, the
> > errors may not be reported to user except by nonzero exit status. With
> > "setsebool -V" it's possible to see errors from commit phase, but
> > otherwise the unfixed command is silent:
> >
> >   # setsebool -V -P secure_mode_insmod=off
> > libsemanage.semanage_install_final_tmp: Could not copy
> > /var/lib/selinux/final/default/contexts/files/file_contexts to
> > /etc/selinux/default/contexts/files/file_contexts. (Read-only file system).
> > libsemanage.semanage_install_final_tmp: Could not copy
> > /var/lib/selinux/final/default/contexts/files/file_contexts to
> > /etc/selinux/default/contexts/files/file_contexts. (Read-only file system).
> >
> > Fixed version alerts the user about problems even without -V:
> >   # setsebool -P secure_mode_insmod=off
> > Failed to commit changes to booleans: Read-only file system
> >
> > Signed-off-by: Topi Miettinen <toiwoton@gmail.com>
>
> Looks good to me. The patch below has been mangled (tabs have been
> replaced by spaces) but I took the patch from your Pull Request
> (https://github.com/SELinuxProject/selinux/pull/227.patch) and it
> applied cleanly.
>
> Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
>
> If nobody raises an objection, I will merge the patch tomorrow.

Merged.

Thanks,
Nicolas
>
> > ---
> >   policycoreutils/setsebool/setsebool.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/policycoreutils/setsebool/setsebool.c
> > b/policycoreutils/setsebool/setsebool.c
> > index 9d8abfac..60da5df1 100644
> > --- a/policycoreutils/setsebool/setsebool.c
> > +++ b/policycoreutils/setsebool/setsebool.c
> > @@ -200,8 +200,10 @@ static int semanage_set_boolean_list(size_t boolcnt,
> >
> >          if (no_reload)
> >                  semanage_set_reload(handle, 0);
> > -       if (semanage_commit(handle) < 0)
> > +       if (semanage_commit(handle) < 0) {
> > +               fprintf(stderr, "Failed to commit changes to booleans:
> > %m\n");
> >                  goto err;
> > +       }
> >
> >          semanage_disconnect(handle);
> >          semanage_handle_destroy(handle);
> > --
> > 2.26.2