| Message ID | 20210113210948.217575-2-plautrba@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2,1/2] setfiles: Do not abort on labeling error | expand |
Petr Lautrbach <plautrba@redhat.com> writes: > `setfiles -d` doesn't have any impact on number of errors before it > aborts. It always aborts on first invalid context in spec file. > > Signed-off-by: Petr Lautrbach <plautrba@redhat.com> > --- > policycoreutils/setfiles/Makefile | 3 --- > policycoreutils/setfiles/ru/setfiles.8 | 2 +- > policycoreutils/setfiles/setfiles.8 | 3 +-- > policycoreutils/setfiles/setfiles.c | 18 ------------------ > 4 files changed, 2 insertions(+), 24 deletions(-) > > diff --git a/policycoreutils/setfiles/Makefile b/policycoreutils/setfiles/Makefile > index bc5a8db789a5..a3bbbe116b7f 100644 > --- a/policycoreutils/setfiles/Makefile > +++ b/policycoreutils/setfiles/Makefile > @@ -5,8 +5,6 @@ SBINDIR ?= /sbin > MANDIR = $(PREFIX)/share/man > AUDITH ?= $(shell test -f /usr/include/libaudit.h && echo y) > > -ABORT_ON_ERRORS=$(shell grep "^\#define ABORT_ON_ERRORS" setfiles.c | awk -S '{ print $$3 }') > - > CFLAGS ?= -g -Werror -Wall -W > override LDLIBS += -lselinux -lsepol > > @@ -26,7 +24,6 @@ restorecon_xattr: restorecon_xattr.o restore.o > > man: > @cp -af setfiles.8 setfiles.8.man > - @sed -i "s/ABORT_ON_ERRORS/$(ABORT_ON_ERRORS)/g" setfiles.8.man > > install: all > [ -d $(DESTDIR)$(MANDIR)/man8 ] || mkdir -p $(DESTDIR)$(MANDIR)/man8 > diff --git a/policycoreutils/setfiles/ru/setfiles.8 b/policycoreutils/setfiles/ru/setfiles.8 > index 27815a3f1eee..910101452625 100644 > --- a/policycoreutils/setfiles/ru/setfiles.8 > +++ b/policycoreutils/setfiles/ru/setfiles.8 > @@ -47,7 +47,7 @@ setfiles \- установить SELinux-контексты безопаснос > проверить действительность контекстов относительно указанной двоичной политики. > .TP > .B \-d > -показать, какая спецификация соответствует каждому из файлов (не прекращать проверку после получения ошибок ABORT_ON_ERRORS). > +показать, какая спецификация соответствует каждому из файлов. > .TP > .BI \-e \ directory > исключить каталог (чтобы исключить более одного каталога, этот параметр необходимо использовать соответствующее количество раз). > diff --git a/policycoreutils/setfiles/setfiles.8 b/policycoreutils/setfiles/setfiles.8 > index e328a5628682..4d28bc9a95c1 100644 > --- a/policycoreutils/setfiles/setfiles.8 > +++ b/policycoreutils/setfiles/setfiles.8 > @@ -57,8 +57,7 @@ option will force a replacement of the entire context. > check the validity of the contexts against the specified binary policy. > .TP > .B \-d > -show what specification matched each file (do not abort validation > -after ABORT_ON_ERRORS errors). > +show what specification matched each file. > .TP > .BI \-e \ directory > directory to exclude (repeat option for more than one directory). > diff --git a/policycoreutils/setfiles/setfiles.c b/policycoreutils/setfiles/setfiles.c > index 10692d6d94a0..92616571ef2a 100644 > --- a/policycoreutils/setfiles/setfiles.c > +++ b/policycoreutils/setfiles/setfiles.c > @@ -23,14 +23,6 @@ static int nerr; > > #define STAT_BLOCK_SIZE 1 > > -/* setfiles will abort its operation after reaching the > - * following number of errors (e.g. invalid contexts), > - * unless it is used in "debug" mode (-d option). > - */ > -#ifndef ABORT_ON_ERRORS > -#define ABORT_ON_ERRORS 10 > -#endif > - > #define SETFILES "setfiles" > #define RESTORECON "restorecon" > static int iamrestorecon; > @@ -56,15 +48,6 @@ static __attribute__((__noreturn__)) void usage(const char *const name) > exit(-1); > } > > -void inc_err(void) > -{ > - nerr++; > - if (nerr > ABORT_ON_ERRORS - 1 && !r_opts.debug) { > - fprintf(stderr, "Exiting after %d errors.\n", ABORT_ON_ERRORS); > - exit(-1); > - } > -} > - > void set_rootpath(const char *arg) > { > if (strlen(arg) == 1 && strncmp(arg, "/", 1) == 0) { > @@ -97,7 +80,6 @@ int canoncon(char **contextp) > *contextp = tmpcon; > } else if (errno != ENOENT) { > rc = -1; > - inc_err(); > } > > return rc; > -- > 2.30.0 If there's no objection I'd like to merge both patches before Wednesday so they'll part of rc2.
Nicolas Iooss <nicolas.iooss@m4x.org> writes: > On Sun, Jan 31, 2021 at 11:39 AM Petr Lautrbach <plautrba@redhat.com> wrote: >> >> Petr Lautrbach <plautrba@redhat.com> writes: >> >> > `setfiles -d` doesn't have any impact on number of errors before it >> > aborts. It always aborts on first invalid context in spec file. >> > >> > Signed-off-by: Petr Lautrbach <plautrba@redhat.com> >> > --- >> > policycoreutils/setfiles/Makefile | 3 --- >> > policycoreutils/setfiles/ru/setfiles.8 | 2 +- >> > policycoreutils/setfiles/setfiles.8 | 3 +-- >> > policycoreutils/setfiles/setfiles.c | 18 ------------------ >> > 4 files changed, 2 insertions(+), 24 deletions(-) >> > >> > diff --git a/policycoreutils/setfiles/Makefile b/policycoreutils/setfiles/Makefile >> > index bc5a8db789a5..a3bbbe116b7f 100644 >> > --- a/policycoreutils/setfiles/Makefile >> > +++ b/policycoreutils/setfiles/Makefile >> > @@ -5,8 +5,6 @@ SBINDIR ?= /sbin >> > MANDIR = $(PREFIX)/share/man >> > AUDITH ?= $(shell test -f /usr/include/libaudit.h && echo y) >> > >> > -ABORT_ON_ERRORS=$(shell grep "^\#define ABORT_ON_ERRORS" setfiles.c | awk -S '{ print $$3 }') >> > - >> > CFLAGS ?= -g -Werror -Wall -W >> > override LDLIBS += -lselinux -lsepol >> > >> > @@ -26,7 +24,6 @@ restorecon_xattr: restorecon_xattr.o restore.o >> > >> > man: >> > @cp -af setfiles.8 setfiles.8.man >> > - @sed -i "s/ABORT_ON_ERRORS/$(ABORT_ON_ERRORS)/g" setfiles.8.man >> > >> > install: all >> > [ -d $(DESTDIR)$(MANDIR)/man8 ] || mkdir -p $(DESTDIR)$(MANDIR)/man8 >> > diff --git a/policycoreutils/setfiles/ru/setfiles.8 b/policycoreutils/setfiles/ru/setfiles.8 >> > index 27815a3f1eee..910101452625 100644 >> > --- a/policycoreutils/setfiles/ru/setfiles.8 >> > +++ b/policycoreutils/setfiles/ru/setfiles.8 >> > @@ -47,7 +47,7 @@ setfiles \- установить SELinux-контексты безопаснос >> > проверить действительность контекстов относительно указанной двоичной политики. >> > .TP >> > .B \-d >> > -показать, какая спецификация соответствует каждому из файлов (не прекращать проверку после получения ошибок ABORT_ON_ERRORS). >> > +показать, какая спецификация соответствует каждому из файлов. >> > .TP >> > .BI \-e \ directory >> > исключить каталог (чтобы исключить более одного каталога, этот параметр необходимо использовать соответствующее количество раз). >> > diff --git a/policycoreutils/setfiles/setfiles.8 b/policycoreutils/setfiles/setfiles.8 >> > index e328a5628682..4d28bc9a95c1 100644 >> > --- a/policycoreutils/setfiles/setfiles.8 >> > +++ b/policycoreutils/setfiles/setfiles.8 >> > @@ -57,8 +57,7 @@ option will force a replacement of the entire context. >> > check the validity of the contexts against the specified binary policy. >> > .TP >> > .B \-d >> > -show what specification matched each file (do not abort validation >> > -after ABORT_ON_ERRORS errors). >> > +show what specification matched each file. >> > .TP >> > .BI \-e \ directory >> > directory to exclude (repeat option for more than one directory). >> > diff --git a/policycoreutils/setfiles/setfiles.c b/policycoreutils/setfiles/setfiles.c >> > index 10692d6d94a0..92616571ef2a 100644 >> > --- a/policycoreutils/setfiles/setfiles.c >> > +++ b/policycoreutils/setfiles/setfiles.c >> > @@ -23,14 +23,6 @@ static int nerr; >> > >> > #define STAT_BLOCK_SIZE 1 >> > >> > -/* setfiles will abort its operation after reaching the >> > - * following number of errors (e.g. invalid contexts), >> > - * unless it is used in "debug" mode (-d option). >> > - */ >> > -#ifndef ABORT_ON_ERRORS >> > -#define ABORT_ON_ERRORS 10 >> > -#endif >> > - >> > #define SETFILES "setfiles" >> > #define RESTORECON "restorecon" >> > static int iamrestorecon; >> > @@ -56,15 +48,6 @@ static __attribute__((__noreturn__)) void usage(const char *const name) >> > exit(-1); >> > } >> > >> > -void inc_err(void) >> > -{ >> > - nerr++; >> > - if (nerr > ABORT_ON_ERRORS - 1 && !r_opts.debug) { >> > - fprintf(stderr, "Exiting after %d errors.\n", ABORT_ON_ERRORS); >> > - exit(-1); >> > - } >> > -} >> > - >> > void set_rootpath(const char *arg) >> > { >> > if (strlen(arg) == 1 && strncmp(arg, "/", 1) == 0) { >> > @@ -97,7 +80,6 @@ int canoncon(char **contextp) >> > *contextp = tmpcon; >> > } else if (errno != ENOENT) { >> > rc = -1; >> > - inc_err(); >> > } >> > >> > return rc; >> > -- >> > 2.30.0 >> >> >> If there's no objection I'd like to merge both patches before Wednesday >> so they'll part of rc2. > > I took a look and both patches look good to me. Nevertheless > policycoreutils/setfiles/setfiles.c stil contains a "static int nerr;" > which becomes unused, after this patch. This variable should probably > be dropped, for example with: > > diff --git a/policycoreutils/setfiles/setfiles.c > b/policycoreutils/setfiles/setfiles.c > index 92616571ef2a..f018d161aa9e 100644 > --- a/policycoreutils/setfiles/setfiles.c > +++ b/policycoreutils/setfiles/setfiles.c > @@ -19,7 +19,6 @@ static int warn_no_match; > static int null_terminated; > static int request_digest; > static struct restore_opts r_opts; > -static int nerr; > > #define STAT_BLOCK_SIZE 1 > > @@ -161,7 +160,6 @@ int main(int argc, char **argv) > warn_no_match = 0; > request_digest = 0; > policyfile = NULL; > - nerr = 0; > > r_opts.abort_on_error = 0; > r_opts.progname = strdup(argv[0]); > @@ -427,9 +425,6 @@ int main(int argc, char **argv) > r_opts.selabel_opt_digest = (request_digest ? (char *)1 : NULL); > r_opts.selabel_opt_path = altpath; > > - if (nerr) > - exit(-1); > - > restore_init(&r_opts); > > if (use_input_file) { > > This clean-up could be done after you merged the patches. So: > Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org> Merged. > Thanks! > Nicolas
diff --git a/policycoreutils/setfiles/Makefile b/policycoreutils/setfiles/Makefile index bc5a8db789a5..a3bbbe116b7f 100644 --- a/policycoreutils/setfiles/Makefile +++ b/policycoreutils/setfiles/Makefile @@ -5,8 +5,6 @@ SBINDIR ?= /sbin MANDIR = $(PREFIX)/share/man AUDITH ?= $(shell test -f /usr/include/libaudit.h && echo y) -ABORT_ON_ERRORS=$(shell grep "^\#define ABORT_ON_ERRORS" setfiles.c | awk -S '{ print $$3 }') - CFLAGS ?= -g -Werror -Wall -W override LDLIBS += -lselinux -lsepol @@ -26,7 +24,6 @@ restorecon_xattr: restorecon_xattr.o restore.o man: @cp -af setfiles.8 setfiles.8.man - @sed -i "s/ABORT_ON_ERRORS/$(ABORT_ON_ERRORS)/g" setfiles.8.man install: all [ -d $(DESTDIR)$(MANDIR)/man8 ] || mkdir -p $(DESTDIR)$(MANDIR)/man8 diff --git a/policycoreutils/setfiles/ru/setfiles.8 b/policycoreutils/setfiles/ru/setfiles.8 index 27815a3f1eee..910101452625 100644 --- a/policycoreutils/setfiles/ru/setfiles.8 +++ b/policycoreutils/setfiles/ru/setfiles.8 @@ -47,7 +47,7 @@ setfiles \- установить SELinux-контексты безопаснос проверить действительность контекстов относительно указанной двоичной политики. .TP .B \-d -показать, какая спецификация соответствует каждому из файлов (не прекращать проверку после получения ошибок ABORT_ON_ERRORS). +показать, какая спецификация соответствует каждому из файлов. .TP .BI \-e \ directory исключить каталог (чтобы исключить более одного каталога, этот параметр необходимо использовать соответствующее количество раз). diff --git a/policycoreutils/setfiles/setfiles.8 b/policycoreutils/setfiles/setfiles.8 index e328a5628682..4d28bc9a95c1 100644 --- a/policycoreutils/setfiles/setfiles.8 +++ b/policycoreutils/setfiles/setfiles.8 @@ -57,8 +57,7 @@ option will force a replacement of the entire context. check the validity of the contexts against the specified binary policy. .TP .B \-d -show what specification matched each file (do not abort validation -after ABORT_ON_ERRORS errors). +show what specification matched each file. .TP .BI \-e \ directory directory to exclude (repeat option for more than one directory). diff --git a/policycoreutils/setfiles/setfiles.c b/policycoreutils/setfiles/setfiles.c index 10692d6d94a0..92616571ef2a 100644 --- a/policycoreutils/setfiles/setfiles.c +++ b/policycoreutils/setfiles/setfiles.c @@ -23,14 +23,6 @@ static int nerr; #define STAT_BLOCK_SIZE 1 -/* setfiles will abort its operation after reaching the - * following number of errors (e.g. invalid contexts), - * unless it is used in "debug" mode (-d option). - */ -#ifndef ABORT_ON_ERRORS -#define ABORT_ON_ERRORS 10 -#endif - #define SETFILES "setfiles" #define RESTORECON "restorecon" static int iamrestorecon; @@ -56,15 +48,6 @@ static __attribute__((__noreturn__)) void usage(const char *const name) exit(-1); } -void inc_err(void) -{ - nerr++; - if (nerr > ABORT_ON_ERRORS - 1 && !r_opts.debug) { - fprintf(stderr, "Exiting after %d errors.\n", ABORT_ON_ERRORS); - exit(-1); - } -} - void set_rootpath(const char *arg) { if (strlen(arg) == 1 && strncmp(arg, "/", 1) == 0) { @@ -97,7 +80,6 @@ int canoncon(char **contextp) *contextp = tmpcon; } else if (errno != ENOENT) { rc = -1; - inc_err(); } return rc;
`setfiles -d` doesn't have any impact on number of errors before it aborts. It always aborts on first invalid context in spec file. Signed-off-by: Petr Lautrbach <plautrba@redhat.com> --- policycoreutils/setfiles/Makefile | 3 --- policycoreutils/setfiles/ru/setfiles.8 | 2 +- policycoreutils/setfiles/setfiles.8 | 3 +-- policycoreutils/setfiles/setfiles.c | 18 ------------------ 4 files changed, 2 insertions(+), 24 deletions(-)