On Mon, 19 Sept 2022 at 10:39, Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote:
>
> (Resending without HTML...)
>
> On Fri, Sep 16, 2022 at 5:37 PM Christian Göttsche
> <cgzones@xxxxxxxxxxxxxx> wrote:
> >
> > On Fri, 16 Sept 2022 at 16:14, Petr Lautrbach <plautrba@xxxxxxxxxx> wrote:
> > >
> > > `fixfiles -M relabel` temporary bind mounts filestems before relabeling
> > > but it leaves a directory mounted in /tmp/tmp.XXXX when a user hits
> > > CTRL-C. It means that if the user run `fixfiles -M relabel` again and
> > > answered Y to clean out /tmp directory, it would remove all data from
> > > mounted fs.
> > >
> > > This patch changes the location where `fixfiles` mounts fs to /run and
> > > adds a handler for exit signals which tries to umount fs mounted by
> > > `fixfiles`.
> >
> > What about additionally using mount namespaces, if available:
>
> What benefit would it bring? (Sorry, I'm not very familiar with mount
> namespaces.)
The bind mounts will only be visible to the process and its children
used in the unshare command (and processes explicitly joining that
mount namespace) and not to other (parallel or subsequent) instances
of fixfiles.
>
> >
> > @@ -256,10 +256,16 @@
> > test -z ${TMP_MOUNT+x} && echo "Unable to find
> > temporary directory!" && exit 1
> >
> > mkdir -p "${TMP_MOUNT}${m}" || exit 1
> > - mount --bind "${m}" "${TMP_MOUNT}${m}" || exit 1
> > - ${SETFILES} ${VERBOSE} ${EXCLUDEDIRS} ${FORCEFLAG}
> > ${THREADS} $* -q ${FC} -r "${TMP_MOUNT}" "${TMP_MOUNT}${m}"
> > - umount "${TMP_MOUNT}${m}" || exit 1
> > - rm -rf "${TMP_MOUNT}" || echo "Error cleaning up."
> > + if ! unshare --mount /bin/sh -c "mount --bind
> > \"${m}\" \"${TMP_MOUNT}${m}\" || exit 1 && ${SETFILES} ${VERBOSE}
> > ${EXCLUDEDIRS} ${FORCEFLAG} ${THREADS} $* -q ${FC} -r \"${TMP_MOUNT}\"
> > \"${TMP_MOUNT}${m}\" || true"; then
> > + echo "Creating new mount namespace failed,
> > operating in root namespace"
> > + mount --bind "${m}" "${TMP_MOUNT}${m}" || exit 1
> > + ${SETFILES} ${VERBOSE} ${EXCLUDEDIRS}
> > ${FORCEFLAG} ${THREADS} $* -q ${FC} -r "${TMP_MOUNT}"
> > "${TMP_MOUNT}${m}"
> > + umount "${TMP_MOUNT}${m}" || exit 1
> > + fi
> > + if [ "${m}" != '/' ]; then
> > + rmdir "${TMP_MOUNT}${m}" || echo "Error
> > cleaning up ${TMP_MOUNT}${m}."
> > + fi
> > + rmdir "${TMP_MOUNT}" || echo "Error cleaning
> > up ${TMP_MOUNT}."
> > done;
> > fi
> > else
> >
> > ?
> >
> > >
> > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2125355
> > >
> > > Signed-off-by: Petr Lautrbach <plautrba@xxxxxxxxxx>
> > > ---
> > > v2:
> > >
> > > - set trap on EXIT instead of SIGINT
> > >
> > > v3:
> > >
> > > - use /run instead of /tmp for mountpoints
> > >
> > >
> > > policycoreutils/scripts/fixfiles | 13 ++++++++++++-
> > > 1 file changed, 12 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/policycoreutils/scripts/fixfiles b/policycoreutils/scripts/fixfiles
> > > index c72ca0eb9d61..acf5f0996c19 100755
> > > --- a/policycoreutils/scripts/fixfiles
> > > +++ b/policycoreutils/scripts/fixfiles
> > > @@ -207,6 +207,15 @@ rpm -q --qf '[%{FILESTATES} %{FILENAMES}\n]' "$1" | grep '^0 ' | cut -f2- -d ' '
> > > [ ${PIPESTATUS[0]} != 0 ] && echo "$1 not found" >/dev/stderr
> > > }
> > >
> > > +# unmount tmp bind mount before exit
> > > +umount_TMP_MOUNT() {
> > > + if [ -n "$TMP_MOUNT" ]; then
> > > + umount "${TMP_MOUNT}${m}" || exit 130
> > > + rm -rf "${TMP_MOUNT}" || echo "Error cleaning up."
> > > + fi
> > > + exit 130
> > > +}
> > > +
> > > #
> > > # restore
> > > # if called with -n will only check file context
> > > @@ -251,8 +260,9 @@ case "$RESTORE_MODE" in
> > > else
> > > # we bind mount so we can fix the labels of files that have already been
> > > # mounted over
> > > + trap umount_TMP_MOUNT EXIT
> > > for m in `echo $FILESYSTEMSRW`; do
> > > - TMP_MOUNT="$(mktemp -d)"
> > > + TMP_MOUNT="$(mktemp -p /run -d fixfiles.XXXXXXXXXX)"
> > > test -z ${TMP_MOUNT+x} && echo "Unable to find temporary directory!" && exit 1
> > >
> > > mkdir -p "${TMP_MOUNT}${m}" || exit 1
> > > @@ -261,6 +271,7 @@ case "$RESTORE_MODE" in
> > > umount "${TMP_MOUNT}${m}" || exit 1
> > > rm -rf "${TMP_MOUNT}" || echo "Error cleaning up."
> > > done;
> > > + trap EXIT
> > > fi
> > > else
> > > echo >&2 "fixfiles: No suitable file systems found"
> > > --
> > > 2.37.3
> > >
> >
>
> --
> Ondrej Mosnacek
> Senior Software Engineer, Linux Security - SELinux kernel
> Red Hat, Inc.
>
