Re: [PATCH] tools/run_privatens: check if it is mount point for --make-private

[Naohiro Aota <Naohiro.Aota@wdc.com>]

On Mon Mar 3, 2025 at 10:29 PM JST, Naohiro Aota wrote:
> On Mon Mar 3, 2025 at 7:10 PM JST, Zorro Lang wrote:
>> On Mon, Mar 03, 2025 at 03:42:09PM +0900, Naohiro Aota wrote:
>>> While /tmp is mounted with tmpfs in the most setup, it still can be a non-mount
>>> point. For example, I'm running the fstests in a container, which does not
>>> mount /tmp inside the container.
>>> 
>>> Running any test case on such system results in having the following error
>>> printed, which leads to all the test cases fail due to the output difference.
>>> 
>>>   mount: /tmp: not mount point or bad option.
>>>          dmesg(1) may have more information after failed mount system call.
>>> 
>>> These lines are printed by the "mount --make-private" command. So, fix that by
>>> using mountpoint command to check if the directory is a mount point or not.
>>> 
>>> Fixes: 247ab01fa227 ("check: run tests in a private pid/mount namespace")
>>> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
>>> ---
>>>  tools/run_privatens | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/tools/run_privatens b/tools/run_privatens
>>> index df94974ab30c..c52e0128b8f9 100755
>>> --- a/tools/run_privatens
>>> +++ b/tools/run_privatens
>>> @@ -8,7 +8,7 @@
>>>  
>>>  if [ -n "${FSTESTS_ISOL}" ]; then
>>>  	for path in /proc /tmp; do
>>> -		mount --make-private "$path"
>>> +		mountpoint "$path" >/dev/null && mount --make-private "$path"
>>
>> Oh, if /tmp isn't a mountpoint on your system, don't you need to think about ...
>>
>>>  	done
>>>  	mount -t proc proc /proc
>>>  	mount -t tmpfs tmpfs /tmp
>>         ^^^^^^^^^^^^^^^^^^^^^^^^^
>>
>> ... this step? I think the first run_privatens process will remain a "mounted"
>> /tmp on your system.
>>
>> And then later tests will use this tmpfs. That's nearly equal to "We always
>> make sure there's a tmpfs mount on /tmp at the beginning of fstests running".
>
> That does not happen because we are already in a mount namespace created
> by nsexec. The mounted "/tmp" is local to each test, and each test
> showed the error above.
>
>>
>> But if we don't run that "mount tmpfs" step, I think it's not what this script
>> wants (to isolate the data in /tmp). Right?
>
> I guess we can do these instead?
>
> mount --make-private -t proc proc /proc
> mount --make-private -t tmpfs tmpfs /tmp
>
> Actually, I'm not quite confindent that we really need "--make-private"
> here. As we mount new instance of proc and tmpfs and we don't bind them,
> I don't see much point making them private.

No, I was wrong. We still need to make them private, not to propagate
"mount tmpfs" or "mount proc" to the outside. If not, we will see new
instance of tmpfs mounted on /tmp in the PID 1 environment, which
breaks several applications :(.

So, in the end, I think my patch did it correctly, no?

>
>>
>> Thanks,
>> Zorro
>>
>>
>>
>>> -- 
>>> 2.48.1
>>> 
>>>