Re: Missing check for CAP_SYS_ADMIN in do_reconfigure_mnt()

[tianyu zhou <tyjoe.linux@gmail.com>]

Thanks for reminding!

But here is one more question about the CAP_SYS_ADMIN check inside
may_mount(): why does it check the CAP in
current->nsproxy->mnt_ns->user_ns?

for do_remount(), it checks CAP_SYS_ADMIN in path->mnt->mnt_sb->s_user_ns;
for path_mount(), it checks CAP_SYS_ADMIN in current->nsproxy->mnt_ns->user_ns.

Is these two user ns are same during runtime? (If they are same, then
it will be redundant check in path_mount() and its callee
do_remount(); if they are not same, maybe do_reconfigure_mnt() need
more check for path->mnt->mnt_sb->s_user_ns)

Tianyu

Matthew Wilcox <willy@infradead.org> 于2021年6月1日周二 上午1:07写道：
>
> On Mon, May 31, 2021 at 10:59:54PM +0800, tianyu zhou wrote:
> > Hi, function do_remount() in fs/namespace.c checks the CAP_SYS_ADMIN
> > before it calls set_mount_attributes().
> >
> > However, in another caller of set_mount_attributes(),
> > do_reconfigure_mnt(), I have not found any check for CAP_SYS_ADMIN.
> > So, is there a missing check bug inside do_reconfigure_mnt() ? (which
> > makes it possible for normal user to reach set_mount_attributes())
>
> You weren't looking hard enough ...
>
> path_mount()
>         if (!may_mount())
>                 return -EPERM;
> ...
>         if ((flags & (MS_REMOUNT | MS_BIND)) == (MS_REMOUNT | MS_BIND))
>                 return do_reconfigure_mnt(path, mnt_flags);
>
> (this is the only call to do_reconfigure_mnt())
>
> and:
>
> static inline bool may_mount(void)
> {
>         return ns_capable(current->nsproxy->mnt_ns->user_ns, CAP_SYS_ADMIN);
> }
>