[KernelAddressSanitizer] Fix globals exclusion for indirect aliases

[melver]

Summary

GlobalAlias::getAliasee() may not always point directly to a
GlobalVariable. In such cases, try to find the canonical GlobalVariable
that the alias refers to.

Link: https://github.com/ClangBuiltLinux/linux/issues/1208

[melver]

Fix recursive case

[dvyukov]

Have you checked if there is already a function that does this? Frequently there is :)
Some functions that look similar based on names:
stripPointerCasts
stripPointerCastsAndOffsets
stripPointerCastsAndAliases
canonicalizeAlias

[melver]

Add test

[melver]

Simplify using stripPointerCastsAndAliases()

[melver]

In D92846#2440118, @dvyukov wrote:

Have you checked if there is already a function that does this? Frequently there is :)
Some functions that look similar based on names:
stripPointerCasts
stripPointerCastsAndOffsets
stripPointerCastsAndAliases
canonicalizeAlias

Yup, stripPointerCastsAndAliases() works.

[melver]

Style

[melver]

Revert unnecessary reformat

[dvyukov]

The code looks reasonable to me. I see it only affects kernel, so assuming you booted kernel, we should be fine.
I can rubber-stamp it, but if you want more meaningful review, please wait for Alex or Nick.

[melver]

In D92846#2442447, @dvyukov wrote:

The code looks reasonable to me. I see it only affects kernel, so assuming you booted kernel, we should be fine.
I can rubber-stamp it, but if you want more meaningful review, please wait for Alex or Nick.

Added Nick.

I suppose all I'd like a 2nd confirmation on is if stripPointerCastsAndAliases() is the right thing to use here.

Nick or Alex, if you could have a brief look would be great -- thanks!

Thanks,

• Marco

[nickdesaulniers]

Thanks for the patch.

clang/test/CodeGen/asan-globals-alias.cpp

30

Do we want to add checks for the aliases: global_alias_2, __global_alias_2_alias, and __mod_joydev_ids_device_table?

[melver]

clang/test/CodeGen/asan-globals-alias.cpp

30

I found that depending on optimization level, what the aliases end up aliasing varies (e.g. for the alias-of-alias at -O2 it turns it into an alias to the global_alias_2). Not sure how to best capture this in the test without it becoming flaky. I suppose we could check that the alias somehow references either what was specified directly or the final global, but I don't think that's the job of this test since it's not directly related to ASan or KASAN.

Or did you mean to just check they exist?

[nickdesaulniers]

clang/test/CodeGen/asan-globals-alias.cpp

30

Existence check. If it doesn't add anything, feel free to skip the suggestion.

[melver]

Check aliases exist in test

clang/test/CodeGen/asan-globals-alias.cpp

 // RUN: %clang_cc1 -triple x86_64-linux -fsanitize=address -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,ASAN
+// RUN: %clang_cc1 -triple x86_64-linux -O2 -fsanitize=address -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,ASAN
 // RUN: %clang_cc1 -triple x86_64-linux -fsanitize=kernel-address -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,KASAN
+// RUN: %clang_cc1 -triple x86_64-linux -O2 -fsanitize=kernel-address -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,KASAN
 //
 // Not all platforms support aliases - test for Linux only.
 
-int global;                                                         // to generate ctor for at least 1 global
-int aliased_global;                                                 // KASAN - ignore globals prefixed by aliases with __-prefix (below)
-extern int __attribute__((alias("aliased_global"))) __global_alias; // KASAN - aliased_global ignored
+int global; // generate ctor for at least 1 global
+int aliased_global; // KASAN ignored
+extern int __attribute__((alias("aliased_global"))) __global_alias;
+
+// Recursive alias:
+int aliased_global_2; // KASAN ignored
+extern int __attribute__((alias("aliased_global_2"))) global_alias_2;
+extern int __attribute__((alias("global_alias_2"))) __global_alias_2_alias;
+
+// Potential indirect alias:
+struct input_device_id {
+  unsigned long keybit[24];
+  unsigned long driver_info;
+};
+struct input_device_id joydev_ids[] = { { {1}, 1234 } }; // KASAN ignored
+extern struct input_device_id __attribute__((alias("joydev_ids"))) __mod_joydev_ids_device_table;
 
 // ASAN: @aliased_global{{.*}} global { i32, [60 x i8] }{{.*}}, align 32
+// ASAN: @aliased_global_2{{.*}} global { i32, [60 x i8] }{{.*}}, align 32
+// ASAN: @joydev_ids{{.*}} global { {{.*}}[56 x i8] zeroinitializer }, align 32
 // KASAN: @aliased_global{{.*}} global i32
+// KASAN: @aliased_global_2{{.*}} global i32
+// KASAN: @joydev_ids{{.*}} global [1 x {{.*}}i64 1234 }], align 16
 
 // CHECK-LABEL: define internal void @asan.module_ctor
-// ASAN: call void @__asan_register_globals({{.*}}, i{{32|64}} 2)
+// ASAN: call void @__asan_register_globals({{.*}}, i{{32|64}} 4)
 // KASAN: call void @__asan_register_globals({{.*}}, i{{32|64}} 1)
 // CHECK-NEXT: ret void
 
 // CHECK-LABEL: define internal void @asan.module_dtor
 // CHECK-NEXT: call void @__asan_unregister_globals
 // CHECK-NEXT: ret void

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

InstrumentGlobalsWithMetadataArray(IRBuilder<> &IRB, Module &M,
                                      ArrayRef<Constant *> MetadataInitializers);
 
   GlobalVariable *CreateMetadataGlobal(Module &M, Constant *Initializer,
                                        StringRef OriginalName);
   void SetComdatForGlobalMetadata(GlobalVariable *G, GlobalVariable *Metadata,
                                   StringRef InternalSuffix);
   Instruction *CreateAsanModuleDtor(Module &M);
 
-  bool canInstrumentAliasedGlobal(const GlobalAlias &GA) const;
+  const GlobalVariable *getExcludedAliasedGlobal(const GlobalAlias &GA,
+                                                 bool Rec = false) const;
   bool shouldInstrumentGlobal(GlobalVariable *G) const;
   bool ShouldUseMachOGlobalsSection() const;
   StringRef getGlobalMetadataSection() const;
   void poisonOneInitializer(Function &GlobalInit, GlobalValue *ModuleName);
   void createInitializerPoisonCalls(Module &M, GlobalValue *ModuleName);
   uint64_t getMinRedzoneSizeForGlobal() const {
     return getRedzoneSizeForScale(Mapping.Scale);
   }
if (Function *F = dyn_cast<Function>(CS->getOperand(1))) {
       // Don't instrument CTORs that will run before asan.module_ctor.
       if (Priority->getLimitedValue() <= GetCtorAndDtorPriority(TargetTriple))
         continue;
       poisonOneInitializer(*F, ModuleName);
     }
   }
 }
 
-bool ModuleAddressSanitizer::canInstrumentAliasedGlobal(
-    const GlobalAlias &GA) const {
-  // In case this function should be expanded to include rules that do not just
-  // apply when CompileKernel is true, either guard all existing rules with an
-  // 'if (CompileKernel) { ... }' or be absolutely sure that all these rules
-  // should also apply to user space.
-  assert(CompileKernel && "Only expecting to be called when compiling kernel");
+const GlobalVariable *
+ModuleAddressSanitizer::getExcludedAliasedGlobal(const GlobalAlias &GA,
+                                                 bool Rec) const {
+  if (!Rec) { // Non-recursive case.
+    // In case this function should be expanded to include rules that do not
+    // just apply when CompileKernel is true, either guard all existing rules
+    // with an 'if (CompileKernel) { ... }' or be absolutely sure that all these
+    // rules should also apply to user space.
+    assert(CompileKernel &&
+           "Only expecting to be called when compiling kernel");
 
     // When compiling the kernel, globals that are aliased by symbols prefixed
     // by "__" are special and cannot be padded with a redzone.
-  if (GA.getName().startswith("__"))
-    return false;
+    if (!GA.getName().startswith("__"))
+      return nullptr;
+  }
 
-  return true;
+  if (const auto *GV = dyn_cast<GlobalVariable>(GA.getAliasee())) {
+    // Find GlobalVariable from aliasee.
+    return GV;
+  } else if (const auto *CE = dyn_cast<ConstantExpr>(GA.getAliasee())) {
+    // Pointer expression into GlobalVariable; find it from one of the operands.
+    for (const Use &U : CE->operands()) {
+      if (const auto *GV = dyn_cast<GlobalVariable>(U))
+        return GV;
+    }
+  } else if (const auto *GAA = dyn_cast<GlobalAlias>(GA.getAliasee())) {
+    // Recursive GlobalAlias
+    return getExcludedAliasedGlobal(*GAA, true);
+  }
+
+  // Not a GlobalVariable alias, ignore.
+  return nullptr;
 }
 
 bool ModuleAddressSanitizer::shouldInstrumentGlobal(GlobalVariable *G) const {
   Type *Ty = G->getValueType();
   LLVM_DEBUG(dbgs() << "GLOBAL: " << *G << "\n");
 
   // FIXME: Metadata should be attched directly to the global directly instead
   // of being added to llvm.asan.globals.

 // redzones and inserts this function into llvm.global_ctors.
 // Sets *CtorComdat to true if the global registration code emitted into the
 // asan constructor is comdat-compatible.
 bool ModuleAddressSanitizer::InstrumentGlobals(IRBuilder<> &IRB, Module &M,
                                                bool *CtorComdat) {
   *CtorComdat = false;
 
   // Build set of globals that are aliased by some GA, where
-  // canInstrumentAliasedGlobal(GA) returns false.
+  // getExcludedAliasedGlobal(GA) returns the relevant GlobalVariable.
   SmallPtrSet<const GlobalVariable *, 16> AliasedGlobalExclusions;
   if (CompileKernel) {
     for (auto &GA : M.aliases()) {
-      if (const auto *GV = dyn_cast<GlobalVariable>(GA.getAliasee())) {
-        if (!canInstrumentAliasedGlobal(GA))
-          AliasedGlobalExclusions.insert(GV);
-      }
-    }
-  }
+      if (const GlobalVariable *GV = getExcludedAliasedGlobal(GA))
 
   SmallVector<GlobalVariable *, 16> GlobalsToChange;
   for (auto &G : M.globals()) {
     if (!AliasedGlobalExclusions.count(&G) && shouldInstrumentGlobal(&G))
       GlobalsToChange.push_back(&G);
   }
 
   size_t n = GlobalsToChange.size();