Issue 209210
Summary [compiler-rt][sanitizer_common] Socket-address POST hooks unpoison a fixed 16 bytes instead of the written length
Labels new issue
Assignees
Reporter pdokoupil
    # Background

The `accept`, `accept4`, `getsockname`, `getpeername`, and `recvfrom` post-hooks unpoison a **fixed** `sizeof(sanitizer_kernel_sockaddr)` (16 bytes) of the returned address buffer, ignoring the actual length the kernel wrote (reported back through the `addrlen` out-parameter). The kernel writes `min(caller_capacity, real_addr_len)` bytes and sets `*addrlen` to `real_addr_len`.

So the fixed 16 B:
- **over-unpoisons** for short addresses (e.g. a 2-byte unnamed `AF_UNIX` address → 14 bytes of the buffer wrongly marked initialized → false negatives), and
- **under-unpoisons** for long addresses (a `sockaddr_un` path can be up to 110 bytes → reads of bytes 16.. wrongly reported → false positives).

# Affected code (all @ llvmorg-20.1.2, all still on `main`)

1. `accept` (see [L1932-L1940](https://github.com/llvm/llvm-project/blob/llvmorg-20.1.2/compiler-rt/lib/sanitizer_common/sanitizer_common_syscalls.inc#L1932-L1940).
2. `accept4` (see [L1945-L1953](https://github.com/llvm/llvm-project/blob/llvmorg-20.1.2/compiler-rt/lib/sanitizer_common/sanitizer_common_syscalls.inc#L1945-L1953))
3. `getsockname` (see [L1958-L1966](https://github.com/llvm/llvm-project/blob/llvmorg-20.1.2/compiler-rt/lib/sanitizer_common/sanitizer_common_syscalls.inc#L1958-L1966))
4. `getpeername` (see [L1971-L1979](https://github.com/llvm/llvm-project/blob/llvmorg-20.1.2/compiler-rt/lib/sanitizer_common/sanitizer_common_syscalls.inc#L1971-L1979))
5. `recvfrom` (`arg4` src_addr, see [L2030-L2041](https://github.com/llvm/llvm-project/blob/llvmorg-20.1.2/compiler-rt/lib/sanitizer_common/sanitizer_common_syscalls.inc#L2030-L2041))

## Example from `getsockname`:

```c
POST_SYSCALL(getsockname)
(long res, long arg0, sanitizer_kernel_sockaddr *arg1, void *arg2) {
  if (res >= 0) {
    if (arg1)
      POST_WRITE(arg1, sizeof(*arg1));   // fixed 16, not *arg2 (the written length)
    if (arg2)
      POST_WRITE(arg2, sizeof(unsigned));
  }
}
```

The libc interceptors seem to make this right as they clamp to the returned `*addrlen`, capped at the caller's capacity:
- [getsockname L2908-L2921](https://github.com/llvm/llvm-project/blob/llvmorg-20.1.2/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc#L2908-L2921),
- [accept L3121-L3141](https://github.com/llvm/llvm-project/blob/llvmorg-20.1.2/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc#L3121-L3141).

# Impact

Raw-syscall users (see details of our use case in https://github.com/llvm/llvm-project/issues/206978) get both directions of wrongness depending on the address length: masked uninitialized reads for short addresses, spurious reports for long ones. The `addrlen` out-param needed to size this correctly is available in the argument list.

# Reproducer (getsockname; over-unpoison direction)

```cpp
// clang++ -fsanitize=memory -g repro.cpp -o repro && ./repro
#include <sanitizer/linux_syscall_hooks.h>
#include <sanitizer/msan_interface.h>
#include <cstdio>

int main() {
  char addr[128];
  __msan_poison(addr, sizeof(addr));
  unsigned addrlen = 2;   // simulate that kernel wrote a 2-byte address (e.g. unnamed AF_UNIX)
  __sanitizer_syscall_post_getsockname(0, 3, addr, &addrlen);
  long off = __msan_test_shadow(addr + 2, 14);   // bytes the kernel did NOT write
  std::printf("bytes [2, 16) after post_getsockname: %s\n",
              off == -1 ? "UNPOISONED (bug: fixed-16 over-unpoison)" : "still poisoned (ok)");
  return off == -1 ? 1 : 0;
}
```

Observed (clang 15.0.7 and clang 20.1.x): `UNPOISONED (bug ...)`. The under-unpoison direction is the mirror image: with a >16-byte address the hook leaves bytes 16.. poisoned, producing false positives on read.

## Kernel evidence (v6.6): why `*addrlen` alone is not enough

All five hooks return their address through
[`move_addr_to_user`](https://github.com/torvalds/linux/blob/v6.6/net/socket.c#L275-L300)
(e.g. [`__sys_getsockname`](https://github.com/torvalds/linux/blob/v6.6/net/socket.c#L2085-L2110), [`__sys_recvfrom`](https://github.com/torvalds/linux/blob/v6.6/net/socket.c#L2247-L2252)):

```c
int move_addr_to_user(struct sockaddr_storage *kaddr, int klen,
                      void __user *uaddr, int __user *ulen) {
  int len;
  get_user(len, ulen);            // len = caller's buffer capacity (INPUT)
  if (len > klen) len = klen;     // bytes actually written = min(capacity, klen)
  if (len) copy_to_user(uaddr, kaddr, len);
  return __put_user(klen, ulen);  // *ulen := klen  (the FULL, untruncated length)
}
```

The number of bytes actually written is `min(pre_call_capacity, klen)`, but on return `*addrlen == klen` — the full address length, which may be **larger** than both what was written and the caller's buffer. So neither the fixed 16 nor the post-call `*addrlen` is the correct unpoison size.

# The Question

The correct unpoison size is the number of bytes the kernel copied, i.e. `min(pre_call_capacity, klen)`. A `POST_SYSCALL` hook needs *both* operands but only ever has *one*:
- `klen` — recoverable from the post-call `*addrlen`.
- `pre_call_capacity` — **unrecoverable**: `__put_user(klen, ulen)` overwrote it.

With only `klen` available, it seems that every stateless choice is wrong in a different ways:

| POST hook uses | address longer than 16 (IPv6 = 28, `AF_UNIX` ≤ 110) | buffer smaller than `klen` |
|---|---|---|
| `POST_WRITE(addr, 16)` (today) | under-unpoisons bytes `16..klen` → **false positives** on legit reads | probably tolerable (≤ 16 = the conventional minimum buffer) |
| `POST_WRITE(addr, *addrlen)` | correct *iff* the buffer was big enough | over-unpoisons **past the end of the buffer** → out-of-bounds shadow write + **false negatives** |

The libc interceptors seem to avoid the dilemma only because they run *around* the real call: they snapshot the capacity **before** it and clamp to it afterwards ([getsockname](https://github.com/llvm/llvm-project/blob/llvmorg-20.1.2/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc#L2908-L2921), [accept](https://github.com/llvm/llvm-project/blob/llvmorg-20.1.2/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc#L3121-L3141)).

Unfortunately the pre/post syscall-hook *pair* has no way to carry that information from the pre-hook to the post-hook, so it cannot do the clamp the same way it's done in libc.

Overall I can think of two viable resolutions (with different drawbacks):

1. **Fix it properly (a framework change).** Give the pre-hook a place to store the caller's `*addrlen`, and have the post-hook clamp to
   `min(saved_capacity, *addrlen)`, which is exactly the interceptor's approach. This, however, needs pre→post state (a thread-local, or a hook-signature change), so definitely a bigger scope change that might not be worth it.
2. **Document the limitation.** Keep a fixed-size unpoison and note that raw-syscall address-return sizing is approximate. Simplest, but leaves the both-directions inaccuracy in place.

I'm filing this as an **issue rather than a PR** because it's not obvious which of the two options (or other options I could have missed), makes more sense to be implemented, without having maintainers/someone else's opinion.
_______________________________________________
llvm-bugs mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-bugs

Reply via email to