Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
MIME-Version: 1.0
From:   Jann Horn <jannh@google.com>
Date:   Fri, 27 Oct 2023 19:12:46 +0200
Message-ID: <CAG48ez0ppjcT=QxU-jtCUfb5xQb3mLr=5FcwddF_VKfEBPs_Dg@mail.gmail.com>
Subject: BPF: bpf_d_path() can be invoked on "struct path" not holding proper
 references, resulting in kernel memory corruption
To:     Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Jiri Olsa <jolsa@kernel.org>
Cc:     Martin KaFai Lau <martin.lau@linux.dev>,
        Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>,
        Stanislav Fomichev <sdf@google.com>,
        Hao Luo <haoluo@google.com>, KP Singh <kpsingh@google.com>,
        Matt Bobrowski <mattbobrowski@google.com>,
        bpf <bpf@vger.kernel.org>,
        kernel list <linux-kernel@vger.kernel.org>,
        Al Viro <viro@zeniv.linux.org.uk>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi!

bpf_d_path() can be invoked on a "struct path" that results from
following a pointer chain involving pointers that can concurrently
change; this can lead to stuff like use-after-free in d_path().

For example, the BPF verifier permits stuff like
bpf_d_path(&current->mm->exe_file->f_path, ...), which is not actually
safe in many contexts:

current->mm->exe_file can concurrently change; so by the time
bpf_d_path() is called, the file's refcount might have gone to zero,
and __fput() may have already mostly torn down the file. "struct file"
currently has some limited RCU lifetime, but that is supposed to be an
implementation detail that BPF shouldn't be relying on, and "struct
file" will soon have even less RCU lifetime than before (see
<https://lore.kernel.org/all/20230930-glitzer-errungenschaft-b86880c177c4@brauner/>).

When __fput() tears down a file, it drops the references held by
file->f_path.mnt and file->f_path.dentry. "struct vfsmount" has some
kind of RCU lifetime, but "struct dentry" will be freed directly in
dentry_free() if it has DCACHE_NORCU set, which is the case if it was
allocated via d_alloc_pseudo(), which is how memfd files are
allocated.

So the following race is possible, if we start in a situation where
current->mm->exe_file points to a memfd:

thread A            thread B
========            ========
begin RCU section
begin BPF program
compute path = &current->mm->exe_file->f_path

                    prctl(PR_SET_MM, PR_SET_MM_MAP, ...)
                      updates current->mm->exe_file
                      calls fput() on old ->exe_file
                    __fput() runs
                      dput(dentry);
                      mntput(mnt)

invoke helper bpf_d_path(path, ...)
  d_path()
    reads path->dentry->d_op  *** UAF read ***
    reads path->dentry->d_op->d_dname  *** read through wild pointer ***
    path->dentry->d_op->d_dname(...) *** wild pointer call ***

So if an attacker managed to reallocate the old "struct dentry" with
attacker-controlled data, they could probably get the kernel to call
an attacker-provided function pointer, eventually letting an attacker
gain kernel privileges.

Obviously this is not a bug an unprivileged attacker can just hit
directly on a system where no legitimate BPF programs are already
running, because loading tracing BPF programs requires privileges; but
if a privileged process loads a tracing BPF program that does
something unsafe like "bpf_d_path(&current->mm->exe_file->f_path,
...)", an attacker might be able to leverage that.


If BPF wants to be able to promise that buggy BPF code can't crash the
kernel (or, worse, introduce privilege escalation vulnerabilities in
the kernel), then I think BPF programs must not be allowed to follow
any pointer chain and pass the object at the end of it into BPF
helpers.