Bug 2125355

This https://github.com/bachradsusi/SELinuxProject-selinux/commit/a4cb900cfb65fa0ed7b3c9928a7a4634be4a70ff could help.

I'm going to test it a bit and propose it upstream.

Wouldn't this happen if anything else tries to clean up in /tmp too?  like tmpcleaning cron-jobs?  Mounting anything of value under /tmp seems like a really bad idea.

(In reply to Peder Stray from comment #2)
> Wouldn't this happen if anything else tries to clean up in /tmp too?  like tmpcleaning cron-jobs?  Mounting anything of value under /tmp seems like a really bad idea.


For what it's worth, I would tend to agree with this. I've always gone by the rule of thumb that you shouldn't put anything in /tmp that you would be sad to not have after a reboot. Granted bind mounting is less risky and just rebooting won't cause bind mounted data to be lost, but since stuff in /tmp is intended to be transient and disposable it's not all that uncommon for programs (including fixfiles) to treat it as such and delete it all.


(In reply to Petr Lautrbach from comment #1)
> This https://github.com/bachradsusi/SELinuxProject-selinux/commit/a4cb900cfb65fa0ed7b3c9928a7a4634be4a70ff could help.
> 
> I'm going to test it a bit and propose it upstream.


Id be curious to hear the results of your tests, but I can envision a few potential problems with this:

1. I dont think that the running `setfiles` command will allow you to just umount the bind mounted filesystem, since it is still running the restore operation on it. I suspect you might be able to solve this by killing the setfiles process first or by using `umount -l`, though these both sound like the type of thing that might open the possibility of filesystem corruption. I dont think repeatedly re-calling the umount command or waiting for it would work either, since that would end up waiting for the relabel command to finish, and chances are if the user was going to re-run the fixfiles command they already would have by then.

2. Even if the above point is actually not a problem, this approach would, I believe, only guard against pressing <ctrl>+<c>. This is probably the most common way that the fixfiles command might get interrupted, but is by no means the only way.


---------------------------------------------------------------------------------------------------------------------------

If I might suggest an alternate (untested) solution - the same loop you modify in your linked commit, change to this instead:

```
TMP_MOUNT_BASEDIR="$(mktemp -p /mnt -d)"     # doesnt have to be at /mnt, but shouldnt be anywhere under /tmp
mount -t tmpfs tmpfs "${TMP_MOUNT_BASEDIR}"

for m in `echo $FILESYSTEMSRW`; do
	TMP_MOUNT="$(mktemp -p "${TMP_MOUNT_BASEDIR}" -d)"
	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."
done;

umount "${TMP_MOUNT_BASEDIR}"
rm -rf "${TMP_MOUNT_BASEDIR}"
```

Basically, use mktemp to get a unique directory somewhere that isnt under /tmp, then create the directory and mount a tmpfs to it, then run the setfiles loop (but have mktemp create temp dirs under your newly mounted tmpfs instead of under /tmp), then when setfiles is done umount the tmpfs and remove the directory.

I think this would work, so long as $TMP_MOUNT_BASEDIR is chosen somewhere that fixfiles has permissions to create a directory and mount a tmpfs to it (but that isnt under /tmp).

There's a thread on mailing upstream list starting with https://lore.kernel.org/selinux/CAJ2a_DeBWkHziE4+DsRqqLULtGkdX68c8jdU3Hxs++84NoPpsQ@mail.gmail.com/T/#m264867de1eb0fe872a1191d74a9b6e3166493d5e with v3 version of the patch which uses '/run' instead of '/tmp' - https://lore.kernel.org/selinux/CAJ2a_DeBWkHziE4+DsRqqLULtGkdX68c8jdU3Hxs++84NoPpsQ@mail.gmail.com/T/#mf5e06bf175ee04b4b8a4c3984be87afdabd45e06

It would be great if the discussion continue only there in order not to duplicate messages and suggestions.

> 
> (In reply to Petr Lautrbach from comment #1)
> > This https://github.com/bachradsusi/SELinuxProject-selinux/commit/a4cb900cfb65fa0ed7b3c9928a7a4634be4a70ff could help.
> > 
> > I'm going to test it a bit and propose it upstream.
> 
> 
> Id be curious to hear the results of your tests, but I can envision a few
> potential problems with this:
> 
> 1. I dont think that the running `setfiles` command will allow you to just
> umount the bind mounted filesystem, since it is still running the restore
> operation on it. I suspect you might be able to solve this by killing the
> setfiles process first or by using `umount -l`, though these both sound like
> the type of thing that might open the possibility of filesystem corruption.
> I dont think repeatedly re-calling the umount command or waiting for it
> would work either, since that would end up waiting for the relabel command
> to finish, and chances are if the user was going to re-run the fixfiles
> command they already would have by then.


When I tried to reproduce this problem and hit Ctr-C there was no setfiles process left, only the mountpoint and therefore umount simply worked.

What shell or kind of terminal do you use?

 
> 2. Even if the above point is actually not a problem, this approach would, I
> believe, only guard against pressing <ctrl>+<c>. This is probably the most
> common way that the fixfiles command might get interrupted, but is by no
> means the only way.
> 

The v3 version mentioned above already trap's on all EXIT signals.

> When I tried to reproduce this problem and hit Ctr-C there was no setfiles
> process left, only the mountpoint and therefore umount simply worked.
> 


[root@P1 ~]# fixfiles -M relabel

    Files in the /tmp directory may be labeled incorrectly, this command
    can remove all files in /tmp.  If you choose to remove files from /tmp,
    a reboot will be required after completion.

    Do you wish to clean out the /tmp directory [N]? 
Relabeling / /boot /dev /dev/hugepages /dev/mqueue /dev/pts /dev/shm /home /run /run/user/1000 /sys /sys/fs/cgroup /sys/fs/pstore /sys/kernel/debug /sys/kernel/tracing /tmp /var
6k^C

[root@P1 ~]# ps ax | grep setfiles
  99107 pts/2    S+     0:00 grep --color=auto setfiles

[root@P1 ~]# mount | grep /tmp/tmp
/dev/mapper/luks-123e-32c7-4d1f-323b-23c7440558c on /tmp/tmp.0sXcAjjdKd type xfs (rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=32k,noquota)

(In reply to Petr Lautrbach from comment #4)
 
> When I tried to reproduce this problem and hit Ctr-C there was no setfiles
> process left, only the mountpoint and therefore umount simply worked.
> 
> What shell or kind of terminal do you use?


Sorry for the delay in replying....life has been extra hectic lately.

I use the Fedora KDE spin and this was run on "konsole" - the standard KDE terminal.

Unfortunately, I can't just go back and check logs (being they got deleted and all), but trying to remember back I *might* have run fixfiles using the parallel relabeling flag (`-T <N>`) on the first run. Perhaps the reason that `<ctrl> + <c>` didn't stop the first fixfiles run was because the -T flag made fixfiles fork off the fixfiles processes that actually touched the filesystem, and these were not stopped when the main fixfiles process was interrupted?

FEDORA-2023-398d7812ee has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2023-398d7812ee

FEDORA-2023-398d7812ee has been pushed to the Fedora 38 stable repository.
If problem still persists, please make note of it in this bug report.

FEDORA-2023-568a3be5b8 has been submitted as an update to Fedora 37. https://bodhi.fedoraproject.org/updates/FEDORA-2023-568a3be5b8

FEDORA-2023-568a3be5b8 has been pushed to the Fedora 37 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf upgrade --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-568a3be5b8`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-568a3be5b8

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

FEDORA-2023-568a3be5b8 has been pushed to the Fedora 37 stable repository.
If problem still persists, please make note of it in this bug report.