On Fri, 2017-01-27 at 12:47 +0000, Richard Haines wrote:
> This fixes the following bug:
> UX regression: setfiles progress indicator is now misleading and
> confusing in fixfiles.
>
> The outputting of * is replaced by the number of files in 1k
> increments
> as the previous versions. If "/" is specified on the pathname, then
> this
> will indicate a mass relabel, an example output will be:
> restorecon -nRp /etc /tmp /boot /
> /etc 100.0%
> /tmp 100.0%
> /boot 100.0%
> 3.2%
>
> Also setfiles(8) and restorecon(8) versions that are implemented
> using
> the selinux_restorecon(3) function do not support the [-o filename]
> option as this was deprecated. This has now been made clear by
> displaying
> a message to stderr.
>
> The documentation has also been updated to reflect these changes.
>
> Reported-by: Alan Jenkins <alan.christopher.jenkins@xxxxxxxxx>
> Signed-off-by: Richard Haines <richard_c_haines@xxxxxxxxxxxxxx>
Thanks, applied.
> ---
> libselinux/include/selinux/restorecon.h | 11 +++++++---
> libselinux/man/man3/selinux_restorecon.3 | 12 +++++++++--
> libselinux/src/selinux_restorecon.c | 37 ++++++++------------
> ------------
> policycoreutils/setfiles/.gitignore | 1 -
> policycoreutils/setfiles/Makefile | 7 ++----
> policycoreutils/setfiles/restore.c | 3 ++-
> policycoreutils/setfiles/restore.h | 9 +-------
> policycoreutils/setfiles/restorecon.8 | 4 ++--
> policycoreutils/setfiles/setfiles.8 | 6 +++---
> policycoreutils/setfiles/setfiles.c | 27 +++++++---------------
> -
> 10 files changed, 45 insertions(+), 72 deletions(-)
>
> diff --git a/libselinux/include/selinux/restorecon.h
> b/libselinux/include/selinux/restorecon.h
> index 7cfdee1..de694cd 100644
> --- a/libselinux/include/selinux/restorecon.h
> +++ b/libselinux/include/selinux/restorecon.h
> @@ -50,9 +50,9 @@ extern int selinux_restorecon(const char *pathname,
> */
> #define SELINUX_RESTORECON_VERBOSE 0x0010
> /*
> - * Show progress by printing * to stdout every 1000 files, unless
> - * relabeling the entire OS, that will then show the approximate
> - * percentage complete.
> + * If SELINUX_RESTORECON_PROGRESS is true and
> + * SELINUX_RESTORECON_MASS_RELABEL is true, then output approx %
> complete,
> + * else output the number of files in 1k blocks processed to stdout.
> */
> #define SELINUX_RESTORECON_PROGRESS 0x0020
> /*
> @@ -91,6 +91,11 @@ extern int selinux_restorecon(const char
> *pathname,
> * mounts to be excluded from relabeling checks.
> */
> #define SELINUX_RESTORECON_IGNORE_MOUNTS 0x2000
> +/*
> + * Set if there is a mass relabel required.
> + * See SELINUX_RESTORECON_PROGRESS flag for details.
> + */
> +#define SELINUX_RESTORECON_MASS_RELABEL 0x400
> 0
>
> /**
> * selinux_restorecon_set_sehandle - Set the global fc handle.
> diff --git a/libselinux/man/man3/selinux_restorecon.3
> b/libselinux/man/man3/selinux_restorecon.3
> index d698818..1eac6ed 100644
> --- a/libselinux/man/man3/selinux_restorecon.3
> +++ b/libselinux/man/man3/selinux_restorecon.3
> @@ -88,8 +88,16 @@ will take precedence.
> .RE
> .sp
> .B SELINUX_RESTORECON_PROGRESS
> -show progress by printing * to stdout every 1000 files unless
> relabeling the
> -entire OS, that will then show the approximate percentage complete.
> +show progress by outputting the number of files in 1k blocks
> processed
> +to stdout. If the
> +.B SELINUX_RESTORECON_MASS_RELABEL
> +flag is also set then the approximate percentage complete will be
> shown.
> +.sp
> +.B SELINUX_RESTORECON_MASS_RELABEL
> +generally set when relabeling the entire OS, that will then show the
> +approximate percentage complete. The
> +.B SELINUX_RESTORECON_PROGRESS
> +flag must also be set.
> .sp
> .B SELINUX_RESTORECON_REALPATH
> convert passed-in
> diff --git a/libselinux/src/selinux_restorecon.c
> b/libselinux/src/selinux_restorecon.c
> index 38acbd2..9fdafea 100644
> --- a/libselinux/src/selinux_restorecon.c
> +++ b/libselinux/src/selinux_restorecon.c
> @@ -41,7 +41,7 @@
> #define SYS_PATH "/sys"
> #define SYS_PREFIX SYS_PATH "/"
>
> -#define STAR_COUNT 1000
> +#define STAR_COUNT 1024
>
> static struct selabel_handle *fc_sehandle = NULL;
> static unsigned char *fc_digest = NULL;
> @@ -68,18 +68,12 @@ static uint64_t efile_count; /* Estimated
> total number of files */
> struct dir_xattr *dir_xattr_list;
> static struct dir_xattr *dir_xattr_last;
>
> -/*
> - * If SELINUX_RESTORECON_PROGRESS is set and mass_relabel = true,
> then
> - * output approx % complete, else output * for every STAR_COUNT
> files
> - * processed to stdout.
> - */
> -static bool mass_relabel;
> -
> /* restorecon_flags for passing to restorecon_sb() */
> struct rest_flags {
> bool nochange;
> bool verbose;
> bool progress;
> + bool mass_relabel;
> bool set_specctx;
> bool add_assoc;
> bool ignore_digest;
> @@ -631,14 +625,14 @@ static int restorecon_sb(const char *pathname,
> const struct stat *sb,
> if (flags->progress) {
> fc_count++;
> if (fc_count % STAR_COUNT == 0) {
> - if (mass_relabel && efile_count > 0) {
> + if (flags->mass_relabel && efile_count > 0)
> {
> pc = (fc_count < efile_count) ?
> (100.0 *
> fc_count / efile_count)
> : 100;
> fprintf(stdout, "\r%-.1f%%",
> (double)pc);
> } else {
> - fprintf(stdout, "*");
> + fprintf(stdout, "\r%luk", fc_count /
> STAR_COUNT);
> }
> - fflush(stdout);
> + fflush(stdout);
> }
> }
>
> @@ -750,6 +744,8 @@ int selinux_restorecon(const char *pathname_orig,
> SELINUX_RESTORECON_VERBOSE) ? true : false;
> flags.progress = (restorecon_flags &
> SELINUX_RESTORECON_PROGRESS) ? true : false;
> + flags.mass_relabel = (restorecon_flags &
> + SELINUX_RESTORECON_MASS_RELABEL) ? true : false;
> flags.recurse = (restorecon_flags &
> SELINUX_RESTORECON_RECURSE) ? true : false;
> flags.set_specctx = (restorecon_flags &
> @@ -904,17 +900,6 @@ int selinux_restorecon(const char
> *pathname_orig,
> }
> }
>
> - mass_relabel = false;
> - if (!strcmp(pathname, "/")) {
> - mass_relabel = true;
> - if (flags.set_xdev && flags.progress)
> - /*
> - * Need to recalculate to get accurate %
> complete
> - * as only root device id will be processed.
> - */
> - efile_count = file_system_count(pathname);
> - }
> -
> if (flags.set_xdev)
> fts_flags = FTS_PHYSICAL | FTS_NOCHDIR | FTS_XDEV;
> else
> @@ -1009,12 +994,8 @@ int selinux_restorecon(const char
> *pathname_orig,
> }
>
> out:
> - if (flags.progress) {
> - if (mass_relabel)
> - fprintf(stdout, "\r100.0%%\n");
> - else
> - fprintf(stdout, "\n");
> - }
> + if (flags.progress && flags.mass_relabel)
> + fprintf(stdout, "\r%s 100.0%%\n", pathname);
>
> sverrno = errno;
> (void) fts_close(fts);
> diff --git a/policycoreutils/setfiles/.gitignore
> b/policycoreutils/setfiles/.gitignore
> index 583eb6c..5e899c9 100644
> --- a/policycoreutils/setfiles/.gitignore
> +++ b/policycoreutils/setfiles/.gitignore
> @@ -1,2 +1 @@
> -restorecon.8.man
> setfiles.8.man
> diff --git a/policycoreutils/setfiles/Makefile
> b/policycoreutils/setfiles/Makefile
> index 43364f9..92300c9 100644
> --- a/policycoreutils/setfiles/Makefile
> +++ b/policycoreutils/setfiles/Makefile
> @@ -5,7 +5,6 @@ MANDIR = $(PREFIX)/share/man
> LIBDIR ?= $(PREFIX)/lib
> AUDITH = $(shell ls /usr/include/libaudit.h 2>/dev/null)
>
> -PROGRESS_STEP=$(shell grep "^\#define STAR_COUNT" restore.h | awk -S
> '{ print $$3 }')
> ABORT_ON_ERRORS=$(shell grep "^\#define ABORT_ON_ERRORS" setfiles.c
> | awk -S '{ print $$3 }')
>
> CFLAGS ?= -g -Werror -Wall -W
> @@ -28,8 +27,6 @@ restorecon_xattr: restorecon_xattr.o restore.o
>
> man:
> @cp -af setfiles.8 setfiles.8.man
> - @cp -af restorecon.8 restorecon.8.man
> - @sed -i "s/STAR_COUNT/$(PROGRESS_STEP)/g" setfiles.8.man
> restorecon.8.man
> @sed -i "s/ABORT_ON_ERRORS/$(ABORT_ON_ERRORS)/g"
> setfiles.8.man
>
> install: all
> @@ -39,11 +36,11 @@ install: all
> (cd $(SBINDIR) && ln -sf setfiles restorecon)
> install -m 755 restorecon_xattr $(SBINDIR)
> install -m 644 setfiles.8.man $(MANDIR)/man8/setfiles.8
> - install -m 644 restorecon.8.man $(MANDIR)/man8/restorecon.8
> + install -m 644 restorecon.8 $(MANDIR)/man8/restorecon.8
> install -m 644 restorecon_xattr.8
> $(MANDIR)/man8/restorecon_xattr.8
>
> clean:
> - rm -f setfiles restorecon restorecon_xattr *.o
> setfiles.8.man restorecon.8.man
> + rm -f setfiles restorecon restorecon_xattr *.o
> setfiles.8.man
>
> indent:
> ../../scripts/Lindent $(wildcard *.[ch])
> diff --git a/policycoreutils/setfiles/restore.c
> b/policycoreutils/setfiles/restore.c
> index cf04e96..50d192a 100644
> --- a/policycoreutils/setfiles/restore.c
> +++ b/policycoreutils/setfiles/restore.c
> @@ -35,7 +35,8 @@ void restore_init(struct restore_opts *opts)
> r_opts->recurse | r_opts->userealpath |
> r_opts->xdev | r_opts->abort_on_error |
> r_opts->syslog_changes | r_opts-
> >log_matches |
> - r_opts->ignore_noent | r_opts-
> >ignore_mounts;
> + r_opts->ignore_noent | r_opts-
> >ignore_mounts |
> + r_opts->mass_relabel;
>
> /* Use setfiles, restorecon and restorecond own handles */
> selinux_restorecon_set_sehandle(r_opts->hnd);
> diff --git a/policycoreutils/setfiles/restore.h
> b/policycoreutils/setfiles/restore.h
> index 97fbdf4..b64042a 100644
> --- a/policycoreutils/setfiles/restore.h
> +++ b/policycoreutils/setfiles/restore.h
> @@ -17,18 +17,12 @@
> #include <limits.h>
> #include <stdint.h>
>
> -/*
> - * STAR_COUNT is also defined in libselinux/src/selinux_restorecon.c
> where it
> - * is used to output "*" for each number of files processed. Defined
> here for
> - * inclusion in man pages.
> -*/
> -#define STAR_COUNT 1000
> -
> /* Things that need to be init'd */
> struct restore_opts {
> unsigned int nochange;
> unsigned int verbose;
> unsigned int progress;
> + unsigned int mass_relabel;
> unsigned int set_specctx;
> unsigned int add_assoc;
> unsigned int ignore_digest;
> @@ -49,7 +43,6 @@ struct restore_opts {
> const char *selabel_opt_path;
> const char *selabel_opt_digest;
> int debug;
> - FILE *outfile;
> };
>
> void restore_init(struct restore_opts *opts);
> diff --git a/policycoreutils/setfiles/restorecon.8
> b/policycoreutils/setfiles/restorecon.8
> index bd27113..0f81db4 100644
> --- a/policycoreutils/setfiles/restorecon.8
> +++ b/policycoreutils/setfiles/restorecon.8
> @@ -115,10 +115,10 @@ don't change any file labels (passive
> check). To display the files whose labels
> .BR \-v .
> .TP
> .BI \-o \ outfilename
> -Deprecated, SELinux policy will probably block this access. Use
> shell redirection to save list of files with incorrect context in
> filename.
> +Deprecated - This option is no longer supported.
> .TP
> .B \-p
> -show progress by printing * every STAR_COUNT files unless relabeling
> the entire
> +show progress by printing the number of files in 1k blocks unless
> relabeling the entire
> OS, that will then show the approximate percentage complete. Note
> that the
> .B \-p
> and
> diff --git a/policycoreutils/setfiles/setfiles.8
> b/policycoreutils/setfiles/setfiles.8
> index 6901e13..9501845 100644
> --- a/policycoreutils/setfiles/setfiles.8
> +++ b/policycoreutils/setfiles/setfiles.8
> @@ -106,11 +106,11 @@ seclabel fs mounted on a directory below this.
> .B \-n
> don't change any file labels (passive check).
> .TP
> -.BI \-o \ filename
> -Deprecated, SELinux policy will probably block this access. Use
> shell redirection to save list of files with incorrect context in
> filename.
> +.BI \-o \ outfilename
> +Deprecated - This option is no longer supported.
> .TP
> .B \-p
> -show progress by printing * every STAR_COUNT files unless relabeling
> the entire
> +show progress by printing the number of files in 1k blocks unless
> relabeling the entire
> OS, that will then show the approximate percentage complete. Note
> that the
> .B \-p
> and
> diff --git a/policycoreutils/setfiles/setfiles.c
> b/policycoreutils/setfiles/setfiles.c
> index d767131..e3df212 100644
> --- a/policycoreutils/setfiles/setfiles.c
> +++ b/policycoreutils/setfiles/setfiles.c
> @@ -160,7 +160,7 @@ int main(int argc, char **argv)
> char *buf = NULL;
> size_t buf_len;
> const char *base;
> - int mass_relabel = 0, errors = 0;
> + int errors = 0;
> const char *ropts = "e:f:hiIDlmno:pqrsvFRW0";
> const char *sopts = "c:de:f:hiIDlmno:pqr:svFR:W0";
> const char *opts;
> @@ -318,19 +318,8 @@ int main(int argc, char **argv)
> r_opts.nochange =
> SELINUX_RESTORECON_NOCHANGE;
> break;
> case 'o': /* Deprecated */
> - if (strcmp(optarg, "-") == 0) {
> - r_opts.outfile = stdout;
> - break;
> - }
> -
> - r_opts.outfile = fopen(optarg, "w");
> - if (!r_opts.outfile) {
> - fprintf(stderr, "Error opening %s:
> %s\n",
> - optarg, strerror(errno));
> -
> - usage(argv[0]);
> - }
> - __fsetlocking(r_opts.outfile,
> FSETLOCKING_BYCALLER);
> + fprintf(stderr, "%s: -o option no longer
> supported\n",
> + r_opts.progname);
> break;
> case 'q':
> /* Deprecated - Was only used to say whether
> print
> @@ -394,7 +383,7 @@ int main(int argc, char **argv)
>
> for (i = optind; i < argc; i++) {
> if (!strcmp(argv[i], "/"))
> - mass_relabel = 1;
> + r_opts.mass_relabel =
> SELINUX_RESTORECON_MASS_RELABEL;
> }
>
> cb.func_log = log_callback;
> @@ -466,7 +455,7 @@ int main(int argc, char **argv)
> while ((len = getdelim(&buf, &buf_len, delim, f)) >
> 0) {
> buf[len - 1] = 0;
> if (!strcmp(buf, "/"))
> - mass_relabel = 1;
> + r_opts.mass_relabel =
> SELINUX_RESTORECON_MASS_RELABEL;
> errors |= process_glob(buf, &r_opts) < 0;
> }
> if (strcmp(input_filename, "-") != 0)
> @@ -476,7 +465,7 @@ int main(int argc, char **argv)
> errors |= process_glob(argv[i], &r_opts) <
> 0;
> }
>
> - maybe_audit_mass_relabel(mass_relabel, errors);
> + maybe_audit_mass_relabel(r_opts.mass_relabel, errors);
>
> if (warn_no_match)
> selabel_stats(r_opts.hnd);
> @@ -484,8 +473,8 @@ int main(int argc, char **argv)
> selabel_close(r_opts.hnd);
> restore_finish();
>
> - if (r_opts.outfile)
> - fclose(r_opts.outfile);
> + if (r_opts.progress)
> + fprintf(stdout, "\n");
>
> exit(errors ? -1 : 0);
> }
_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.
