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

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.