Message-ID: <64e77e88-5c67-409f-9543-caea6a5e20d1@themaw.net>
Date: Fri, 14 Nov 2025 08:07:50 +0800
From: Ian Kent <raven@...maw.net>
To: Christian Brauner <brauner@...nel.org>
Cc: Al Viro <viro@...iv.linux.org.uk>,
 Kernel Mailing List <linux-kernel@...r.kernel.org>,
 autofs mailing list <autofs@...r.kernel.org>,
 linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH 2/2] autofs: dont trigger mount if it cant succeed

On 14/11/25 07:49, Ian Kent wrote:
> On 13/11/25 21:19, Christian Brauner wrote:
>> On Thu, Nov 13, 2025 at 08:14:36AM +0800, Ian Kent wrote:
>>> On 12/11/25 19:01, Christian Brauner wrote:
>>>> On Tue, Nov 11, 2025 at 08:27:42PM +0800, Ian Kent wrote:
>>>>> On 11/11/25 18:55, Christian Brauner wrote:
>>>>>> On Tue, Nov 11, 2025 at 10:24:35AM +0000, Al Viro wrote:
>>>>>>> On Tue, Nov 11, 2025 at 11:19:59AM +0100, Christian Brauner wrote:
>>>>>>>
>>>>>>>>> +    sbi->owner = current->nsproxy->mnt_ns;
>>>>>>>> ns_ref_get()
>>>>>>>> Can be called directly on the mount namespace.
>>>>>>> ... and would leak all mounts in the mount tree, unless I'm missing
>>>>>>> something subtle.
>>>>>> Right, I thought you actually wanted to pin it.
>>>>>> Anyway, you could take a passive reference but I think that's 
>>>>>> nonsense
>>>>>> as well. The following should do it:
>>>>> Right, I'll need to think about this for a little while, I did think
>>>>>
>>>>> of using an id for the comparison but I diverged down the wrong 
>>>>> path so
>>>>>
>>>>> this is a very welcome suggestion. There's still the handling of 
>>>>> where
>>>>>
>>>>> the daemon goes away (crash or SIGKILL, yes people deliberately do 
>>>>> this
>>>>>
>>>>> at times, think simulated disaster recovery) which I've missed in 
>>>>> this
>>>> Can you describe the problem in more detail and I'm happy to help you
>>>> out here. I don't yet understand what the issue is.
>>> I thought the patch description was ok but I'll certainly try.
>> I'm sorry, we're talking past each other: I was interested in your
>> SIGKILL problem when the daemon crashes. You seemed to say that you
>> needed additional changes for that case. So I'm trying to understand
>> what the fundamental additional problem is with a crashing daemon that
>> would require additional changes here.
>
> Right, sorry.
>
> It's pretty straight forward.
>
>
> If the daemon is shutdown (or killed summarily) and there are busy
>
> mounts left mounted then when started again they are "re-connected to"
>
> by the newly running daemon. So there's a need to update the mnt_ns_id in
>
> the ioctl that is used to set the new pipefd.
>
>
> I can't provide a patch fragment because I didn't realise the id in 
> ns_common
>
> was added a your recent patch series and I briefly went down a path 
> trying to
>
> compile against 6.16.7 before I realised I hadn't been paying attention.
>
>
> The setting needs to be put in 
> fs/autofs/dev-ioctl.c:autofs_dev_ioctl_setpipefd().

As an fyi, unrelated to what I'm trying to fix here but related

to SIGKILL behaviour. I have a user that uses a custom disaster

recovery procedure that essentially does a SIGKILL and expects

automount(8) to startup and continue using changed servers which

it doesn't. I'm pretty sure I just need to re-queue or discard

any entries in the wait queue but haven't got it quite right yet

(I think I'm forgetting to update the flags). So eventually

there'll be another autofs patch which might look like it belongs

with this one but is actually quite different.


>
> Ian
>
>>
>>>
>>> Consider using automount in a container.
>>>
>>>
>>> For people to use autofs in a container either automount(8) in the init
>>>
>>> mount namespace or an independently running automount(8) entirely 
>>> within
>>>
>>> the container can be used. The later is done by adding a volume option
>>>
>>> (or options) to the container to essentially bind mount the autofs 
>>> mount
>>>
>>> into the container and the option syntax allows the volume to be set
>>>
>>> propagation slave if it is not already set by default (shared is bad,
>>>
>>> the automounts must not propagate back to where they came from). If the
>>>
>>> automount(8) instance is entirely within the container that also works
>>>
>>> fine as everything is isolated within the container (no volume options
>>>
>>> are needed).
>>>
>>>
>>> Now with unshare(1) (and there are other problematic cases, I think 
>>> systemd
>>>
>>> private temp gets caught here too) where using something like 
>>> "unshare -Urm"
>>>
>>> will create a mount namespace that includes any autofs mounts and 
>>> sets them
>>>
>>> propagation private. These mounts cannot be unmounted within the mount
>>>
>>> namepsace by the namespace creator and accessing a directory within the
>> Right, but that should only be true for unprivileged containers where we
>> lock mounts at copy_mnt_ns().
>>
>>> autofs mount will trigger a callback to automount(8) in the init 
>>> namespace
>>>
>>> which mounts the requested mount. But the newly created mount 
>>> namespace is
>>>
>>> propagation private so the process in the new mount namespace loops 
>>> around
>>>
>>> sending mount requests that cannot be satisfied. The odd thing is 
>>> that on
>>> the
>>>
>>> second callback to automount(8) returns an error which does complete 
>>> the
>>>
>>> ->d_automount() call but doesn't seem to result in breaking the loop in
>>>
>>> __traverse_mounts() for some unknown reason. One way to resolve this 
>>> is to
>>>
>>> check if the mount can be satisfied and if not bail out immediately and
>>>
>>> returning an error in this case does work.
>> Yes, that's sensible. And fwiw, I think for private mounts that's the
>> semantics you want. You have disconnected from the "managing" mount
>> namespace - for lack of a better phrase - so you shouldn't get the mount
>> events.
>>
>>> I was tempted to work out how to not include the autofs mounts in 
>>> the cloned
>>>
>>> namespace but that's file system specific code in the VFS which is 
>>> not ok
>>> and
>>>
>>> it (should) also be possible for the namespace creator to "mount
>>> --make-shared"
>>>
>>> in the case the creator wants the mount to function and this would 
>>> prevent
>>> that.
>>>
>>> So I don't think this is the right thing to do.
>>>
>>>
>>> There's also the inability of the mount namespace creator to umount the
>>> autofs
>>>
>>> mount which could also resolve the problem which I haven't looked 
>>> into yet.
>> Ok, again, that should only be an issue with unprivileged mount
>> namespaces, i.e., owned by another user namespace. This isn't easily
>> doable. If the unprivileged mount namespaces can unmount the automount
>> it might reveal hidden/overmounted directories that weren't supposed to
>> be exposed to the container - I hate these semantics btw.
>>
>>>
>>> Have I made sense?
>> Yes, though that's not the question I tried to ask you. :)
>>
>>>
>>> Clearly there's nothing on autofs itself and why one would want to 
>>> use it
>>>
>>> but I don't think that matters for the description.
>>>
>>>
>>> Ian
>>>
>

Powered by blists - more mailing lists