Re: Missing check for CAP_SYS_ADMIN in do_reconfigure_mnt()

[Al Viro <viro@zeniv.linux.org.uk>]

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().
> 
> --------------------
> // fs/namespace.c
> static int do_remount(struct path *path, int ms_flags, int sb_flags,
>               int mnt_flags, void *data)
> {
>         ....
>         if (ns_capable(sb->s_user_ns, CAP_SYS_ADMIN)) {
>             err = reconfigure_super(fc);
>             if (!err) {
>                 lock_mount_hash();
>                 set_mount_attributes(mnt, mnt_flags);       // <===
> protected function
>                 unlock_mount_hash();
>             }
>         ...
> }
> --------------------
> 
> 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())

IDGI.  By the same token, there are callers of e.g. memcpy() with
CAP_SYS_ADMIN checks upstream of those, as well as those that are
called without any such checks whatsoever.  The answer to such
observation would obviously be "what of that?" and I really wonder
what your criteria are.

For another example, in the same function you have lock_mount_hash()
calls as well; are you going to report the calls of that made without
CAP_SYS_ADMIN?

IOW, what are the heuristics you are using to select the functions
you deem suspicious?