Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sanitizer handler calls emitted without regard to -mregparm #89670

Closed
kees opened this issue Apr 22, 2024 · 2 comments · Fixed by #89707
Closed

Sanitizer handler calls emitted without regard to -mregparm #89670

kees opened this issue Apr 22, 2024 · 2 comments · Fixed by #89707

Comments

@kees
Copy link
Contributor

kees commented Apr 22, 2024

When sanitizer calls are emitted, the -mregparm=3 option used by the Linux kernel appears to be ignored. For example, here is a build where the argument are being pushed instead of placed in %eax and %edx (from lkdtm_ARRAY_BOUNDS):

   0xc18e3a5a <+202>:   push   %ebx
   0xc18e3a5b <+203>:   push   $0xc26001a0
   0xc18e3a60 <+208>:   call   0xc157d430 <__ubsan_handle_out_of_bounds>

The kernel's handler isn't expecting them on the stack. For example, this is setting a bit in the sanitizer's passed-in data structure (from __ubsan_handle_out_of_bounds):

   0xc157d491 <+97>:    btsl   $0x1f,%ds:0x4(%eax)
   0xc157d497 <+103>:   jae    0xc157d4a1 <__ubsan_handle_out_of_bounds+113>

KSPP/linux#350

@kees
Copy link
Contributor Author

kees commented Apr 22, 2024

From clang/lib/CodeGen/CGCall.cpp:

llvm::CallBase *
CodeGenFunction::EmitRuntimeCallOrInvoke(llvm::FunctionCallee callee,
                                         ArrayRef<llvm::Value *> args,
                                         const Twine &name) {
  llvm::CallBase *call = EmitCallOrInvoke(callee, args, name);
  call->setCallingConv(getRuntimeCC());
  return call;
}

the calling convention doesn't seem to have any knowledge of where -mregparm gets recorded in clang/lib/CodeGen/CodeGenModule.cpp from CodeGenModule::Release():

  // Record mregparm value now so it is visible through rest of codegen.
  if (Context.getTargetInfo().getTriple().getArch() == llvm::Triple::x86)
    getModule().addModuleFlag(llvm::Module::Error, "NumRegisterParameters",
                              CodeGenOpts.NumRegisterParameters);

kees added a commit that referenced this issue Apr 29, 2024
…89707)

When building the Linux kernel for i386, the -mregparm=3 option is
enabled. Crashes were observed in the sanitizer handler functions, and
the problem was found to be mismatched calling convention.

As was fixed in commit c167c0a ("[BuildLibCalls] infer inreg param
attrs from NumRegisterParameters"), call arguments need to be marked as
"in register" when -mregparm is set. Use the same helper developed there
to update the function arguments.

Since CreateRuntimeFunction() is actually part of CodeGenModule, storage
of the -mregparm value is also moved to the constructor, as doing this
in Release() is too late.

Fixes: #89670
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 29, 2024

@llvm/issue-subscribers-clang-codegen

Author: Kees Cook (kees)

When sanitizer calls are emitted, the `-mregparm=3` option used by the Linux kernel appears to be ignored. For example, here is a build where the argument are being pushed instead of placed in `%eax` and `%edx` (from `lkdtm_ARRAY_BOUNDS`):
   0xc18e3a5a &lt;+202&gt;:   push   %ebx
   0xc18e3a5b &lt;+203&gt;:   push   $0xc26001a0
   0xc18e3a60 &lt;+208&gt;:   call   0xc157d430 &lt;__ubsan_handle_out_of_bounds&gt;

The kernel's handler isn't expecting them on the stack. For example, this is setting a bit in the sanitizer's passed-in data structure (from __ubsan_handle_out_of_bounds):

   0xc157d491 &lt;+97&gt;:    btsl   $0x1f,%ds:0x4(%eax)
   0xc157d497 &lt;+103&gt;:   jae    0xc157d4a1 &lt;__ubsan_handle_out_of_bounds+113&gt;

KSPP/linux#350

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants