Skip to content

Commit ecb4534

Browse files
q2venPaolo Abeni
authored and
Paolo Abeni
committed
af_unix: Terminate sun_path when bind()ing pathname socket.
kernel test robot reported slab-out-of-bounds access in strlen(). [0] Commit 06d4c8a ("af_unix: Fix fortify_panic() in unix_bind_bsd().") removed unix_mkname_bsd() call in unix_bind_bsd(). If sunaddr->sun_path is not terminated by user and we don't enable CONFIG_INIT_STACK_ALL_ZERO=y, strlen() will do the out-of-bounds access during file creation. Let's go back to strlen()-with-sockaddr_storage way and pack all 108 trickiness into unix_mkname_bsd() with bold comments. [0]: BUG: KASAN: slab-out-of-bounds in strlen (lib/string.c:?) Read of size 1 at addr ffff000015492777 by task fortify_strlen_/168 CPU: 0 PID: 168 Comm: fortify_strlen_ Not tainted 6.5.0-rc1-00333-g3329b603ebba #16 Hardware name: linux,dummy-virt (DT) Call trace: dump_backtrace (arch/arm64/kernel/stacktrace.c:235) show_stack (arch/arm64/kernel/stacktrace.c:242) dump_stack_lvl (lib/dump_stack.c:107) print_report (mm/kasan/report.c:365 mm/kasan/report.c:475) kasan_report (mm/kasan/report.c:590) __asan_report_load1_noabort (mm/kasan/report_generic.c:378) strlen (lib/string.c:?) getname_kernel (./include/linux/fortify-string.h:? fs/namei.c:226) kern_path_create (fs/namei.c:3926) unix_bind (net/unix/af_unix.c:1221 net/unix/af_unix.c:1324) __sys_bind (net/socket.c:1792) __arm64_sys_bind (net/socket.c:1801) invoke_syscall (arch/arm64/kernel/syscall.c:? arch/arm64/kernel/syscall.c:52) el0_svc_common (./include/linux/thread_info.h:127 arch/arm64/kernel/syscall.c:147) do_el0_svc (arch/arm64/kernel/syscall.c:189) el0_svc (./arch/arm64/include/asm/daifflags.h:28 arch/arm64/kernel/entry-common.c:133 arch/arm64/kernel/entry-common.c:144 arch/arm64/kernel/entry-common.c:648) el0t_64_sync_handler (arch/arm64/kernel/entry-common.c:?) el0t_64_sync (arch/arm64/kernel/entry.S:591) Allocated by task 168: kasan_set_track (mm/kasan/common.c:45 mm/kasan/common.c:52) kasan_save_alloc_info (mm/kasan/generic.c:512) __kasan_kmalloc (mm/kasan/common.c:383) __kmalloc (mm/slab_common.c:? mm/slab_common.c:998) unix_bind (net/unix/af_unix.c:257 net/unix/af_unix.c:1213 net/unix/af_unix.c:1324) __sys_bind (net/socket.c:1792) __arm64_sys_bind (net/socket.c:1801) invoke_syscall (arch/arm64/kernel/syscall.c:? arch/arm64/kernel/syscall.c:52) el0_svc_common (./include/linux/thread_info.h:127 arch/arm64/kernel/syscall.c:147) do_el0_svc (arch/arm64/kernel/syscall.c:189) el0_svc (./arch/arm64/include/asm/daifflags.h:28 arch/arm64/kernel/entry-common.c:133 arch/arm64/kernel/entry-common.c:144 arch/arm64/kernel/entry-common.c:648) el0t_64_sync_handler (arch/arm64/kernel/entry-common.c:?) el0t_64_sync (arch/arm64/kernel/entry.S:591) The buggy address belongs to the object at ffff000015492700 which belongs to the cache kmalloc-128 of size 128 The buggy address is located 0 bytes to the right of allocated 119-byte region [ffff000015492700, ffff000015492777) The buggy address belongs to the physical page: page:00000000aeab52ba refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x55492 anon flags: 0x3fffc0000000200(slab|node=0|zone=0|lastcpupid=0xffff) page_type: 0xffffffff() raw: 03fffc0000000200 ffff0000084018c0 fffffc00003d0e00 0000000000000005 raw: 0000000000000000 0000000080100010 00000001ffffffff 0000000000000000 page dumped because: kasan: bad access detected Memory state around the buggy address: ffff000015492600: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ffff000015492680: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc >ffff000015492700: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 07 fc ^ ffff000015492780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc ffff000015492800: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb Fixes: 06d4c8a ("af_unix: Fix fortify_panic() in unix_bind_bsd().") Reported-by: kernel test robot <[email protected]> Closes: https://lore.kernel.org/netdev/[email protected]/ Signed-off-by: Kuniyuki Iwashima <[email protected]> Reviewed-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Paolo Abeni <[email protected]>
1 parent e46e06f commit ecb4534

File tree

1 file changed

+16
-5
lines changed

1 file changed

+16
-5
lines changed

net/unix/af_unix.c

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -289,17 +289,29 @@ static int unix_validate_addr(struct sockaddr_un *sunaddr, int addr_len)
289289
return 0;
290290
}
291291

292-
static void unix_mkname_bsd(struct sockaddr_un *sunaddr, int addr_len)
292+
static int unix_mkname_bsd(struct sockaddr_un *sunaddr, int addr_len)
293293
{
294+
struct sockaddr_storage *addr = (struct sockaddr_storage *)sunaddr;
295+
short offset = offsetof(struct sockaddr_storage, __data);
296+
297+
BUILD_BUG_ON(offset != offsetof(struct sockaddr_un, sun_path));
298+
294299
/* This may look like an off by one error but it is a bit more
295300
* subtle. 108 is the longest valid AF_UNIX path for a binding.
296301
* sun_path[108] doesn't as such exist. However in kernel space
297302
* we are guaranteed that it is a valid memory location in our
298303
* kernel address buffer because syscall functions always pass
299304
* a pointer of struct sockaddr_storage which has a bigger buffer
300-
* than 108.
305+
* than 108. Also, we must terminate sun_path for strlen() in
306+
* getname_kernel().
307+
*/
308+
addr->__data[addr_len - offset] = 0;
309+
310+
/* Don't pass sunaddr->sun_path to strlen(). Otherwise, 108 will
311+
* cause panic if CONFIG_FORTIFY_SOURCE=y. Let __fortify_strlen()
312+
* know the actual buffer.
301313
*/
302-
((char *)sunaddr)[addr_len] = 0;
314+
return strlen(addr->__data) + offset + 1;
303315
}
304316

305317
static void __unix_remove_socket(struct sock *sk)
@@ -1208,8 +1220,7 @@ static int unix_bind_bsd(struct sock *sk, struct sockaddr_un *sunaddr,
12081220
struct path parent;
12091221
int err;
12101222

1211-
addr_len = strnlen(sunaddr->sun_path, sizeof(sunaddr->sun_path))
1212-
+ offsetof(struct sockaddr_un, sun_path) + 1;
1223+
addr_len = unix_mkname_bsd(sunaddr, addr_len);
12131224
addr = unix_create_addr(sunaddr, addr_len);
12141225
if (!addr)
12151226
return -ENOMEM;

0 commit comments

Comments
 (0)