| Message ID | 20191021055505.25372-1-jason@perfinion.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | policycoreutils: semodule: Enable CIL logging | expand |
On 10/21/19 1:55 AM, Jason Zaman wrote: > semodule -v will turn on semodule's own verbose logging but not logging > from CIL. This change makes the verbose flag also set cil's log level. > > By default (ie no -v flag), this will enable CIL_ERR, and each -v will > increase the level from there. > > Tested with a duplicated fcontext in the policy. > Before this change: > # semodule -v -B > Committing changes: > Problems processing filecon rules > Failed post db handling > semodule: Failed! > > After this change: > # semodule -v -B > [ ... snip ... ] > Found conflicting filecon rules > at /var/lib/selinux/mcs/tmp/modules/400/mycustom/cil:159 > at /var/lib/selinux/mcs/tmp/modules/400/mycustom/cil:158 > Problems processing filecon rules > Failed post db handling > semodule: Failed! > > Closes: https://github.com/SELinuxProject/selinux/issues/176 > Signed-off-by: Jason Zaman <jason@perfinion.com> > --- > > I also opened a PR here to run travis tests: https://github.com/SELinuxProject/selinux/pull/182 > > This only affects semodule -v, I tested out setsebool and it doesnt die > on a duplicated fcontext so I skipped it there. Should all the tools set > it or only as-needed? Do we want to make some general guidelines for > what kind of tools should set the CIL logging? Thanks, applied. Not sure about whether other tools should set the cil log level. I think current cil logging leaves a lot to be desired, especially since the pathnames identified in the output when invoked from semodule/libsemanage are temporary. It would be preferable IMHO to display the actual conflicting rules themselves and not just the file name and line number. > > -- Jason > > > policycoreutils/semodule/semodule.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/policycoreutils/semodule/semodule.c b/policycoreutils/semodule/semodule.c > index a76797f5..a1f75e16 100644 > --- a/policycoreutils/semodule/semodule.c > +++ b/policycoreutils/semodule/semodule.c > @@ -22,6 +22,7 @@ > #include <libgen.h> > #include <limits.h> > > +#include <sepol/cil/cil.h> > #include <semanage/modules.h> > > enum client_modes { > @@ -238,7 +239,7 @@ static void parse_command_line(int argc, char **argv) > set_mode(LIST_M, optarg); > break; > case 'v': > - verbose = 1; > + verbose++; > break; > case 'r': > set_mode(REMOVE_M, optarg); > @@ -350,6 +351,8 @@ int main(int argc, char *argv[]) > } > parse_command_line(argc, argv); > > + cil_set_log_level(CIL_ERR + verbose); > + > if (build) > commit = 1; > >
diff --git a/policycoreutils/semodule/semodule.c b/policycoreutils/semodule/semodule.c index a76797f5..a1f75e16 100644 --- a/policycoreutils/semodule/semodule.c +++ b/policycoreutils/semodule/semodule.c @@ -22,6 +22,7 @@ #include <libgen.h> #include <limits.h> +#include <sepol/cil/cil.h> #include <semanage/modules.h> enum client_modes { @@ -238,7 +239,7 @@ static void parse_command_line(int argc, char **argv) set_mode(LIST_M, optarg); break; case 'v': - verbose = 1; + verbose++; break; case 'r': set_mode(REMOVE_M, optarg); @@ -350,6 +351,8 @@ int main(int argc, char *argv[]) } parse_command_line(argc, argv); + cil_set_log_level(CIL_ERR + verbose); + if (build) commit = 1;
semodule -v will turn on semodule's own verbose logging but not logging from CIL. This change makes the verbose flag also set cil's log level. By default (ie no -v flag), this will enable CIL_ERR, and each -v will increase the level from there. Tested with a duplicated fcontext in the policy. Before this change: # semodule -v -B Committing changes: Problems processing filecon rules Failed post db handling semodule: Failed! After this change: # semodule -v -B [ ... snip ... ] Found conflicting filecon rules at /var/lib/selinux/mcs/tmp/modules/400/mycustom/cil:159 at /var/lib/selinux/mcs/tmp/modules/400/mycustom/cil:158 Problems processing filecon rules Failed post db handling semodule: Failed! Closes: https://github.com/SELinuxProject/selinux/issues/176 Signed-off-by: Jason Zaman <jason@perfinion.com> --- I also opened a PR here to run travis tests: https://github.com/SELinuxProject/selinux/pull/182 This only affects semodule -v, I tested out setsebool and it doesnt die on a duplicated fcontext so I skipped it there. Should all the tools set it or only as-needed? Do we want to make some general guidelines for what kind of tools should set the CIL logging? -- Jason policycoreutils/semodule/semodule.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)