From 49a64d87364b3fc9be1829250081fc41e36ce385 Mon Sep 17 00:00:00 2001
From: Peter Wu <pwu@cloudflare.com>
Date: Fri, 29 Aug 2025 14:45:00 +0200
Subject: [PATCH] clang: Handle empty kheaders directory gracefully, fix race
 condition

Do not assume an existing `/tmp/kheaders-$(uname -r)` directory to
contain headers. By default, systemd removes files older than 10 days
via tmpfiles config. If a header file is missing, proceed by removing
the old directory before attempting a new extraction.

This also fixes a vulnerability where bcc could be convinced to use a
kheaders directory of an unprivileged user:

 1. Attacker creates a kheaders dir symlink to a non-existing file. This
    bypasses file_exists and non-root user checks in get_proc_kheaders.
 2. extract_kheaders proceeds to extract a headers tarball to a
    temporary directory. Within this race window, an attacker can remove
    the symlink, and replace it by a directory under its control.
 3. extract_kheaders returns success even though the directory is under
    control of an attacker:

        // fails because the attacker created the directory.
        ret = rename(dirpath_tmp, dirpath.c_str());
        if (ret)
          // OOPS: return code is overwritten when tmp dir is removed
          ret = system(("rm -rf " + std::string(dirpath_tmp)).c_str());

Fix by ignoring temporary directory removal failures, and by checking
whether a parallel process created a legitimate privileged directory
with the expected header files.

All privileged directory checks do not follow symlinks. Remove the
owner check for PROC_KHEADERS_PATH as we assume /sys to be trusted.

Fixes #4213

Fixes: 008ea09e8911 ("clang: check header ownership (#4928)")
Signed-off-by: Peter Wu <pwu@cloudflare.com>
---
 src/cc/frontends/clang/kbuild_helper.cc | 60 ++++++++++++++++---------
 1 file changed, 40 insertions(+), 20 deletions(-)

diff --git a/src/cc/frontends/clang/kbuild_helper.cc b/src/cc/frontends/clang/kbuild_helper.cc
index 4f09a398e5f3..c32d7b18370b 100644
--- a/src/cc/frontends/clang/kbuild_helper.cc
+++ b/src/cc/frontends/clang/kbuild_helper.cc
@@ -153,22 +153,31 @@ static inline bool file_exists(const char *f)
   return (stat(f, &buffer) == 0);
 }
 
-static inline bool file_exists_and_ownedby(const char *f, uid_t uid)
+static inline int check_privileged_dir(const char *f)
 {
-  struct stat buffer;
-  int ret = stat(f, &buffer) == 0;
-  if (ret) {
-    if (buffer.st_uid != uid) {
-      std::cout << "ERROR: header file ownership unexpected: " << std::string(f) << "\n";
-      return false;
-    }
+  struct stat buf;
+  if (lstat(f, &buf) != 0)
+    return -errno;
+
+  if (!S_ISDIR(buf.st_mode)) {
+    std::cout << "ERROR: not a directory: " << std::string(f) << "\n";
+    return -ENOTDIR;
   }
-  return ret;
+  if (buf.st_uid != 0) {
+    std::cout << "ERROR: not owned by root: " << std::string(f) << "\n";
+    return -EACCES;
+  }
+  return 0;
 }
 
 static inline bool proc_kheaders_exists(void)
 {
-  return file_exists_and_ownedby(PROC_KHEADERS_PATH, 0);
+  return file_exists(PROC_KHEADERS_PATH);
+}
+
+static inline bool dir_has_kheaders(const std::string &dirpath)
+{
+  return file_exists((dirpath + "/include/linux/kconfig.h").c_str());
 }
 
 static inline const char *get_tmp_dir() {
@@ -216,11 +225,16 @@ static inline int extract_kheaders(const std::string &dirpath,
 
   /*
    * If the new directory exists, it could have raced with a parallel
-   * extraction, in this case just delete the old directory and ignore.
+   * extraction, in this case just delete the old directory and reuse the
+   * existing directory if valid.
    */
   ret = rename(dirpath_tmp, dirpath.c_str());
-  if (ret)
-    ret = system(("rm -rf " + std::string(dirpath_tmp)).c_str());
+  if (ret) {
+    system(("rm -rf " + std::string(dirpath_tmp)).c_str());
+
+    if (check_privileged_dir(dirpath.c_str()) == 0 && dir_has_kheaders(dirpath))
+      ret = 0;
+  }
 
 cleanup:
   if (module) {
@@ -244,14 +258,20 @@ int get_proc_kheaders(std::string &dirpath)
            uname_data.release);
   dirpath = std::string(dirpath_tmp);
 
-  if (file_exists(dirpath_tmp)) {
-    if (file_exists_and_ownedby(dirpath_tmp, 0))
+  int ret = check_privileged_dir(dirpath_tmp);
+  if (ret == 0) {
+    // Previously extracted directory still exists, reuse it.
+    if (dir_has_kheaders(dirpath_tmp))
       return 0;
-    else
-      // The path exists, but is owned by a non-root user
-      // Something fishy is going on
-      return -EEXIST;
-  }
+
+    // Existing directories without headers could be caused by tmpfiles
+    // cleanups. Just remove all empty directories, and retry.
+    ret = system(("rm -rf " + dirpath).c_str());
+    if (ret)
+      return ret;
+  } else if (ret != -ENOENT)
+    // Missing directories are fine, wrong ownership or I/O errors are not.
+    return ret;
 
   // First time so extract it
   return extract_kheaders(dirpath, uname_data);