On Fri, Oct 16, 2020 at 01:12:24AM -0700, syzbot wrote: > dashboard link: https://syzkaller.appspot.com/bug?extid=92ead4eb8e26a26d465e > [...] > Reported-by: syzbot+92ead4eb8e26a26d465e@xxxxxxxxxxxxxxxxxxxxxxxxx > [...] > UBSAN: array-index-out-of-bounds in crypto/af_alg.c:166:2 > index 91 is out of range for type '__u8 [64]' This seems to be an "as intended", if very odd. false positive (the actual memory area is backed by the on-stack _K_SS_MAXSIZE-sized sockaddr_storage "address" variable in __sys_bind. But yes, af_alg's salg_name member size here doesn't make sense. The origin appears to be that 3f69cc60768b ("crypto: af_alg - Allow arbitrarily long algorithm names") intentionally didn't extend the kernel structure (which is actually just using the UAPI structure). I don't see a reason the UAPI couldn't have been extended: it's a sockaddr implementation, so the size is always passed in as part of the existing API. At the very least the kernel needs to switch to using a correctly-sized structure: I expected UBSAN_BOUNDS to be enabled globally by default at some point in the future (with the minimal runtime -- the instrumentation is tiny and catches real issues). Reproduction: struct sockaddr_alg sa = { .salg_family = AF_ALG, .salg_type = "skcipher", .salg_name = "cbc(aes)" }; fd = socket(AF_ALG, SOCK_SEQPACKET, 0); bind(fd, (void *)&sa, sizeof(sa)); Replace "sizeof(sa)" with x where 64<x<=128. -- Kees Cook
