All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/6] Revert "policycoreutils: let output of `fixfiles` be redirected (as normal)"
@ 2017-05-04 17:01 Alan Jenkins
  2017-05-04 17:01 ` [PATCH 2/6] policycoreutils: fixfiles: remove (broken) redundant code Alan Jenkins
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Alan Jenkins @ 2017-05-04 17:01 UTC (permalink / raw)
  To: selinux; +Cc: Alan Jenkins

This reverts commit ac7899fc3ad6221e195dd13cdf14b346897314ae,
which is not yet part of an officially tagged release
(or release candidate).

`LOGFILE=/proc/self/fd/1` was wrong.

`LOGFILE=$(tty)` was being relied on in one case (exclude_dirs),
to log messages from a function run specifically with stdout redirected
(captured into a variable).

Having `logit "message"` break inside redirected functions
is a nasty leaky abstraction.

This caused e.g. `fixfiles restore` to terminate early with the error

    skipping: No such file or directory

if the user had configured any excluded paths in
/etc/selinux/fixfiles_exclude_dirs
---
 policycoreutils/scripts/fixfiles | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/policycoreutils/scripts/fixfiles b/policycoreutils/scripts/fixfiles
index bc74d69..75d7762 100755
--- a/policycoreutils/scripts/fixfiles
+++ b/policycoreutils/scripts/fixfiles
@@ -119,7 +119,11 @@ VERBOSE="-p"
 FORCEFLAG=""
 DIRS=""
 RPMILES=""
-LOGFILE=/proc/self/fd/1
+LOGFILE=`tty`
+if [ $? != 0 ]; then
+    LOGFILE="/dev/null"
+fi
+LOGGER=/usr/sbin/logger
 SETFILES=/sbin/setfiles
 RESTORECON=/sbin/restorecon
 FILESYSTEMSRW=`get_rw_labeled_mounts`
@@ -134,11 +138,11 @@ else
 fi
 
 #
-# Write to LOGFILE
+# Log to either syslog or a LOGFILE
 #
 logit () {
 if [ -n $LOGFILE ]; then
-    echo $1 >> "$LOGFILE"
+    echo $1 >> $LOGFILE
 fi
 }
 #
-- 
2.9.3

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

* [PATCH 2/6] policycoreutils: fixfiles: remove (broken) redundant code
  2017-05-04 17:01 [PATCH 1/6] Revert "policycoreutils: let output of `fixfiles` be redirected (as normal)" Alan Jenkins
@ 2017-05-04 17:01 ` Alan Jenkins
  2017-05-04 17:01 ` [PATCH 3/6] policycoreutils: fixfiles: clarify exclude_dirs() Alan Jenkins
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Alan Jenkins @ 2017-05-04 17:01 UTC (permalink / raw)
  To: selinux; +Cc: Alan Jenkins

setfiles is now run with $exclude_dirs.
We shouldn't need to patch the file contexts as well.

This is fortunate, since the file context patching code was broken
(by the same commit which introduced the redundancy).  It takes the
list of directories to exclude from $tempdirs, but $tempdirs is
never set.

Also messages about skipping directories were printed twice.  Firstly when
exclude_dirs is generated, and secondly in the file context patching code.

Also TEMPFCFILE was only removed in one path out of several.
---
 policycoreutils/scripts/fixfiles | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/policycoreutils/scripts/fixfiles b/policycoreutils/scripts/fixfiles
index 75d7762..7e5ce88 100755
--- a/policycoreutils/scripts/fixfiles
+++ b/policycoreutils/scripts/fixfiles
@@ -230,21 +230,6 @@ fi
 LogReadOnly
 #
 exclude_dirs="`exclude_dirs_from_relabelling $OPTION`"
-if [ -n "${exclude_dirs}" ]
-then
-	TEMPFCFILE=`mktemp ${FC}.XXXXXXXXXX`
-	test -z "$TEMPFCFILE" && exit
-	/bin/cp -p ${FC} ${TEMPFCFILE} &>/dev/null || exit
-	tmpdirs=${tempdirs//-e/}
-	for p in ${tmpdirs}
-	do
-		p="${p%/}"
-		p1="${p}(/.*)? -- <<none>>"
-		echo "${p1}" >> $TEMPFCFILE
-		logit "skipping the directory ${p}"
-	done
-FC=$TEMPFCFILE
-fi
 if [ ! -z "$RPMFILES" ]; then
     for i in `echo "$RPMFILES" | sed 's/,/ /g'`; do
 	rpmlist $i | ${RESTORECON} $exclude_dirs ${FORCEFLAG} ${VERBOSE} $* -R -i -f - >>$LOGFILE 2>&1
@@ -265,7 +250,7 @@ if [ ${OPTION} != "Relabel" ]; then
     return
 fi
 echo "Cleaning up labels on /tmp"
-rm -rf /tmp/gconfd-* /tmp/pulse-* /tmp/orbit-* $TEMPFCFILE
+rm -rf /tmp/gconfd-* /tmp/pulse-* /tmp/orbit-*
 
 UNDEFINED=`get_undefined_type` || exit $?
 UNLABELED=`get_unlabeled_type` || exit $?
-- 
2.9.3

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

* [PATCH 3/6] policycoreutils: fixfiles: clarify exclude_dirs()
  2017-05-04 17:01 [PATCH 1/6] Revert "policycoreutils: let output of `fixfiles` be redirected (as normal)" Alan Jenkins
  2017-05-04 17:01 ` [PATCH 2/6] policycoreutils: fixfiles: remove (broken) redundant code Alan Jenkins
@ 2017-05-04 17:01 ` Alan Jenkins
  2017-05-04 17:01 ` [PATCH 4/6] policycoreutils: fixfiles: fix logging about R/O filesystems Alan Jenkins
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Alan Jenkins @ 2017-05-04 17:01 UTC (permalink / raw)
  To: selinux; +Cc: Alan Jenkins

The usage of exclude_dirs() is non-obvious.

It turns out it is only used by the `-C` mode of fixfiles.  The other four
modes use the narrower list generated by exclude_dirs_from_relabelling().
Let's make this distinction more obvious.

(The purpose of the extra exclusions is not clear.  E.g. there's an
exclusion for /dev.  Whereas the `fixfiles check` mode explicitly tells you
that it's going to relabel /dev, without causing any problem.  Maybe that
part is out of date?  But without some explanation of the list, I don't
want to change anything!)
---
 policycoreutils/scripts/fixfiles | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/policycoreutils/scripts/fixfiles b/policycoreutils/scripts/fixfiles
index 7e5ce88..dae073d 100755
--- a/policycoreutils/scripts/fixfiles
+++ b/policycoreutils/scripts/fixfiles
@@ -101,15 +101,6 @@ exclude_dirs_from_relabelling() {
     echo "$exclude_from_relabelling"
 }
 
-exclude_dirs() {
-    exclude=
-    for i in /sys /proc /dev /run /mnt /var/tmp /var/lib/BackupPC /home /tmp /dev; do
-	[ -e $i ]  && exclude="$exclude -e $i";
-    done
-    exclude="$exclude `exclude_dirs_from_relabelling`"
-    echo "$exclude"
-}
-
 #
 # Set global Variables
 #
@@ -162,6 +153,12 @@ newer() {
 # run restorecon on all files affected by the differences.
 #
 diff_filecontext() {
+exclude_dirs=
+for i in /sys /proc /dev /run /mnt /var/tmp /var/lib/BackupPC /home /tmp /dev; do
+    [ -e $i ]  && exclude_dirs="$exclude_dirs -e $i";
+done
+exclude_dirs="$exclude_dirs `exclude_dirs_from_relabelling`"
+
 if [ -f ${PREFC} -a -x /usr/bin/diff ]; then
 	TEMPFILE=`mktemp ${FC}.XXXXXXXXXX`
 	test -z "$TEMPFILE" && exit
@@ -191,7 +188,7 @@ if [ -f ${PREFC} -a -x /usr/bin/diff ]; then
 		  esac; \
 	       fi; \
 	    done | \
-	${RESTORECON} ${VERBOSE} -i -f - -R $* `exclude_dirs`; \
+	${RESTORECON} ${VERBOSE} -i -f - -R $* $exclude_dirs; \
 	rm -f ${TEMPFILE} ${PREFCTEMPFILE}
 fi
 }
-- 
2.9.3

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

* [PATCH 4/6] policycoreutils: fixfiles: fix logging about R/O filesystems
  2017-05-04 17:01 [PATCH 1/6] Revert "policycoreutils: let output of `fixfiles` be redirected (as normal)" Alan Jenkins
  2017-05-04 17:01 ` [PATCH 2/6] policycoreutils: fixfiles: remove (broken) redundant code Alan Jenkins
  2017-05-04 17:01 ` [PATCH 3/6] policycoreutils: fixfiles: clarify exclude_dirs() Alan Jenkins
@ 2017-05-04 17:01 ` Alan Jenkins
  2017-05-04 17:01 ` [PATCH 5/6] policycoreutils: fixfiles: move logit call outside of redirected function Alan Jenkins
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Alan Jenkins @ 2017-05-04 17:01 UTC (permalink / raw)
  To: selinux; +Cc: Alan Jenkins

The LogReadOnly() call which warns the user about R/O filesystems, applies
to the `-B` mode (newer() function), and the `fixfiles check` mode
(no paths).

Make sure to print it for these modes, and these modes only.
---
 policycoreutils/scripts/fixfiles | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/policycoreutils/scripts/fixfiles b/policycoreutils/scripts/fixfiles
index dae073d..01bf97c 100755
--- a/policycoreutils/scripts/fixfiles
+++ b/policycoreutils/scripts/fixfiles
@@ -136,16 +136,27 @@ if [ -n $LOGFILE ]; then
     echo $1 >> $LOGFILE
 fi
 }
+
+#
+# Log all Read Only file systems
+#
+LogReadOnly() {
+if [ ! -z "$FILESYSTEMSRO" ]; then
+    logit "Warning: Skipping the following R/O filesystems:"
+    logit "$FILESYSTEMSRO"
+fi
+}
+
 #
 # Find files newer then the passed in date and fix the label
 #
 newer() {
     DATE=$1
     shift
+    LogReadOnly
     for m in `echo $FILESYSTEMSRW`; do
 	find $m -mount -newermt $DATE -print0 2>/dev/null | ${RESTORECON} ${FORCEFLAG} ${VERBOSE} $* -i -0 -f -
     done;
-
 }
 
 #
@@ -192,15 +203,6 @@ if [ -f ${PREFC} -a -x /usr/bin/diff ]; then
 	rm -f ${TEMPFILE} ${PREFCTEMPFILE}
 fi
 }
-#
-# Log all Read Only file systems
-#
-LogReadOnly() {
-if [ ! -z "$FILESYSTEMSRO" ]; then
-    logit "Warning: Skipping the following R/O filesystems:"
-    logit "$FILESYSTEMSRO"
-fi
-}
 
 rpmlist() {
 rpm -q --qf '[%{FILESTATES} %{FILENAMES}\n]' "$1" | grep '^0 ' | cut -f2- -d ' '
@@ -224,9 +226,8 @@ if [ ! -z "$BOOTTIME" ]; then
     exit $?
 fi
 [ -x /usr/sbin/genhomedircon ] && /usr/sbin/genhomedircon
-LogReadOnly
-#
-exclude_dirs="`exclude_dirs_from_relabelling $OPTION`"
+
+exclude_dirs="`exclude_dirs_from_relabelling`"
 if [ ! -z "$RPMFILES" ]; then
     for i in `echo "$RPMFILES" | sed 's/,/ /g'`; do
 	rpmlist $i | ${RESTORECON} $exclude_dirs ${FORCEFLAG} ${VERBOSE} $* -R -i -f - >>$LOGFILE 2>&1
@@ -238,6 +239,7 @@ if [ ! -z "$FILEPATH" ]; then
     return
 fi
 if [  -n "${FILESYSTEMSRW}" ]; then
+    LogReadOnly
     echo "${OPTION}ing `echo ${FILESYSTEMSRW}`"
     ${SETFILES} ${VERBOSE} $exclude_dirs -q ${FORCEFLAG} $* ${FC} ${FILESYSTEMSRW} >>$LOGFILE 2>&1
 else
-- 
2.9.3

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

* [PATCH 5/6] policycoreutils: fixfiles: move logit call outside of redirected function
  2017-05-04 17:01 [PATCH 1/6] Revert "policycoreutils: let output of `fixfiles` be redirected (as normal)" Alan Jenkins
                   ` (2 preceding siblings ...)
  2017-05-04 17:01 ` [PATCH 4/6] policycoreutils: fixfiles: fix logging about R/O filesystems Alan Jenkins
@ 2017-05-04 17:01 ` Alan Jenkins
  2017-05-04 17:01 ` [PATCH 6/6] policycoreutils: fixfiles: deprecate -l option Alan Jenkins
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Alan Jenkins @ 2017-05-04 17:01 UTC (permalink / raw)
  To: selinux; +Cc: Alan Jenkins

Move call to logit() outside a function which has its output redirected.
See next commit for explanation.

The logit calls are moved into a new function LogExcluded(), similar to
LogReadOnly().  I don't see a pretty way to resolve this, so I just went
for the most explicit approach I could think of.

Behaviour change: diff_filecontext will now log *all* excluded paths.
I think that approach is an improvement, because e.g. the fact that `-C`
mode excludes `/home` was not previouslly documented anywhere.
---
 policycoreutils/scripts/fixfiles | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/policycoreutils/scripts/fixfiles b/policycoreutils/scripts/fixfiles
index 01bf97c..183efe9 100755
--- a/policycoreutils/scripts/fixfiles
+++ b/policycoreutils/scripts/fixfiles
@@ -95,7 +95,6 @@ exclude_dirs_from_relabelling() {
 	  [[ ! "${i}" =~ ^/.* ]] && continue
 	  [[ ! -d "${i}" ]] && continue
 	  exclude_from_relabelling="$exclude_from_relabelling -e $i"
-	  logit "skipping the directory $i"
 	done < /etc/selinux/fixfiles_exclude_dirs
     fi
     echo "$exclude_from_relabelling"
@@ -148,6 +147,15 @@ fi
 }
 
 #
+# Log directories excluded from relabelling by configuration file
+#
+LogExcluded() {
+for i in ${EXCLUDEDIRS//-e / }; do
+    logit "skipping the directory $i"
+done
+}
+
+#
 # Find files newer then the passed in date and fix the label
 #
 newer() {
@@ -164,11 +172,11 @@ newer() {
 # run restorecon on all files affected by the differences.
 #
 diff_filecontext() {
-exclude_dirs=
+EXCLUDEDIRS="`exclude_dirs_from_relabelling`"
 for i in /sys /proc /dev /run /mnt /var/tmp /var/lib/BackupPC /home /tmp /dev; do
-    [ -e $i ]  && exclude_dirs="$exclude_dirs -e $i";
+    [ -e $i ]  && EXCLUDEDIRS="${EXCLUDEDIRS} -e $i";
 done
-exclude_dirs="$exclude_dirs `exclude_dirs_from_relabelling`"
+LogExcluded
 
 if [ -f ${PREFC} -a -x /usr/bin/diff ]; then
 	TEMPFILE=`mktemp ${FC}.XXXXXXXXXX`
@@ -199,7 +207,7 @@ if [ -f ${PREFC} -a -x /usr/bin/diff ]; then
 		  esac; \
 	       fi; \
 	    done | \
-	${RESTORECON} ${VERBOSE} -i -f - -R $* $exclude_dirs; \
+	${RESTORECON} ${VERBOSE} -i -f - -R $* ${EXCLUDEDIRS}; \
 	rm -f ${TEMPFILE} ${PREFCTEMPFILE}
 fi
 }
@@ -227,21 +235,23 @@ if [ ! -z "$BOOTTIME" ]; then
 fi
 [ -x /usr/sbin/genhomedircon ] && /usr/sbin/genhomedircon
 
-exclude_dirs="`exclude_dirs_from_relabelling`"
+EXCLUDEDIRS="`exclude_dirs_from_relabelling`"
+LogExcluded
+
 if [ ! -z "$RPMFILES" ]; then
     for i in `echo "$RPMFILES" | sed 's/,/ /g'`; do
-	rpmlist $i | ${RESTORECON} $exclude_dirs ${FORCEFLAG} ${VERBOSE} $* -R -i -f - >>$LOGFILE 2>&1
+	rpmlist $i | ${RESTORECON} ${EXCLUDEDIRS} ${FORCEFLAG} ${VERBOSE} $* -R -i -f - >>$LOGFILE 2>&1
     done
     exit $?
 fi
 if [ ! -z "$FILEPATH" ]; then
-    ${RESTORECON} $exclude_dirs ${FORCEFLAG} ${VERBOSE} -R $* -- "$FILEPATH" >>$LOGFILE 2>&1
+    ${RESTORECON} ${EXCLUDEDIRS} ${FORCEFLAG} ${VERBOSE} -R $* -- "$FILEPATH" >>$LOGFILE 2>&1
     return
 fi
 if [  -n "${FILESYSTEMSRW}" ]; then
     LogReadOnly
     echo "${OPTION}ing `echo ${FILESYSTEMSRW}`"
-    ${SETFILES} ${VERBOSE} $exclude_dirs -q ${FORCEFLAG} $* ${FC} ${FILESYSTEMSRW} >>$LOGFILE 2>&1
+    ${SETFILES} ${VERBOSE} ${EXCLUDEDIRS} -q ${FORCEFLAG} $* ${FC} ${FILESYSTEMSRW} >>$LOGFILE 2>&1
 else
     echo >&2 "fixfiles: No suitable file systems found"
 fi
-- 
2.9.3

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

* [PATCH 6/6] policycoreutils: fixfiles: deprecate -l option
  2017-05-04 17:01 [PATCH 1/6] Revert "policycoreutils: let output of `fixfiles` be redirected (as normal)" Alan Jenkins
                   ` (3 preceding siblings ...)
  2017-05-04 17:01 ` [PATCH 5/6] policycoreutils: fixfiles: move logit call outside of redirected function Alan Jenkins
@ 2017-05-04 17:01 ` Alan Jenkins
  2017-05-04 17:28 ` [PATCH 1/6] Revert "policycoreutils: let output of `fixfiles` be redirected (as normal)" Dominick Grift
  2017-05-05 17:31 ` James Carter
  6 siblings, 0 replies; 9+ messages in thread
From: Alan Jenkins @ 2017-05-04 17:01 UTC (permalink / raw)
  To: selinux; +Cc: Alan Jenkins

...and write log messages to standard output.

Some versions of fixfiles in 2004 created a logfile by default.
Apparently they also used `tee` to log to standard output at the same time.
We're also told that the logfile was implemented because there was too
much output generated for use on a tty, and it scrolled out of reach.

https://bugzilla.redhat.com/show_bug.cgi?id=131707

In the current version, none of these original reasons for `-l` remain.

The logfile is not created by default.  If no log file is specified,
messages are written to stdin [sic]... if and only stdin is a tty.  If
stdin is not a tty, the log defaults to /dev/null.

When a user runs fixfiles on a tty and finds there is too much output, she
is likely to try redirecting standard output and/or standard error using
the shell.  She will find this doesn't help, because fixfiles is writing
the verbose log messages to standard input.

I tried to fix the problem non-intrusively, by changing the default log
file to `/dev/stdout`.  Sadly, this breaks down where you have
`echo >>$LOGFILE "Log message"` inside a specific function, which is run
with output redirected in order to "return" a string value (captured
into a variable).  exclude_dirs_from_relabelling() was such a function.

I was trying to abstract over writing to both normal files and stdout, but
my abstraction "leaks" in a non-obvious way.

There is a simple solution.  We can write the log messages to standard
output.  When we are passed `-l` by a legacy script, we can redirect
standard output to the logfile.

This removes any distinctions between the logfile and "non-log" messages.
Some calls to restorecon were missing redirections to the log file.
"Cleaning out /tmp" was written to the log file, but "Cleaning out labels
on /tmp" was not.  There were no comments to explain these distinctions.
---
 policycoreutils/scripts/fixfiles | 36 ++++++++++++------------------------
 1 file changed, 12 insertions(+), 24 deletions(-)

diff --git a/policycoreutils/scripts/fixfiles b/policycoreutils/scripts/fixfiles
index 183efe9..c876432 100755
--- a/policycoreutils/scripts/fixfiles
+++ b/policycoreutils/scripts/fixfiles
@@ -109,11 +109,6 @@ VERBOSE="-p"
 FORCEFLAG=""
 DIRS=""
 RPMILES=""
-LOGFILE=`tty`
-if [ $? != 0 ]; then
-    LOGFILE="/dev/null"
-fi
-LOGGER=/usr/sbin/logger
 SETFILES=/sbin/setfiles
 RESTORECON=/sbin/restorecon
 FILESYSTEMSRW=`get_rw_labeled_mounts`
@@ -128,21 +123,12 @@ else
 fi
 
 #
-# Log to either syslog or a LOGFILE
-#
-logit () {
-if [ -n $LOGFILE ]; then
-    echo $1 >> $LOGFILE
-fi
-}
-
-#
 # Log all Read Only file systems
 #
 LogReadOnly() {
 if [ ! -z "$FILESYSTEMSRO" ]; then
-    logit "Warning: Skipping the following R/O filesystems:"
-    logit "$FILESYSTEMSRO"
+    echo "Warning: Skipping the following R/O filesystems:"
+    echo "$FILESYSTEMSRO"
 fi
 }
 
@@ -151,7 +137,7 @@ fi
 #
 LogExcluded() {
 for i in ${EXCLUDEDIRS//-e / }; do
-    logit "skipping the directory $i"
+    echo "skipping the directory $i"
 done
 }
 
@@ -240,18 +226,18 @@ LogExcluded
 
 if [ ! -z "$RPMFILES" ]; then
     for i in `echo "$RPMFILES" | sed 's/,/ /g'`; do
-	rpmlist $i | ${RESTORECON} ${EXCLUDEDIRS} ${FORCEFLAG} ${VERBOSE} $* -R -i -f - >>$LOGFILE 2>&1
+	rpmlist $i | ${RESTORECON} ${EXCLUDEDIRS} ${FORCEFLAG} ${VERBOSE} $* -R -i -f -
     done
     exit $?
 fi
 if [ ! -z "$FILEPATH" ]; then
-    ${RESTORECON} ${EXCLUDEDIRS} ${FORCEFLAG} ${VERBOSE} -R $* -- "$FILEPATH" >>$LOGFILE 2>&1
+    ${RESTORECON} ${EXCLUDEDIRS} ${FORCEFLAG} ${VERBOSE} -R $* -- "$FILEPATH"
     return
 fi
 if [  -n "${FILESYSTEMSRW}" ]; then
     LogReadOnly
     echo "${OPTION}ing `echo ${FILESYSTEMSRW}`"
-    ${SETFILES} ${VERBOSE} ${EXCLUDEDIRS} -q ${FORCEFLAG} $* ${FC} ${FILESYSTEMSRW} >>$LOGFILE 2>&1
+    ${SETFILES} ${VERBOSE} ${EXCLUDEDIRS} -q ${FORCEFLAG} $* ${FC} ${FILESYSTEMSRW}
 else
     echo >&2 "fixfiles: No suitable file systems found"
 fi
@@ -272,7 +258,7 @@ exit 0
 }
 
 fullrelabel() {
-    logit "Cleaning out /tmp"
+    echo "Cleaning out /tmp"
     find /tmp/ -mindepth 1 -delete
     LogReadOnly
     restore Relabel
@@ -325,9 +311,9 @@ esac
 }
 usage() {
 	echo $"""
-Usage: $0 [-v] [-F]  [-N time ] [-l logfile ] { check | restore| [-f] relabel | verify } [[dir/file] ... ]
+Usage: $0 [-v] [-F] [-N time ] { check | restore| [-f] relabel | verify } [[dir/file] ... ]
 or
-Usage: $0 [-v] [-F] -R rpmpackage[,rpmpackage...] [-l logfile ] { check | restore | verify }
+Usage: $0 [-v] [-F] -R rpmpackage[,rpmpackage...] { check | restore | verify }
 or
 Usage: $0 [-v] [-F] -C PREVIOUS_FILECONTEXT { check | restore | verify }
 or
@@ -356,7 +342,9 @@ while getopts "N:BC:FfR:l:v" i; do
 		RPMFILES=$OPTARG
 		;;
 	l)
-		LOGFILE=$OPTARG
+		# Old scripts use obsolete option `-l logfile`
+		echo "Redirecting output to $OPTARG"
+		exec >>"$OPTARG" 2>&1
 		;;
 	C)
 		PREFC=$OPTARG
-- 
2.9.3

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

* Re: [PATCH 1/6] Revert "policycoreutils: let output of `fixfiles` be redirected (as normal)"
  2017-05-04 17:01 [PATCH 1/6] Revert "policycoreutils: let output of `fixfiles` be redirected (as normal)" Alan Jenkins
                   ` (4 preceding siblings ...)
  2017-05-04 17:01 ` [PATCH 6/6] policycoreutils: fixfiles: deprecate -l option Alan Jenkins
@ 2017-05-04 17:28 ` Dominick Grift
  2017-05-04 17:32   ` Dominick Grift
  2017-05-05 17:31 ` James Carter
  6 siblings, 1 reply; 9+ messages in thread
From: Dominick Grift @ 2017-05-04 17:28 UTC (permalink / raw)
  To: selinux

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

On Thu, May 04, 2017 at 06:01:17PM +0100, Alan Jenkins wrote:
> This reverts commit ac7899fc3ad6221e195dd13cdf14b346897314ae,
> which is not yet part of an officially tagged release
> (or release candidate).
> 
> `LOGFILE=/proc/self/fd/1` was wrong.
> 
> `LOGFILE=$(tty)` was being relied on in one case (exclude_dirs),
> to log messages from a function run specifically with stdout redirected
> (captured into a variable).
> 
> Having `logit "message"` break inside redirected functions
> is a nasty leaky abstraction.
> 
> This caused e.g. `fixfiles restore` to terminate early with the error
> 
>     skipping: No such file or directory
> 
> if the user had configured any excluded paths in
> /etc/selinux/fixfiles_exclude_dirs
> ---
>  policycoreutils/scripts/fixfiles | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/policycoreutils/scripts/fixfiles b/policycoreutils/scripts/fixfiles
> index bc74d69..75d7762 100755
> --- a/policycoreutils/scripts/fixfiles
> +++ b/policycoreutils/scripts/fixfiles
> @@ -119,7 +119,11 @@ VERBOSE="-p"
>  FORCEFLAG=""
>  DIRS=""
>  RPMILES=""
> -LOGFILE=/proc/self/fd/1
> +LOGFILE=`tty`
> +if [ $? != 0 ]; then
> +    LOGFILE="/dev/null"
> +fi
> +LOGGER=/usr/sbin/logger

$ ls /usr/bin/logger
/usr/bin/logger
$ ls /usr/sbin/logger
ls: cannot access '/usr/sbin/logger': No such file or directory

>  SETFILES=/sbin/setfiles
>  RESTORECON=/sbin/restorecon
>  FILESYSTEMSRW=`get_rw_labeled_mounts`
> @@ -134,11 +138,11 @@ else
>  fi
>  
>  #
> -# Write to LOGFILE
> +# Log to either syslog or a LOGFILE
>  #
>  logit () {
>  if [ -n $LOGFILE ]; then
> -    echo $1 >> "$LOGFILE"
> +    echo $1 >> $LOGFILE
>  fi
>  }
>  #
> -- 
> 2.9.3
> 

-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

* Re: [PATCH 1/6] Revert "policycoreutils: let output of `fixfiles` be redirected (as normal)"
  2017-05-04 17:28 ` [PATCH 1/6] Revert "policycoreutils: let output of `fixfiles` be redirected (as normal)" Dominick Grift
@ 2017-05-04 17:32   ` Dominick Grift
  0 siblings, 0 replies; 9+ messages in thread
From: Dominick Grift @ 2017-05-04 17:32 UTC (permalink / raw)
  To: selinux

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

On Thu, May 04, 2017 at 07:28:19PM +0200, Dominick Grift wrote:
> On Thu, May 04, 2017 at 06:01:17PM +0100, Alan Jenkins wrote:
> > This reverts commit ac7899fc3ad6221e195dd13cdf14b346897314ae,
> > which is not yet part of an officially tagged release
> > (or release candidate).
> > 
> > `LOGFILE=/proc/self/fd/1` was wrong.
> > 
> > `LOGFILE=$(tty)` was being relied on in one case (exclude_dirs),
> > to log messages from a function run specifically with stdout redirected
> > (captured into a variable).
> > 
> > Having `logit "message"` break inside redirected functions
> > is a nasty leaky abstraction.
> > 
> > This caused e.g. `fixfiles restore` to terminate early with the error
> > 
> >     skipping: No such file or directory
> > 
> > if the user had configured any excluded paths in
> > /etc/selinux/fixfiles_exclude_dirs
> > ---
> >  policycoreutils/scripts/fixfiles | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/policycoreutils/scripts/fixfiles b/policycoreutils/scripts/fixfiles
> > index bc74d69..75d7762 100755
> > --- a/policycoreutils/scripts/fixfiles
> > +++ b/policycoreutils/scripts/fixfiles
> > @@ -119,7 +119,11 @@ VERBOSE="-p"
> >  FORCEFLAG=""
> >  DIRS=""
> >  RPMILES=""
> > -LOGFILE=/proc/self/fd/1
> > +LOGFILE=`tty`
> > +if [ $? != 0 ]; then
> > +    LOGFILE="/dev/null"
> > +fi
> > +LOGGER=/usr/sbin/logger
> 
> $ ls /usr/bin/logger
> /usr/bin/logger
> $ ls /usr/sbin/logger
> ls: cannot access '/usr/sbin/logger': No such file or directory

Whoops, please ignore. This was removed anyway

> 
> >  SETFILES=/sbin/setfiles
> >  RESTORECON=/sbin/restorecon
> >  FILESYSTEMSRW=`get_rw_labeled_mounts`
> > @@ -134,11 +138,11 @@ else
> >  fi
> >  
> >  #
> > -# Write to LOGFILE
> > +# Log to either syslog or a LOGFILE
> >  #
> >  logit () {
> >  if [ -n $LOGFILE ]; then
> > -    echo $1 >> "$LOGFILE"
> > +    echo $1 >> $LOGFILE
> >  fi
> >  }
> >  #
> > -- 
> > 2.9.3
> > 
> 
> -- 
> Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
> https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
> Dominick Grift



-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

* Re: [PATCH 1/6] Revert "policycoreutils: let output of `fixfiles` be redirected (as normal)"
  2017-05-04 17:01 [PATCH 1/6] Revert "policycoreutils: let output of `fixfiles` be redirected (as normal)" Alan Jenkins
                   ` (5 preceding siblings ...)
  2017-05-04 17:28 ` [PATCH 1/6] Revert "policycoreutils: let output of `fixfiles` be redirected (as normal)" Dominick Grift
@ 2017-05-05 17:31 ` James Carter
  6 siblings, 0 replies; 9+ messages in thread
From: James Carter @ 2017-05-05 17:31 UTC (permalink / raw)
  To: Alan Jenkins, selinux

On 05/04/2017 01:01 PM, Alan Jenkins wrote:
> This reverts commit ac7899fc3ad6221e195dd13cdf14b346897314ae,
> which is not yet part of an officially tagged release
> (or release candidate).
> 
> `LOGFILE=/proc/self/fd/1` was wrong.
> 
> `LOGFILE=$(tty)` was being relied on in one case (exclude_dirs),
> to log messages from a function run specifically with stdout redirected
> (captured into a variable).
> 
> Having `logit "message"` break inside redirected functions
> is a nasty leaky abstraction.
> 
> This caused e.g. `fixfiles restore` to terminate early with the error
> 
>      skipping: No such file or directory
> 
> if the user had configured any excluded paths in
> /etc/selinux/fixfiles_exclude_dirs

These six patches have been applied.

Thanks,
Jim

> ---
>   policycoreutils/scripts/fixfiles | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/policycoreutils/scripts/fixfiles b/policycoreutils/scripts/fixfiles
> index bc74d69..75d7762 100755
> --- a/policycoreutils/scripts/fixfiles
> +++ b/policycoreutils/scripts/fixfiles
> @@ -119,7 +119,11 @@ VERBOSE="-p"
>   FORCEFLAG=""
>   DIRS=""
>   RPMILES=""
> -LOGFILE=/proc/self/fd/1
> +LOGFILE=`tty`
> +if [ $? != 0 ]; then
> +    LOGFILE="/dev/null"
> +fi
> +LOGGER=/usr/sbin/logger
>   SETFILES=/sbin/setfiles
>   RESTORECON=/sbin/restorecon
>   FILESYSTEMSRW=`get_rw_labeled_mounts`
> @@ -134,11 +138,11 @@ else
>   fi
>   
>   #
> -# Write to LOGFILE
> +# Log to either syslog or a LOGFILE
>   #
>   logit () {
>   if [ -n $LOGFILE ]; then
> -    echo $1 >> "$LOGFILE"
> +    echo $1 >> $LOGFILE
>   fi
>   }
>   #
> 


-- 
James Carter <jwcart2@tycho.nsa.gov>
National Security Agency

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

end of thread, other threads:[~2017-05-05 17:31 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-04 17:01 [PATCH 1/6] Revert "policycoreutils: let output of `fixfiles` be redirected (as normal)" Alan Jenkins
2017-05-04 17:01 ` [PATCH 2/6] policycoreutils: fixfiles: remove (broken) redundant code Alan Jenkins
2017-05-04 17:01 ` [PATCH 3/6] policycoreutils: fixfiles: clarify exclude_dirs() Alan Jenkins
2017-05-04 17:01 ` [PATCH 4/6] policycoreutils: fixfiles: fix logging about R/O filesystems Alan Jenkins
2017-05-04 17:01 ` [PATCH 5/6] policycoreutils: fixfiles: move logit call outside of redirected function Alan Jenkins
2017-05-04 17:01 ` [PATCH 6/6] policycoreutils: fixfiles: deprecate -l option Alan Jenkins
2017-05-04 17:28 ` [PATCH 1/6] Revert "policycoreutils: let output of `fixfiles` be redirected (as normal)" Dominick Grift
2017-05-04 17:32   ` Dominick Grift
2017-05-05 17:31 ` James Carter

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.