All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] setfiles: Fix setfiles progress indicator
@ 2017-01-27 12:47 Richard Haines
  2017-01-27 14:55 ` Stephen Smalley
  0 siblings, 1 reply; 3+ messages in thread
From: Richard Haines @ 2017-01-27 12:47 UTC (permalink / raw)
  To: selinux, alan.christopher.jenkins

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@gmail.com>
Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
---
 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			0x4000
 
 /**
  * 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);
 }
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] setfiles: Fix setfiles progress indicator
  2017-01-27 12:47 [PATCH] setfiles: Fix setfiles progress indicator Richard Haines
@ 2017-01-27 14:55 ` Stephen Smalley
  2017-01-28 12:21   ` Alan Jenkins
  0 siblings, 1 reply; 3+ messages in thread
From: Stephen Smalley @ 2017-01-27 14:55 UTC (permalink / raw)
  To: Richard Haines, selinux, alan.christopher.jenkins

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@gmail.com>
> Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>

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);
>  }

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] setfiles: Fix setfiles progress indicator
  2017-01-27 14:55 ` Stephen Smalley
@ 2017-01-28 12:21   ` Alan Jenkins
  0 siblings, 0 replies; 3+ messages in thread
From: Alan Jenkins @ 2017-01-28 12:21 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: Richard Haines, selinux

[-- Attachment #1: Type: text/plain, Size: 19430 bytes --]

On 27 January 2017 at 14:55, Stephen Smalley <sds@tycho.nsa.gov> wrote:

> 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@gmail.com>
> > Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
>
>
On 27/01/2017, stephensmalley <notifications@github.com> wrote:
> Fixed by 454768f56d7a941657d800e303994bca086b7546
> There is a change in the UX with respect to reporting progress
> per-filesystem; hopefully that isn't too noisy.
> Other, separate issue is likely that we ought to exclude more of these
> filesystems in fixfiles, as we used to do before introducing seclabel
mount
> option.

Thanks!  I'm sitting in a youth hostel with duff knees, rain outside, and a
Chromebook.  So I can't test this but I have a couple of comments I can
make.

I think the path e.g.`/etc` should be visible next to the percentage right
from the start.  Then I think this output would be fine.  (Because fixfiles
already prints the list of filesystems it's going to relabel at the start).

Having the current progress line showing without the path, but completed
progress lines showing with the path, seems less informative and actively
confusing.

I had another concern looking at the commit message though. The example
shown is suspicious.

The implication is it shows reasonable % progress, for each argument in
turn.  E.g. /etc goes from 0% to 100%, then then next argument is
processed.  But I can't see this working in the code.

/etc is never a separate filesystem (fstab is in /etc).  IIRC from comments
or commit messages, progress indication used the number of inodes on the
target filesystem.  In that case it couldn't possibly have shown progress
correctly for /etc, not as a separate line, so this made me curious.

Looking at the commit, accurate progress indication still seems to rely on
the inode count, so I still have this concern.  (`statvfs` and subtracting
# of files from the max inode count for the FS.  I wonder how this works on
btrfs).

I think the API documentation would want to be improved as well. I would
optimistically read it as saying I could set
SELINUX_RESTORECON_MASS_RELABEL anytime I relabel a large number of files
and want some nice % progress indicator.  But this cannot be true.  AFAICS,
the flag SELINUX_RESTORECON_MASS_RELABEL will only work correctly when
relabelling *everything*; all files on all supported filesystems.  That's
what `exclude_non_seclabel_mounts()` is counting.

Also - I've only checked this now, but I think `restorecon -p / /etc /tmp
/boot` is suspicious for a second reason.  Unlike `setfiles`, `restorecon`
does `xdev = 0; /* Follows mounts */`.  So I don't think it's a good
example command to pass other arguments along with `/`; I don't think you
would ever want to do this.

Regards
Alan


> 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);
> >  }
>

[-- Attachment #2: Type: text/html, Size: 25428 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-01-28 12:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-27 12:47 [PATCH] setfiles: Fix setfiles progress indicator Richard Haines
2017-01-27 14:55 ` Stephen Smalley
2017-01-28 12:21   ` Alan Jenkins

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.