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

[libc] revisit trivial-auto-var-init=pattern #86282

Open
SchrodingerZhu opened this issue Mar 22, 2024 · 1 comment
Open

[libc] revisit trivial-auto-var-init=pattern #86282

SchrodingerZhu opened this issue Mar 22, 2024 · 1 comment
Labels

Comments

@SchrodingerZhu
Copy link
Contributor

SchrodingerZhu commented Mar 22, 2024

We have enabled trivial-auto-var-init=pattern to improve security around uninitialized variables with automatic storage duration. See #78776.

Normally, compiler is smart enough to omit pattern filling if the variable is initialized properly after its creation. However, we might want to revisit some situations to see if [[clang::uninitialized]] is needed.

Consider a common situation in internal usage of syscall wrappers. We may create a template trivial object on stack as buffer and then pass its address to the syscall interface as inout argument.

  LinuxStatFs result;
  // On 32-bit platforms, original fstatfs cannot handle large file systems.
  // In such cases, SYS_fstatfs64 is defined and should be used.
#ifdef SYS_fstatfs64
  int ret = syscall_impl<int>(SYS_fstatfs64, fd, sizeof(result), &result);
#else
  int ret = syscall_impl<int>(SYS_fstatfs, fd, &result);
#endif
  if (ret < 0) {
    libc_errno = -ret;
    return cpp::nullopt;
  }
  return result;

The compiler does not know that the syscall is to populate the fields of the struct. Hence, to be safe, the compiler classifies it as a possible use before initialization. As a result, one may see the following in the generated code:

image

For relative large local variables, it may be worthy to bare in mind that we are using pattern filling for trival auto variables. Adding [[clang::uninitialized]] will improve the code if we are certain that asm/syscall will initialize the variable properly.

@nickdesaulniers
Copy link
Member

Right, by giving the address of a variable to inline asm as input, it's unclear to the compiler if that address will be purely read from vs written to.

If all paths in the kernel initialize this value, it may be ok to leave it unitialized. But I do worry if some of the error paths mail fail to initialize the variable, which could result in the use of unitialized values being passed to llvmlibc's callers.

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

No branches or pull requests

2 participants