On 6/11/25 00:08, Song Liu wrote:
> On Tue, Jun 10, 2025 at 3:26 PM Tingmao Wang <m@xxxxxxxxxx> wrote:
> [..]
>>>
>>> if (!choose_mountpoint(real_mount(path->mnt), root, &p))
>>> return false;
>>> path_put(path);
>>> *path = p;
>>> ret = true;
>>> }
>>>
>>> if (unlikely(IS_ROOT(path->dentry)))
>>> return ret;
>>
>> Returning true here would be the wrong semantic right? This whole thing
>> is only possible when some mount shadows "/". Say if you have a landlock
>> rule on the old "/", but then we mount a new "/" and chroot into it (via
>> "/.."), the landlock rule on the old "/" should not apply, but if we
>> change *path and return true here then this will "expose" that old "/" to
>> landlock.
>
> Could you please provide more specific information about this case?
Apologies, it looks like I was mistaken in the above statement.
I was thinking of something like
# mount --mkdir -t tmpfs none tmproot
# cp busybox tmproot/ && chmod +x tmproot/busybox
# mount --move tmproot /
# env LL_FS_RW=/ LL_FS_RO=/.. ./sandboxer chroot /.. /busybox sh
# echo can write to root > /a
sh: can't create /a: Permission denied
^^^^ this does not work, but I was mistakenly thinking it would
I think because choose_mountpoint_rcu only returns true if
if (mountpoint != m->mnt.mnt_root)
passes, this situation won't cause ret to be true in your code.
But then I can't think of when
if (unlikely(IS_ROOT(path->dentry)))
return ret;
would ever return true, unless somehow d_parent is corrupted? Maybe I'm
just missing something obvious here.
Anyway, since there's a suggestion from Neil to refactor this, this might
not be too important, so feel free to ignore for now.
>
> Thanks,
> Song
>
>> A quick suggestion although I haven't tested anything - maybe we should do
>> a special case check for IS_ROOT inside the
>> if (unlikely(path->dentry == path->mnt->mnt_root))
>> ? Before "path_put(path);", if IS_ROOT(p.dentry) then we just path_get(p)
>> and return false.
>>
>>>
>>> parent = dget_parent(path->dentry);
>>> dput(path->dentry);
>>> path->dentry = parent;
>>> return true;
>>> }
>>>
>>> Thanks,
>>> Song
>>
