linux-kernel - A missing check bug in cgroup1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAA-qYXjxht4+GhTjNb0xmr4dLQYDVpDbO1R_FDcWtnsrQC=VNQ@mail.gmail.com>
Date:   Thu, 16 Sep 2021 15:33:49 +0800
From:   Jinmeng Zhou <jjjinmeng.zhou@...il.com>
To:     tj@...nel.org, lizefan@...wei.com, hannes@...xchg.org
Cc:     shenwenbosmile@...il.com, cgroups@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: A missing check bug in cgroup1_reconfigure()

Dear maintainers,
hi, our team has found a missing check bug on Linux kernel v5.10.7
using static analysis.
There is a checking path where cgroup1_get_tree() calls cgroup1_root_to_use()
to mount cgroup_root after checking capability.
However, another no-checking path exists, cgroup1_reconfigure() calls
trace_cgroup_remount()
to remount without checking capability.
We think there is a missing check bug before mounting cgroup_root in
cgroup1_reconfigure().

Specifically, cgroup1_get_tree() uses ns_capable(ctx->ns->user_ns,
CAP_SYS_ADMIN) to check
the permission before calling the critical function
cgroup1_root_to_use() to mount.

1. // check ns_capable() ////////////////////////////
2. int cgroup1_get_tree(struct fs_context *fc)
3. {
4.  struct cgroup_fs_context *ctx = cgroup_fc2context(fc);
5.  int ret;
6.  /* Check if the caller has permission to mount. */
7.  if (!ns_capable(ctx->ns->user_ns, CAP_SYS_ADMIN))
8.    return -EPERM;
9.  cgroup_lock_and_drain_offline(&cgrp_dfl_root.cgrp);
10. ret = cgroup1_root_to_use(fc);
11. ...
12. }

trace_cgroup_remount() is called to remount cgroup_root in
cgroup1_reconfigure().
However, it lacks the check.
1. int cgroup1_reconfigure(struct fs_context *fc)
2. {
3.  struct cgroup_fs_context *ctx = cgroup_fc2context(fc);
4.  struct kernfs_root *kf_root = kernfs_root_from_sb(fc->root->d_sb);
5.  struct cgroup_root *root = cgroup_root_from_kf(kf_root);
6.  int ret = 0;
7.  u16 added_mask, removed_mask;
8.  ...
9.  trace_cgroup_remount(root);
10. ...
11. }

We find cgroup1_reconfigure() is only used in a variable initialization.
Function cgroup1_get_tree() is also used in this initialization.
Both functions are indirectly called which is hard to trace.
We reasonably consider that the two functions can be equally reached
by the user,
therefore, there is a missing check bug.
1. static const struct fs_context_operations cgroup1_fs_context_ops = {
2. …
3.  .get_tree = cgroup1_get_tree,
4.  .reconfigure = cgroup1_reconfigure,
5. };

Thanks!

Best regards,
Jinmeng Zhou