2024-04-04 12:38:38

by Eric Dumazet

[permalink] [raw]
Subject: [PATCH net] Bluetooth: validate setsockopt( BT_PKT_STATUS / BT_DEFER_SETUP) user input

syzbot reported sco_sock_setsockopt() is copying data without
checking user input length.

BUG: KASAN: slab-out-of-bounds in copy_from_sockptr_offset include/linux/sockptr.h:49 [inline]
BUG: KASAN: slab-out-of-bounds in copy_from_sockptr include/linux/sockptr.h:55 [inline]
BUG: KASAN: slab-out-of-bounds in sco_sock_setsockopt+0xc0b/0xf90 net/bluetooth/sco.c:893
Read of size 4 at addr ffff88805f7b15a3 by task syz-executor.5/12578

CPU: 1 PID: 12578 Comm: syz-executor.5 Not tainted 6.8.0-syzkaller-08951-gfe46a7dd189e #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
print_address_description mm/kasan/report.c:377 [inline]
print_report+0x169/0x550 mm/kasan/report.c:488
kasan_report+0x143/0x180 mm/kasan/report.c:601
copy_from_sockptr_offset include/linux/sockptr.h:49 [inline]
copy_from_sockptr include/linux/sockptr.h:55 [inline]
sco_sock_setsockopt+0xc0b/0xf90 net/bluetooth/sco.c:893
do_sock_setsockopt+0x3b1/0x720 net/socket.c:2311
__sys_setsockopt+0x1ae/0x250 net/socket.c:2334
__do_sys_setsockopt net/socket.c:2343 [inline]
__se_sys_setsockopt net/socket.c:2340 [inline]
__x64_sys_setsockopt+0xb5/0xd0 net/socket.c:2340
do_syscall_64+0xfd/0x240
entry_SYSCALL_64_after_hwframe+0x6d/0x75
RIP: 0033:0x7f3c2487dde9
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 e1 20 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f3c256b40c8 EFLAGS: 00000246 ORIG_RAX: 0000000000000036
RAX: ffffffffffffffda RBX: 00007f3c249abf80 RCX: 00007f3c2487dde9
RDX: 0000000000000010 RSI: 0000000000000112 RDI: 0000000000000008
RBP: 00007f3c248ca47a R08: 0000000000000002 R09: 0000000000000000
R10: 0000000020000080 R11: 0000000000000246 R12: 0000000000000000
R13: 000000000000004d R14: 00007f3c249abf80 R15: 00007fff5dcf4978
</TASK>

Allocated by task 12578:
kasan_save_stack mm/kasan/common.c:47 [inline]
kasan_save_track+0x3f/0x80 mm/kasan/common.c:68
poison_kmalloc_redzone mm/kasan/common.c:370 [inline]
__kasan_kmalloc+0x98/0xb0 mm/kasan/common.c:387
kasan_kmalloc include/linux/kasan.h:211 [inline]
__do_kmalloc_node mm/slub.c:3966 [inline]
__kmalloc+0x233/0x4a0 mm/slub.c:3979
kmalloc include/linux/slab.h:632 [inline]
__cgroup_bpf_run_filter_setsockopt+0xd2f/0x1040 kernel/bpf/cgroup.c:1869
do_sock_setsockopt+0x6b4/0x720 net/socket.c:2293
__sys_setsockopt+0x1ae/0x250 net/socket.c:2334
__do_sys_setsockopt net/socket.c:2343 [inline]
__se_sys_setsockopt net/socket.c:2340 [inline]
__x64_sys_setsockopt+0xb5/0xd0 net/socket.c:2340
do_syscall_64+0xfd/0x240
entry_SYSCALL_64_after_hwframe+0x6d/0x75

The buggy address belongs to the object at ffff88805f7b15a0
which belongs to the cache kmalloc-8 of size 8
The buggy address is located 1 bytes to the right of
allocated 2-byte region [ffff88805f7b15a0, ffff88805f7b15a2)

The buggy address belongs to the physical page:
page:ffffea00017dec40 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x5f7b1
flags: 0xfff00000000800(slab|node=0|zone=1|lastcpupid=0x7ff)
page_type: 0xffffffff()
raw: 00fff00000000800 ffff888014c41280 ffffea0000a26d80 dead000000000002
raw: 0000000000000000 0000000080800080 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
page_owner tracks the page as allocated
page last allocated via order 0, migratetype Unmovable, gfp_mask 0x12cc0(GFP_KERNEL|__GFP_NOWARN|__GFP_NORETRY), pid 5091, tgid 5091 (syz-executor.3), ts 75758857522, free_ts 75730585588
set_page_owner include/linux/page_owner.h:31 [inline]
post_alloc_hook+0x1ea/0x210 mm/page_alloc.c:1533
prep_new_page mm/page_alloc.c:1540 [inline]
get_page_from_freelist+0x33ea/0x3580 mm/page_alloc.c:3311
__alloc_pages+0x256/0x680 mm/page_alloc.c:4569
__alloc_pages_node include/linux/gfp.h:238 [inline]
alloc_pages_node include/linux/gfp.h:261 [inline]
alloc_slab_page+0x5f/0x160 mm/slub.c:2175
allocate_slab mm/slub.c:2338 [inline]
new_slab+0x84/0x2f0 mm/slub.c:2391
___slab_alloc+0xc73/0x1260 mm/slub.c:3525
__slab_alloc mm/slub.c:3610 [inline]
__slab_alloc_node mm/slub.c:3663 [inline]
slab_alloc_node mm/slub.c:3835 [inline]
__do_kmalloc_node mm/slub.c:3965 [inline]
__kmalloc_node_track_caller+0x2d6/0x4e0 mm/slub.c:3986
kstrdup+0x3a/0x80 mm/util.c:62
__kernfs_new_node+0x9d/0x880 fs/kernfs/dir.c:611
kernfs_new_node+0x13a/0x240 fs/kernfs/dir.c:691
kernfs_create_dir_ns+0x43/0x120 fs/kernfs/dir.c:1052
sysfs_create_dir_ns+0x189/0x3a0 fs/sysfs/dir.c:59
create_dir lib/kobject.c:73 [inline]
kobject_add_internal+0x435/0x8d0 lib/kobject.c:240
kobject_add_varg lib/kobject.c:374 [inline]
kobject_init_and_add+0x124/0x190 lib/kobject.c:457
netdev_queue_add_kobject net/core/net-sysfs.c:1786 [inline]
netdev_queue_update_kobjects+0x1ee/0x5f0 net/core/net-sysfs.c:1838
register_queue_kobjects net/core/net-sysfs.c:1900 [inline]
netdev_register_kobject+0x265/0x320 net/core/net-sysfs.c:2140
page last free pid 5103 tgid 5103 stack trace:
reset_page_owner include/linux/page_owner.h:24 [inline]
free_pages_prepare mm/page_alloc.c:1140 [inline]
free_unref_page_prepare+0x968/0xa90 mm/page_alloc.c:2346
free_unref_page_list+0x5a3/0x850 mm/page_alloc.c:2532
release_pages+0x2744/0x2a80 mm/swap.c:1042
tlb_batch_pages_flush mm/mmu_gather.c:98 [inline]
tlb_flush_mmu_free mm/mmu_gather.c:293 [inline]
tlb_flush_mmu+0x34d/0x4e0 mm/mmu_gather.c:300
tlb_finish_mmu+0xd4/0x200 mm/mmu_gather.c:392
exit_mmap+0x4b6/0xd40 mm/mmap.c:3300
__mmput+0x115/0x3c0 kernel/fork.c:1345
exit_mm+0x220/0x310 kernel/exit.c:569
do_exit+0x99e/0x27e0 kernel/exit.c:865
do_group_exit+0x207/0x2c0 kernel/exit.c:1027
__do_sys_exit_group kernel/exit.c:1038 [inline]
__se_sys_exit_group kernel/exit.c:1036 [inline]
__x64_sys_exit_group+0x3f/0x40 kernel/exit.c:1036
do_syscall_64+0xfd/0x240
entry_SYSCALL_64_after_hwframe+0x6d/0x75

Memory state around the buggy address:
ffff88805f7b1480: 05 fc fc fc 05 fc fc fc fa fc fc fc 05 fc fc fc
ffff88805f7b1500: 05 fc fc fc 05 fc fc fc 05 fc fc fc 05 fc fc fc
>ffff88805f7b1580: 04 fc fc fc 02 fc fc fc fa fc fc fc 05 fc fc fc
^
ffff88805f7b1600: fa fc fc fc 05 fc fc fc fa fc fc fc 05 fc fc fc
ffff88805f7b1680: 05 fc fc fc 05 fc fc fc 00 fc fc fc fa fc fc fc

Fixes: 00398e1d5183 ("Bluetooth: Add support for BT_PKT_STATUS CMSG data for SCO connections")
Fixes: b96e9c671b05 ("Bluetooth: Add BT_DEFER_SETUP option to sco socket")
Reported-by: syzbot <[email protected]>
Signed-off-by: Eric Dumazet <[email protected]>
Cc: Marcel Holtmann <[email protected]>
Cc: Johan Hedberg <[email protected]>
Cc: Luiz Augusto von Dentz <[email protected]> (supporter:BLUETOOTH SUBSYSTEM)
Cc: [email protected]
---
net/bluetooth/sco.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 43daf965a01e4ac5c9329150080b00dcd63c7e1c..9d013f01865fd2509f28eac3bceadf682f0a5edb 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -843,6 +843,10 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
break;
}

+ if (optlen < sizeof(u32)) {
+ err = -EINVAL;
+ break;
+ }
if (copy_from_sockptr(&opt, optval, sizeof(u32))) {
err = -EFAULT;
break;
@@ -890,6 +894,10 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
break;

case BT_PKT_STATUS:
+ if (optlen < sizeof(u32)) {
+ err = -EINVAL;
+ break;
+ }
if (copy_from_sockptr(&opt, optval, sizeof(u32))) {
err = -EFAULT;
break;
--
2.44.0.478.gd926399ef9-goog



2024-04-04 13:09:26

by bluez.test.bot

[permalink] [raw]
Subject: RE: [net] Bluetooth: validate setsockopt( BT_PKT_STATUS / BT_DEFER_SETUP) user input

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=841415

---Test result---

Test Summary:
CheckPatch FAIL 0.99 seconds
GitLint FAIL 0.99 seconds
SubjectPrefix PASS 0.11 seconds
BuildKernel PASS 30.51 seconds
CheckAllWarning PASS 33.21 seconds
CheckSparse WARNING 38.89 seconds
CheckSmatch FAIL 35.46 seconds
BuildKernel32 PASS 29.60 seconds
TestRunnerSetup PASS 523.49 seconds
TestRunner_l2cap-tester PASS 18.03 seconds
TestRunner_iso-tester PASS 32.44 seconds
TestRunner_bnep-tester PASS 4.65 seconds
TestRunner_mgmt-tester PASS 111.42 seconds
TestRunner_rfcomm-tester PASS 7.25 seconds
TestRunner_sco-tester PASS 14.87 seconds
TestRunner_ioctl-tester PASS 7.67 seconds
TestRunner_mesh-tester PASS 5.78 seconds
TestRunner_smp-tester PASS 6.76 seconds
TestRunner_userchan-tester PASS 4.86 seconds
IncrementalBuild PASS 27.97 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[net] Bluetooth: validate setsockopt( BT_PKT_STATUS / BT_DEFER_SETUP) user input
WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#104:
CPU: 1 PID: 12578 Comm: syz-executor.5 Not tainted 6.8.0-syzkaller-08951-gfe46a7dd189e #0

WARNING: Possible repeated word: 'Google'
#105:
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024

WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
#225:
Reported-by: syzbot <[email protected]>
Signed-off-by: Eric Dumazet <[email protected]>

total: 0 errors, 3 warnings, 0 checks, 20 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/src/13617776.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[net] Bluetooth: validate setsockopt( BT_PKT_STATUS / BT_DEFER_SETUP) user input

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
6: B1 Line exceeds max length (95>80): " BUG: KASAN: slab-out-of-bounds in copy_from_sockptr_offset include/linux/sockptr.h:49 [inline]"
7: B1 Line exceeds max length (88>80): " BUG: KASAN: slab-out-of-bounds in copy_from_sockptr include/linux/sockptr.h:55 [inline]"
8: B1 Line exceeds max length (90>80): " BUG: KASAN: slab-out-of-bounds in sco_sock_setsockopt+0xc0b/0xf90 net/bluetooth/sco.c:893"
11: B1 Line exceeds max length (89>80): "CPU: 1 PID: 12578 Comm: syz-executor.5 Not tainted 6.8.0-syzkaller-08951-gfe46a7dd189e #0"
12: B1 Line exceeds max length (89>80): "Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024"
31: B1 Line exceeds max length (199>80): "Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 e1 20 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48"
64: B1 Line exceeds max length (90>80): "page:ffffea00017dec40 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x5f7b1"
71: B1 Line exceeds max length (186>80): "page last allocated via order 0, migratetype Unmovable, gfp_mask 0x12cc0(GFP_KERNEL|__GFP_NOWARN|__GFP_NORETRY), pid 5091, tgid 5091 (syz-executor.3), ts 75758857522, free_ts 75730585588"
134: B1 Line exceeds max length (81>80): "Cc: Luiz Augusto von Dentz <[email protected]> (supporter:BLUETOOTH SUBSYSTEM)"
##############################
Test: CheckSparse - WARNING
Desc: Run sparse tool with linux kernel
Output:
net/bluetooth/sco.c: note: in included file:./include/net/bluetooth/hci_core.h:150:35: warning: array of flexible structures
##############################
Test: CheckSmatch - FAIL
Desc: Run smatch tool with source
Output:

Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: net/bluetooth/hci_core.o] Error 139
make[4]: *** Deleting file 'net/bluetooth/hci_core.o'
make[3]: *** [scripts/Makefile.build:485: net/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:485: net] Error 2
make[2]: *** Waiting for unfinished jobs....
Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: drivers/bluetooth/bcm203x.o] Error 139
make[4]: *** Deleting file 'drivers/bluetooth/bcm203x.o'
make[4]: *** Waiting for unfinished jobs....
make[3]: *** [scripts/Makefile.build:485: drivers/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:485: drivers] Error 2
make[1]: *** [/github/workspace/src/src/Makefile:1919: .] Error 2
make: *** [Makefile:240: __sub-make] Error 2


---
Regards,
Linux Bluetooth

2024-04-04 18:31:29

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH net] Bluetooth: validate setsockopt( BT_PKT_STATUS / BT_DEFER_SETUP) user input

Hi Eric,

On Thu, Apr 4, 2024 at 8:36 AM Eric Dumazet <[email protected]> wrote:
>
> syzbot reported sco_sock_setsockopt() is copying data without
> checking user input length.
>
> BUG: KASAN: slab-out-of-bounds in copy_from_sockptr_offset include/linux/sockptr.h:49 [inline]
> BUG: KASAN: slab-out-of-bounds in copy_from_sockptr include/linux/sockptr.h:55 [inline]
> BUG: KASAN: slab-out-of-bounds in sco_sock_setsockopt+0xc0b/0xf90 net/bluetooth/sco.c:893
> Read of size 4 at addr ffff88805f7b15a3 by task syz-executor.5/12578
>
> CPU: 1 PID: 12578 Comm: syz-executor.5 Not tainted 6.8.0-syzkaller-08951-gfe46a7dd189e #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
> Call Trace:
> <TASK>
> __dump_stack lib/dump_stack.c:88 [inline]
> dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
> print_address_description mm/kasan/report.c:377 [inline]
> print_report+0x169/0x550 mm/kasan/report.c:488
> kasan_report+0x143/0x180 mm/kasan/report.c:601
> copy_from_sockptr_offset include/linux/sockptr.h:49 [inline]
> copy_from_sockptr include/linux/sockptr.h:55 [inline]
> sco_sock_setsockopt+0xc0b/0xf90 net/bluetooth/sco.c:893
> do_sock_setsockopt+0x3b1/0x720 net/socket.c:2311
> __sys_setsockopt+0x1ae/0x250 net/socket.c:2334
> __do_sys_setsockopt net/socket.c:2343 [inline]
> __se_sys_setsockopt net/socket.c:2340 [inline]
> __x64_sys_setsockopt+0xb5/0xd0 net/socket.c:2340
> do_syscall_64+0xfd/0x240
> entry_SYSCALL_64_after_hwframe+0x6d/0x75
> RIP: 0033:0x7f3c2487dde9
> Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 e1 20 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48
> RSP: 002b:00007f3c256b40c8 EFLAGS: 00000246 ORIG_RAX: 0000000000000036
> RAX: ffffffffffffffda RBX: 00007f3c249abf80 RCX: 00007f3c2487dde9
> RDX: 0000000000000010 RSI: 0000000000000112 RDI: 0000000000000008
> RBP: 00007f3c248ca47a R08: 0000000000000002 R09: 0000000000000000
> R10: 0000000020000080 R11: 0000000000000246 R12: 0000000000000000
> R13: 000000000000004d R14: 00007f3c249abf80 R15: 00007fff5dcf4978
> </TASK>
>
> Allocated by task 12578:
> kasan_save_stack mm/kasan/common.c:47 [inline]
> kasan_save_track+0x3f/0x80 mm/kasan/common.c:68
> poison_kmalloc_redzone mm/kasan/common.c:370 [inline]
> __kasan_kmalloc+0x98/0xb0 mm/kasan/common.c:387
> kasan_kmalloc include/linux/kasan.h:211 [inline]
> __do_kmalloc_node mm/slub.c:3966 [inline]
> __kmalloc+0x233/0x4a0 mm/slub.c:3979
> kmalloc include/linux/slab.h:632 [inline]
> __cgroup_bpf_run_filter_setsockopt+0xd2f/0x1040 kernel/bpf/cgroup.c:1869
> do_sock_setsockopt+0x6b4/0x720 net/socket.c:2293
> __sys_setsockopt+0x1ae/0x250 net/socket.c:2334
> __do_sys_setsockopt net/socket.c:2343 [inline]
> __se_sys_setsockopt net/socket.c:2340 [inline]
> __x64_sys_setsockopt+0xb5/0xd0 net/socket.c:2340
> do_syscall_64+0xfd/0x240
> entry_SYSCALL_64_after_hwframe+0x6d/0x75
>
> The buggy address belongs to the object at ffff88805f7b15a0
> which belongs to the cache kmalloc-8 of size 8
> The buggy address is located 1 bytes to the right of
> allocated 2-byte region [ffff88805f7b15a0, ffff88805f7b15a2)
>
> The buggy address belongs to the physical page:
> page:ffffea00017dec40 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x5f7b1
> flags: 0xfff00000000800(slab|node=0|zone=1|lastcpupid=0x7ff)
> page_type: 0xffffffff()
> raw: 00fff00000000800 ffff888014c41280 ffffea0000a26d80 dead000000000002
> raw: 0000000000000000 0000000080800080 00000001ffffffff 0000000000000000
> page dumped because: kasan: bad access detected
> page_owner tracks the page as allocated
> page last allocated via order 0, migratetype Unmovable, gfp_mask 0x12cc0(GFP_KERNEL|__GFP_NOWARN|__GFP_NORETRY), pid 5091, tgid 5091 (syz-executor.3), ts 75758857522, free_ts 75730585588
> set_page_owner include/linux/page_owner.h:31 [inline]
> post_alloc_hook+0x1ea/0x210 mm/page_alloc.c:1533
> prep_new_page mm/page_alloc.c:1540 [inline]
> get_page_from_freelist+0x33ea/0x3580 mm/page_alloc.c:3311
> __alloc_pages+0x256/0x680 mm/page_alloc.c:4569
> __alloc_pages_node include/linux/gfp.h:238 [inline]
> alloc_pages_node include/linux/gfp.h:261 [inline]
> alloc_slab_page+0x5f/0x160 mm/slub.c:2175
> allocate_slab mm/slub.c:2338 [inline]
> new_slab+0x84/0x2f0 mm/slub.c:2391
> ___slab_alloc+0xc73/0x1260 mm/slub.c:3525
> __slab_alloc mm/slub.c:3610 [inline]
> __slab_alloc_node mm/slub.c:3663 [inline]
> slab_alloc_node mm/slub.c:3835 [inline]
> __do_kmalloc_node mm/slub.c:3965 [inline]
> __kmalloc_node_track_caller+0x2d6/0x4e0 mm/slub.c:3986
> kstrdup+0x3a/0x80 mm/util.c:62
> __kernfs_new_node+0x9d/0x880 fs/kernfs/dir.c:611
> kernfs_new_node+0x13a/0x240 fs/kernfs/dir.c:691
> kernfs_create_dir_ns+0x43/0x120 fs/kernfs/dir.c:1052
> sysfs_create_dir_ns+0x189/0x3a0 fs/sysfs/dir.c:59
> create_dir lib/kobject.c:73 [inline]
> kobject_add_internal+0x435/0x8d0 lib/kobject.c:240
> kobject_add_varg lib/kobject.c:374 [inline]
> kobject_init_and_add+0x124/0x190 lib/kobject.c:457
> netdev_queue_add_kobject net/core/net-sysfs.c:1786 [inline]
> netdev_queue_update_kobjects+0x1ee/0x5f0 net/core/net-sysfs.c:1838
> register_queue_kobjects net/core/net-sysfs.c:1900 [inline]
> netdev_register_kobject+0x265/0x320 net/core/net-sysfs.c:2140
> page last free pid 5103 tgid 5103 stack trace:
> reset_page_owner include/linux/page_owner.h:24 [inline]
> free_pages_prepare mm/page_alloc.c:1140 [inline]
> free_unref_page_prepare+0x968/0xa90 mm/page_alloc.c:2346
> free_unref_page_list+0x5a3/0x850 mm/page_alloc.c:2532
> release_pages+0x2744/0x2a80 mm/swap.c:1042
> tlb_batch_pages_flush mm/mmu_gather.c:98 [inline]
> tlb_flush_mmu_free mm/mmu_gather.c:293 [inline]
> tlb_flush_mmu+0x34d/0x4e0 mm/mmu_gather.c:300
> tlb_finish_mmu+0xd4/0x200 mm/mmu_gather.c:392
> exit_mmap+0x4b6/0xd40 mm/mmap.c:3300
> __mmput+0x115/0x3c0 kernel/fork.c:1345
> exit_mm+0x220/0x310 kernel/exit.c:569
> do_exit+0x99e/0x27e0 kernel/exit.c:865
> do_group_exit+0x207/0x2c0 kernel/exit.c:1027
> __do_sys_exit_group kernel/exit.c:1038 [inline]
> __se_sys_exit_group kernel/exit.c:1036 [inline]
> __x64_sys_exit_group+0x3f/0x40 kernel/exit.c:1036
> do_syscall_64+0xfd/0x240
> entry_SYSCALL_64_after_hwframe+0x6d/0x75
>
> Memory state around the buggy address:
> ffff88805f7b1480: 05 fc fc fc 05 fc fc fc fa fc fc fc 05 fc fc fc
> ffff88805f7b1500: 05 fc fc fc 05 fc fc fc 05 fc fc fc 05 fc fc fc
> >ffff88805f7b1580: 04 fc fc fc 02 fc fc fc fa fc fc fc 05 fc fc fc
> ^
> ffff88805f7b1600: fa fc fc fc 05 fc fc fc fa fc fc fc 05 fc fc fc
> ffff88805f7b1680: 05 fc fc fc 05 fc fc fc 00 fc fc fc fa fc fc fc
>
> Fixes: 00398e1d5183 ("Bluetooth: Add support for BT_PKT_STATUS CMSG data for SCO connections")
> Fixes: b96e9c671b05 ("Bluetooth: Add BT_DEFER_SETUP option to sco socket")
> Reported-by: syzbot <[email protected]>
> Signed-off-by: Eric Dumazet <[email protected]>
> Cc: Marcel Holtmann <[email protected]>
> Cc: Johan Hedberg <[email protected]>
> Cc: Luiz Augusto von Dentz <[email protected]> (supporter:BLUETOOTH SUBSYSTEM)
> Cc: [email protected]
> ---
> net/bluetooth/sco.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index 43daf965a01e4ac5c9329150080b00dcd63c7e1c..9d013f01865fd2509f28eac3bceadf682f0a5edb 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -843,6 +843,10 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
> break;
> }
>
> + if (optlen < sizeof(u32)) {
> + err = -EINVAL;
> + break;
> + }

Usually we deal with this sort of problem by doing len =
min_t(unsigned int, sizeof(u32), optlen) so we truncate the value if
smaller, perhaps it would be better to have a helper function that
does len check internally.

> if (copy_from_sockptr(&opt, optval, sizeof(u32))) {
> err = -EFAULT;
> break;
> @@ -890,6 +894,10 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
> break;
>
> case BT_PKT_STATUS:
> + if (optlen < sizeof(u32)) {
> + err = -EINVAL;
> + break;
> + }
> if (copy_from_sockptr(&opt, optval, sizeof(u32))) {
> err = -EFAULT;
> break;
> --
> 2.44.0.478.gd926399ef9-goog
>


--
Luiz Augusto von Dentz

2024-04-04 19:04:43

by Eric Dumazet

[permalink] [raw]
Subject: Re: [PATCH net] Bluetooth: validate setsockopt( BT_PKT_STATUS / BT_DEFER_SETUP) user input

On Thu, Apr 4, 2024 at 8:31 PM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Eric,
>
> On Thu, Apr 4, 2024 at 8:36 AM Eric Dumazet <[email protected]> wrote:
> >
> > syzbot reported sco_sock_setsockopt() is copying data without
> > checking user input length.
> >
> > BUG: KASAN: slab-out-of-bounds in copy_from_sockptr_offset include/linux/sockptr.h:49 [inline]
> > BUG: KASAN: slab-out-of-bounds in copy_from_sockptr include/linux/sockptr.h:55 [inline]
> > BUG: KASAN: slab-out-of-bounds in sco_sock_setsockopt+0xc0b/0xf90 net/bluetooth/sco.c:893
> > Read of size 4 at addr ffff88805f7b15a3 by task syz-executor.5/12578
> >
> > CPU: 1 PID: 12578 Comm: syz-executor.5 Not tainted 6.8.0-syzkaller-08951-gfe46a7dd189e #0
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
> > Call Trace:
> > <TASK>
> > __dump_stack lib/dump_stack.c:88 [inline]
> > dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
> > print_address_description mm/kasan/report.c:377 [inline]
> > print_report+0x169/0x550 mm/kasan/report.c:488
> > kasan_report+0x143/0x180 mm/kasan/report.c:601
> > copy_from_sockptr_offset include/linux/sockptr.h:49 [inline]
> > copy_from_sockptr include/linux/sockptr.h:55 [inline]
> > sco_sock_setsockopt+0xc0b/0xf90 net/bluetooth/sco.c:893
> > do_sock_setsockopt+0x3b1/0x720 net/socket.c:2311
> > __sys_setsockopt+0x1ae/0x250 net/socket.c:2334
> > __do_sys_setsockopt net/socket.c:2343 [inline]
> > __se_sys_setsockopt net/socket.c:2340 [inline]
> > __x64_sys_setsockopt+0xb5/0xd0 net/socket.c:2340
> > do_syscall_64+0xfd/0x240
> > entry_SYSCALL_64_after_hwframe+0x6d/0x75
> > RIP: 0033:0x7f3c2487dde9
> > Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 e1 20 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48
> > RSP: 002b:00007f3c256b40c8 EFLAGS: 00000246 ORIG_RAX: 0000000000000036
> > RAX: ffffffffffffffda RBX: 00007f3c249abf80 RCX: 00007f3c2487dde9
> > RDX: 0000000000000010 RSI: 0000000000000112 RDI: 0000000000000008
> > RBP: 00007f3c248ca47a R08: 0000000000000002 R09: 0000000000000000
> > R10: 0000000020000080 R11: 0000000000000246 R12: 0000000000000000
> > R13: 000000000000004d R14: 00007f3c249abf80 R15: 00007fff5dcf4978
> > </TASK>
> >
> > Allocated by task 12578:
> > kasan_save_stack mm/kasan/common.c:47 [inline]
> > kasan_save_track+0x3f/0x80 mm/kasan/common.c:68
> > poison_kmalloc_redzone mm/kasan/common.c:370 [inline]
> > __kasan_kmalloc+0x98/0xb0 mm/kasan/common.c:387
> > kasan_kmalloc include/linux/kasan.h:211 [inline]
> > __do_kmalloc_node mm/slub.c:3966 [inline]
> > __kmalloc+0x233/0x4a0 mm/slub.c:3979
> > kmalloc include/linux/slab.h:632 [inline]
> > __cgroup_bpf_run_filter_setsockopt+0xd2f/0x1040 kernel/bpf/cgroup.c:1869
> > do_sock_setsockopt+0x6b4/0x720 net/socket.c:2293
> > __sys_setsockopt+0x1ae/0x250 net/socket.c:2334
> > __do_sys_setsockopt net/socket.c:2343 [inline]
> > __se_sys_setsockopt net/socket.c:2340 [inline]
> > __x64_sys_setsockopt+0xb5/0xd0 net/socket.c:2340
> > do_syscall_64+0xfd/0x240
> > entry_SYSCALL_64_after_hwframe+0x6d/0x75
> >
> > The buggy address belongs to the object at ffff88805f7b15a0
> > which belongs to the cache kmalloc-8 of size 8
> > The buggy address is located 1 bytes to the right of
> > allocated 2-byte region [ffff88805f7b15a0, ffff88805f7b15a2)
> >
> > The buggy address belongs to the physical page:
> > page:ffffea00017dec40 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x5f7b1
> > flags: 0xfff00000000800(slab|node=0|zone=1|lastcpupid=0x7ff)
> > page_type: 0xffffffff()
> > raw: 00fff00000000800 ffff888014c41280 ffffea0000a26d80 dead000000000002
> > raw: 0000000000000000 0000000080800080 00000001ffffffff 0000000000000000
> > page dumped because: kasan: bad access detected
> > page_owner tracks the page as allocated
> > page last allocated via order 0, migratetype Unmovable, gfp_mask 0x12cc0(GFP_KERNEL|__GFP_NOWARN|__GFP_NORETRY), pid 5091, tgid 5091 (syz-executor.3), ts 75758857522, free_ts 75730585588
> > set_page_owner include/linux/page_owner.h:31 [inline]
> > post_alloc_hook+0x1ea/0x210 mm/page_alloc.c:1533
> > prep_new_page mm/page_alloc.c:1540 [inline]
> > get_page_from_freelist+0x33ea/0x3580 mm/page_alloc.c:3311
> > __alloc_pages+0x256/0x680 mm/page_alloc.c:4569
> > __alloc_pages_node include/linux/gfp.h:238 [inline]
> > alloc_pages_node include/linux/gfp.h:261 [inline]
> > alloc_slab_page+0x5f/0x160 mm/slub.c:2175
> > allocate_slab mm/slub.c:2338 [inline]
> > new_slab+0x84/0x2f0 mm/slub.c:2391
> > ___slab_alloc+0xc73/0x1260 mm/slub.c:3525
> > __slab_alloc mm/slub.c:3610 [inline]
> > __slab_alloc_node mm/slub.c:3663 [inline]
> > slab_alloc_node mm/slub.c:3835 [inline]
> > __do_kmalloc_node mm/slub.c:3965 [inline]
> > __kmalloc_node_track_caller+0x2d6/0x4e0 mm/slub.c:3986
> > kstrdup+0x3a/0x80 mm/util.c:62
> > __kernfs_new_node+0x9d/0x880 fs/kernfs/dir.c:611
> > kernfs_new_node+0x13a/0x240 fs/kernfs/dir.c:691
> > kernfs_create_dir_ns+0x43/0x120 fs/kernfs/dir.c:1052
> > sysfs_create_dir_ns+0x189/0x3a0 fs/sysfs/dir.c:59
> > create_dir lib/kobject.c:73 [inline]
> > kobject_add_internal+0x435/0x8d0 lib/kobject.c:240
> > kobject_add_varg lib/kobject.c:374 [inline]
> > kobject_init_and_add+0x124/0x190 lib/kobject.c:457
> > netdev_queue_add_kobject net/core/net-sysfs.c:1786 [inline]
> > netdev_queue_update_kobjects+0x1ee/0x5f0 net/core/net-sysfs.c:1838
> > register_queue_kobjects net/core/net-sysfs.c:1900 [inline]
> > netdev_register_kobject+0x265/0x320 net/core/net-sysfs.c:2140
> > page last free pid 5103 tgid 5103 stack trace:
> > reset_page_owner include/linux/page_owner.h:24 [inline]
> > free_pages_prepare mm/page_alloc.c:1140 [inline]
> > free_unref_page_prepare+0x968/0xa90 mm/page_alloc.c:2346
> > free_unref_page_list+0x5a3/0x850 mm/page_alloc.c:2532
> > release_pages+0x2744/0x2a80 mm/swap.c:1042
> > tlb_batch_pages_flush mm/mmu_gather.c:98 [inline]
> > tlb_flush_mmu_free mm/mmu_gather.c:293 [inline]
> > tlb_flush_mmu+0x34d/0x4e0 mm/mmu_gather.c:300
> > tlb_finish_mmu+0xd4/0x200 mm/mmu_gather.c:392
> > exit_mmap+0x4b6/0xd40 mm/mmap.c:3300
> > __mmput+0x115/0x3c0 kernel/fork.c:1345
> > exit_mm+0x220/0x310 kernel/exit.c:569
> > do_exit+0x99e/0x27e0 kernel/exit.c:865
> > do_group_exit+0x207/0x2c0 kernel/exit.c:1027
> > __do_sys_exit_group kernel/exit.c:1038 [inline]
> > __se_sys_exit_group kernel/exit.c:1036 [inline]
> > __x64_sys_exit_group+0x3f/0x40 kernel/exit.c:1036
> > do_syscall_64+0xfd/0x240
> > entry_SYSCALL_64_after_hwframe+0x6d/0x75
> >
> > Memory state around the buggy address:
> > ffff88805f7b1480: 05 fc fc fc 05 fc fc fc fa fc fc fc 05 fc fc fc
> > ffff88805f7b1500: 05 fc fc fc 05 fc fc fc 05 fc fc fc 05 fc fc fc
> > >ffff88805f7b1580: 04 fc fc fc 02 fc fc fc fa fc fc fc 05 fc fc fc
> > ^
> > ffff88805f7b1600: fa fc fc fc 05 fc fc fc fa fc fc fc 05 fc fc fc
> > ffff88805f7b1680: 05 fc fc fc 05 fc fc fc 00 fc fc fc fa fc fc fc
> >
> > Fixes: 00398e1d5183 ("Bluetooth: Add support for BT_PKT_STATUS CMSG data for SCO connections")
> > Fixes: b96e9c671b05 ("Bluetooth: Add BT_DEFER_SETUP option to sco socket")
> > Reported-by: syzbot <[email protected]>
> > Signed-off-by: Eric Dumazet <[email protected]>
> > Cc: Marcel Holtmann <[email protected]>
> > Cc: Johan Hedberg <[email protected]>
> > Cc: Luiz Augusto von Dentz <[email protected]> (supporter:BLUETOOTH SUBSYSTEM)
> > Cc: [email protected]
> > ---
> > net/bluetooth/sco.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> > index 43daf965a01e4ac5c9329150080b00dcd63c7e1c..9d013f01865fd2509f28eac3bceadf682f0a5edb 100644
> > --- a/net/bluetooth/sco.c
> > +++ b/net/bluetooth/sco.c
> > @@ -843,6 +843,10 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
> > break;
> > }
> >
> > + if (optlen < sizeof(u32)) {
> > + err = -EINVAL;
> > + break;
> > + }
>
> Usually we deal with this sort of problem by doing len =
> min_t(unsigned int, sizeof(u32), optlen) so we truncate the value if
> smaller, perhaps it would be better to have a helper function that
> does len check internally.

Well, what happens if user provided optlen == 1 ?

opt would then contain 24 bits of garbage.

2024-04-04 21:07:59

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH net] Bluetooth: validate setsockopt( BT_PKT_STATUS / BT_DEFER_SETUP) user input

Hi Eric,

On Thu, Apr 4, 2024 at 3:04 PM Eric Dumazet <[email protected]> wrote:
>
> On Thu, Apr 4, 2024 at 8:31 PM Luiz Augusto von Dentz
> <[email protected]> wrote:
> >
> > Hi Eric,
> >
> > On Thu, Apr 4, 2024 at 8:36 AM Eric Dumazet <[email protected]> wrote:
> > >
> > > syzbot reported sco_sock_setsockopt() is copying data without
> > > checking user input length.
> > >
> > > BUG: KASAN: slab-out-of-bounds in copy_from_sockptr_offset include/linux/sockptr.h:49 [inline]
> > > BUG: KASAN: slab-out-of-bounds in copy_from_sockptr include/linux/sockptr.h:55 [inline]
> > > BUG: KASAN: slab-out-of-bounds in sco_sock_setsockopt+0xc0b/0xf90 net/bluetooth/sco.c:893
> > > Read of size 4 at addr ffff88805f7b15a3 by task syz-executor.5/12578
> > >
> > > CPU: 1 PID: 12578 Comm: syz-executor.5 Not tainted 6.8.0-syzkaller-08951-gfe46a7dd189e #0
> > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
> > > Call Trace:
> > > <TASK>
> > > __dump_stack lib/dump_stack.c:88 [inline]
> > > dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
> > > print_address_description mm/kasan/report.c:377 [inline]
> > > print_report+0x169/0x550 mm/kasan/report.c:488
> > > kasan_report+0x143/0x180 mm/kasan/report.c:601
> > > copy_from_sockptr_offset include/linux/sockptr.h:49 [inline]
> > > copy_from_sockptr include/linux/sockptr.h:55 [inline]
> > > sco_sock_setsockopt+0xc0b/0xf90 net/bluetooth/sco.c:893
> > > do_sock_setsockopt+0x3b1/0x720 net/socket.c:2311
> > > __sys_setsockopt+0x1ae/0x250 net/socket.c:2334
> > > __do_sys_setsockopt net/socket.c:2343 [inline]
> > > __se_sys_setsockopt net/socket.c:2340 [inline]
> > > __x64_sys_setsockopt+0xb5/0xd0 net/socket.c:2340
> > > do_syscall_64+0xfd/0x240
> > > entry_SYSCALL_64_after_hwframe+0x6d/0x75
> > > RIP: 0033:0x7f3c2487dde9
> > > Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 e1 20 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48
> > > RSP: 002b:00007f3c256b40c8 EFLAGS: 00000246 ORIG_RAX: 0000000000000036
> > > RAX: ffffffffffffffda RBX: 00007f3c249abf80 RCX: 00007f3c2487dde9
> > > RDX: 0000000000000010 RSI: 0000000000000112 RDI: 0000000000000008
> > > RBP: 00007f3c248ca47a R08: 0000000000000002 R09: 0000000000000000
> > > R10: 0000000020000080 R11: 0000000000000246 R12: 0000000000000000
> > > R13: 000000000000004d R14: 00007f3c249abf80 R15: 00007fff5dcf4978
> > > </TASK>
> > >
> > > Allocated by task 12578:
> > > kasan_save_stack mm/kasan/common.c:47 [inline]
> > > kasan_save_track+0x3f/0x80 mm/kasan/common.c:68
> > > poison_kmalloc_redzone mm/kasan/common.c:370 [inline]
> > > __kasan_kmalloc+0x98/0xb0 mm/kasan/common.c:387
> > > kasan_kmalloc include/linux/kasan.h:211 [inline]
> > > __do_kmalloc_node mm/slub.c:3966 [inline]
> > > __kmalloc+0x233/0x4a0 mm/slub.c:3979
> > > kmalloc include/linux/slab.h:632 [inline]
> > > __cgroup_bpf_run_filter_setsockopt+0xd2f/0x1040 kernel/bpf/cgroup.c:1869
> > > do_sock_setsockopt+0x6b4/0x720 net/socket.c:2293
> > > __sys_setsockopt+0x1ae/0x250 net/socket.c:2334
> > > __do_sys_setsockopt net/socket.c:2343 [inline]
> > > __se_sys_setsockopt net/socket.c:2340 [inline]
> > > __x64_sys_setsockopt+0xb5/0xd0 net/socket.c:2340
> > > do_syscall_64+0xfd/0x240
> > > entry_SYSCALL_64_after_hwframe+0x6d/0x75
> > >
> > > The buggy address belongs to the object at ffff88805f7b15a0
> > > which belongs to the cache kmalloc-8 of size 8
> > > The buggy address is located 1 bytes to the right of
> > > allocated 2-byte region [ffff88805f7b15a0, ffff88805f7b15a2)
> > >
> > > The buggy address belongs to the physical page:
> > > page:ffffea00017dec40 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x5f7b1
> > > flags: 0xfff00000000800(slab|node=0|zone=1|lastcpupid=0x7ff)
> > > page_type: 0xffffffff()
> > > raw: 00fff00000000800 ffff888014c41280 ffffea0000a26d80 dead000000000002
> > > raw: 0000000000000000 0000000080800080 00000001ffffffff 0000000000000000
> > > page dumped because: kasan: bad access detected
> > > page_owner tracks the page as allocated
> > > page last allocated via order 0, migratetype Unmovable, gfp_mask 0x12cc0(GFP_KERNEL|__GFP_NOWARN|__GFP_NORETRY), pid 5091, tgid 5091 (syz-executor.3), ts 75758857522, free_ts 75730585588
> > > set_page_owner include/linux/page_owner.h:31 [inline]
> > > post_alloc_hook+0x1ea/0x210 mm/page_alloc.c:1533
> > > prep_new_page mm/page_alloc.c:1540 [inline]
> > > get_page_from_freelist+0x33ea/0x3580 mm/page_alloc.c:3311
> > > __alloc_pages+0x256/0x680 mm/page_alloc.c:4569
> > > __alloc_pages_node include/linux/gfp.h:238 [inline]
> > > alloc_pages_node include/linux/gfp.h:261 [inline]
> > > alloc_slab_page+0x5f/0x160 mm/slub.c:2175
> > > allocate_slab mm/slub.c:2338 [inline]
> > > new_slab+0x84/0x2f0 mm/slub.c:2391
> > > ___slab_alloc+0xc73/0x1260 mm/slub.c:3525
> > > __slab_alloc mm/slub.c:3610 [inline]
> > > __slab_alloc_node mm/slub.c:3663 [inline]
> > > slab_alloc_node mm/slub.c:3835 [inline]
> > > __do_kmalloc_node mm/slub.c:3965 [inline]
> > > __kmalloc_node_track_caller+0x2d6/0x4e0 mm/slub.c:3986
> > > kstrdup+0x3a/0x80 mm/util.c:62
> > > __kernfs_new_node+0x9d/0x880 fs/kernfs/dir.c:611
> > > kernfs_new_node+0x13a/0x240 fs/kernfs/dir.c:691
> > > kernfs_create_dir_ns+0x43/0x120 fs/kernfs/dir.c:1052
> > > sysfs_create_dir_ns+0x189/0x3a0 fs/sysfs/dir.c:59
> > > create_dir lib/kobject.c:73 [inline]
> > > kobject_add_internal+0x435/0x8d0 lib/kobject.c:240
> > > kobject_add_varg lib/kobject.c:374 [inline]
> > > kobject_init_and_add+0x124/0x190 lib/kobject.c:457
> > > netdev_queue_add_kobject net/core/net-sysfs.c:1786 [inline]
> > > netdev_queue_update_kobjects+0x1ee/0x5f0 net/core/net-sysfs.c:1838
> > > register_queue_kobjects net/core/net-sysfs.c:1900 [inline]
> > > netdev_register_kobject+0x265/0x320 net/core/net-sysfs.c:2140
> > > page last free pid 5103 tgid 5103 stack trace:
> > > reset_page_owner include/linux/page_owner.h:24 [inline]
> > > free_pages_prepare mm/page_alloc.c:1140 [inline]
> > > free_unref_page_prepare+0x968/0xa90 mm/page_alloc.c:2346
> > > free_unref_page_list+0x5a3/0x850 mm/page_alloc.c:2532
> > > release_pages+0x2744/0x2a80 mm/swap.c:1042
> > > tlb_batch_pages_flush mm/mmu_gather.c:98 [inline]
> > > tlb_flush_mmu_free mm/mmu_gather.c:293 [inline]
> > > tlb_flush_mmu+0x34d/0x4e0 mm/mmu_gather.c:300
> > > tlb_finish_mmu+0xd4/0x200 mm/mmu_gather.c:392
> > > exit_mmap+0x4b6/0xd40 mm/mmap.c:3300
> > > __mmput+0x115/0x3c0 kernel/fork.c:1345
> > > exit_mm+0x220/0x310 kernel/exit.c:569
> > > do_exit+0x99e/0x27e0 kernel/exit.c:865
> > > do_group_exit+0x207/0x2c0 kernel/exit.c:1027
> > > __do_sys_exit_group kernel/exit.c:1038 [inline]
> > > __se_sys_exit_group kernel/exit.c:1036 [inline]
> > > __x64_sys_exit_group+0x3f/0x40 kernel/exit.c:1036
> > > do_syscall_64+0xfd/0x240
> > > entry_SYSCALL_64_after_hwframe+0x6d/0x75
> > >
> > > Memory state around the buggy address:
> > > ffff88805f7b1480: 05 fc fc fc 05 fc fc fc fa fc fc fc 05 fc fc fc
> > > ffff88805f7b1500: 05 fc fc fc 05 fc fc fc 05 fc fc fc 05 fc fc fc
> > > >ffff88805f7b1580: 04 fc fc fc 02 fc fc fc fa fc fc fc 05 fc fc fc
> > > ^
> > > ffff88805f7b1600: fa fc fc fc 05 fc fc fc fa fc fc fc 05 fc fc fc
> > > ffff88805f7b1680: 05 fc fc fc 05 fc fc fc 00 fc fc fc fa fc fc fc
> > >
> > > Fixes: 00398e1d5183 ("Bluetooth: Add support for BT_PKT_STATUS CMSG data for SCO connections")
> > > Fixes: b96e9c671b05 ("Bluetooth: Add BT_DEFER_SETUP option to sco socket")
> > > Reported-by: syzbot <[email protected]>
> > > Signed-off-by: Eric Dumazet <[email protected]>
> > > Cc: Marcel Holtmann <[email protected]>
> > > Cc: Johan Hedberg <[email protected]>
> > > Cc: Luiz Augusto von Dentz <[email protected]> (supporter:BLUETOOTH SUBSYSTEM)
> > > Cc: [email protected]
> > > ---
> > > net/bluetooth/sco.c | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> > > index 43daf965a01e4ac5c9329150080b00dcd63c7e1c..9d013f01865fd2509f28eac3bceadf682f0a5edb 100644
> > > --- a/net/bluetooth/sco.c
> > > +++ b/net/bluetooth/sco.c
> > > @@ -843,6 +843,10 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
> > > break;
> > > }
> > >
> > > + if (optlen < sizeof(u32)) {
> > > + err = -EINVAL;
> > > + break;
> > > + }
> >
> > Usually we deal with this sort of problem by doing len =
> > min_t(unsigned int, sizeof(u32), optlen) so we truncate the value if
> > smaller, perhaps it would be better to have a helper function that
> > does len check internally.
>
> Well, what happens if user provided optlen == 1 ?
>
> opt would then contain 24 bits of garbage.

We might need to do a memset so if userspace just send one byte that
would still work, not that are actually doing memset right now so you
are right that in that respect it is still broken since it accessing
uninitialized value which perhaps is not triggering any build errors
but I wonder if we should attempt to fix that as well.

--
Luiz Augusto von Dentz

2024-04-05 10:28:06

by Eric Dumazet

[permalink] [raw]
Subject: Re: [PATCH net] Bluetooth: validate setsockopt( BT_PKT_STATUS / BT_DEFER_SETUP) user input

On Thu, Apr 4, 2024 at 11:07 PM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Eric,
>
> On Thu, Apr 4, 2024 at 3:04 PM Eric Dumazet <[email protected]> wrote:
> >
> > On Thu, Apr 4, 2024 at 8:31 PM Luiz Augusto von Dentz
> > <[email protected]> wrote:
> > >
> > > Hi Eric,
> > >
> > > On Thu, Apr 4, 2024 at 8:36 AM Eric Dumazet <[email protected]> wrote:
> > > >
> > > > syzbot reported sco_sock_setsockopt() is copying data without
> > > > checking user input length.
> > > >
> > > > BUG: KASAN: slab-out-of-bounds in copy_from_sockptr_offset include/linux/sockptr.h:49 [inline]
> > > > BUG: KASAN: slab-out-of-bounds in copy_from_sockptr include/linux/sockptr.h:55 [inline]
> > > > BUG: KASAN: slab-out-of-bounds in sco_sock_setsockopt+0xc0b/0xf90 net/bluetooth/sco.c:893
> > > > Read of size 4 at addr ffff88805f7b15a3 by task syz-executor.5/12578
> > > >
> > > > CPU: 1 PID: 12578 Comm: syz-executor.5 Not tainted 6.8.0-syzkaller-08951-gfe46a7dd189e #0
> > > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
> > > > Call Trace:
> > > > <TASK>
> > > > __dump_stack lib/dump_stack.c:88 [inline]
> > > > dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
> > > > print_address_description mm/kasan/report.c:377 [inline]
> > > > print_report+0x169/0x550 mm/kasan/report.c:488
> > > > kasan_report+0x143/0x180 mm/kasan/report.c:601
> > > > copy_from_sockptr_offset include/linux/sockptr.h:49 [inline]
> > > > copy_from_sockptr include/linux/sockptr.h:55 [inline]
> > > > sco_sock_setsockopt+0xc0b/0xf90 net/bluetooth/sco.c:893
> > > > do_sock_setsockopt+0x3b1/0x720 net/socket.c:2311
> > > > __sys_setsockopt+0x1ae/0x250 net/socket.c:2334
> > > > __do_sys_setsockopt net/socket.c:2343 [inline]
> > > > __se_sys_setsockopt net/socket.c:2340 [inline]
> > > > __x64_sys_setsockopt+0xb5/0xd0 net/socket.c:2340
> > > > do_syscall_64+0xfd/0x240
> > > > entry_SYSCALL_64_after_hwframe+0x6d/0x75
> > > > RIP: 0033:0x7f3c2487dde9
> > > > Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 e1 20 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48
> > > > RSP: 002b:00007f3c256b40c8 EFLAGS: 00000246 ORIG_RAX: 0000000000000036
> > > > RAX: ffffffffffffffda RBX: 00007f3c249abf80 RCX: 00007f3c2487dde9
> > > > RDX: 0000000000000010 RSI: 0000000000000112 RDI: 0000000000000008
> > > > RBP: 00007f3c248ca47a R08: 0000000000000002 R09: 0000000000000000
> > > > R10: 0000000020000080 R11: 0000000000000246 R12: 0000000000000000
> > > > R13: 000000000000004d R14: 00007f3c249abf80 R15: 00007fff5dcf4978
> > > > </TASK>
> > > >
> > > > Allocated by task 12578:
> > > > kasan_save_stack mm/kasan/common.c:47 [inline]
> > > > kasan_save_track+0x3f/0x80 mm/kasan/common.c:68
> > > > poison_kmalloc_redzone mm/kasan/common.c:370 [inline]
> > > > __kasan_kmalloc+0x98/0xb0 mm/kasan/common.c:387
> > > > kasan_kmalloc include/linux/kasan.h:211 [inline]
> > > > __do_kmalloc_node mm/slub.c:3966 [inline]
> > > > __kmalloc+0x233/0x4a0 mm/slub.c:3979
> > > > kmalloc include/linux/slab.h:632 [inline]
> > > > __cgroup_bpf_run_filter_setsockopt+0xd2f/0x1040 kernel/bpf/cgroup.c:1869
> > > > do_sock_setsockopt+0x6b4/0x720 net/socket.c:2293
> > > > __sys_setsockopt+0x1ae/0x250 net/socket.c:2334
> > > > __do_sys_setsockopt net/socket.c:2343 [inline]
> > > > __se_sys_setsockopt net/socket.c:2340 [inline]
> > > > __x64_sys_setsockopt+0xb5/0xd0 net/socket.c:2340
> > > > do_syscall_64+0xfd/0x240
> > > > entry_SYSCALL_64_after_hwframe+0x6d/0x75
> > > >
> > > > The buggy address belongs to the object at ffff88805f7b15a0
> > > > which belongs to the cache kmalloc-8 of size 8
> > > > The buggy address is located 1 bytes to the right of
> > > > allocated 2-byte region [ffff88805f7b15a0, ffff88805f7b15a2)
> > > >
> > > > The buggy address belongs to the physical page:
> > > > page:ffffea00017dec40 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x5f7b1
> > > > flags: 0xfff00000000800(slab|node=0|zone=1|lastcpupid=0x7ff)
> > > > page_type: 0xffffffff()
> > > > raw: 00fff00000000800 ffff888014c41280 ffffea0000a26d80 dead000000000002
> > > > raw: 0000000000000000 0000000080800080 00000001ffffffff 0000000000000000
> > > > page dumped because: kasan: bad access detected
> > > > page_owner tracks the page as allocated
> > > > page last allocated via order 0, migratetype Unmovable, gfp_mask 0x12cc0(GFP_KERNEL|__GFP_NOWARN|__GFP_NORETRY), pid 5091, tgid 5091 (syz-executor.3), ts 75758857522, free_ts 75730585588
> > > > set_page_owner include/linux/page_owner.h:31 [inline]
> > > > post_alloc_hook+0x1ea/0x210 mm/page_alloc.c:1533
> > > > prep_new_page mm/page_alloc.c:1540 [inline]
> > > > get_page_from_freelist+0x33ea/0x3580 mm/page_alloc.c:3311
> > > > __alloc_pages+0x256/0x680 mm/page_alloc.c:4569
> > > > __alloc_pages_node include/linux/gfp.h:238 [inline]
> > > > alloc_pages_node include/linux/gfp.h:261 [inline]
> > > > alloc_slab_page+0x5f/0x160 mm/slub.c:2175
> > > > allocate_slab mm/slub.c:2338 [inline]
> > > > new_slab+0x84/0x2f0 mm/slub.c:2391
> > > > ___slab_alloc+0xc73/0x1260 mm/slub.c:3525
> > > > __slab_alloc mm/slub.c:3610 [inline]
> > > > __slab_alloc_node mm/slub.c:3663 [inline]
> > > > slab_alloc_node mm/slub.c:3835 [inline]
> > > > __do_kmalloc_node mm/slub.c:3965 [inline]
> > > > __kmalloc_node_track_caller+0x2d6/0x4e0 mm/slub.c:3986
> > > > kstrdup+0x3a/0x80 mm/util.c:62
> > > > __kernfs_new_node+0x9d/0x880 fs/kernfs/dir.c:611
> > > > kernfs_new_node+0x13a/0x240 fs/kernfs/dir.c:691
> > > > kernfs_create_dir_ns+0x43/0x120 fs/kernfs/dir.c:1052
> > > > sysfs_create_dir_ns+0x189/0x3a0 fs/sysfs/dir.c:59
> > > > create_dir lib/kobject.c:73 [inline]
> > > > kobject_add_internal+0x435/0x8d0 lib/kobject.c:240
> > > > kobject_add_varg lib/kobject.c:374 [inline]
> > > > kobject_init_and_add+0x124/0x190 lib/kobject.c:457
> > > > netdev_queue_add_kobject net/core/net-sysfs.c:1786 [inline]
> > > > netdev_queue_update_kobjects+0x1ee/0x5f0 net/core/net-sysfs.c:1838
> > > > register_queue_kobjects net/core/net-sysfs.c:1900 [inline]
> > > > netdev_register_kobject+0x265/0x320 net/core/net-sysfs.c:2140
> > > > page last free pid 5103 tgid 5103 stack trace:
> > > > reset_page_owner include/linux/page_owner.h:24 [inline]
> > > > free_pages_prepare mm/page_alloc.c:1140 [inline]
> > > > free_unref_page_prepare+0x968/0xa90 mm/page_alloc.c:2346
> > > > free_unref_page_list+0x5a3/0x850 mm/page_alloc.c:2532
> > > > release_pages+0x2744/0x2a80 mm/swap.c:1042
> > > > tlb_batch_pages_flush mm/mmu_gather.c:98 [inline]
> > > > tlb_flush_mmu_free mm/mmu_gather.c:293 [inline]
> > > > tlb_flush_mmu+0x34d/0x4e0 mm/mmu_gather.c:300
> > > > tlb_finish_mmu+0xd4/0x200 mm/mmu_gather.c:392
> > > > exit_mmap+0x4b6/0xd40 mm/mmap.c:3300
> > > > __mmput+0x115/0x3c0 kernel/fork.c:1345
> > > > exit_mm+0x220/0x310 kernel/exit.c:569
> > > > do_exit+0x99e/0x27e0 kernel/exit.c:865
> > > > do_group_exit+0x207/0x2c0 kernel/exit.c:1027
> > > > __do_sys_exit_group kernel/exit.c:1038 [inline]
> > > > __se_sys_exit_group kernel/exit.c:1036 [inline]
> > > > __x64_sys_exit_group+0x3f/0x40 kernel/exit.c:1036
> > > > do_syscall_64+0xfd/0x240
> > > > entry_SYSCALL_64_after_hwframe+0x6d/0x75
> > > >
> > > > Memory state around the buggy address:
> > > > ffff88805f7b1480: 05 fc fc fc 05 fc fc fc fa fc fc fc 05 fc fc fc
> > > > ffff88805f7b1500: 05 fc fc fc 05 fc fc fc 05 fc fc fc 05 fc fc fc
> > > > >ffff88805f7b1580: 04 fc fc fc 02 fc fc fc fa fc fc fc 05 fc fc fc
> > > > ^
> > > > ffff88805f7b1600: fa fc fc fc 05 fc fc fc fa fc fc fc 05 fc fc fc
> > > > ffff88805f7b1680: 05 fc fc fc 05 fc fc fc 00 fc fc fc fa fc fc fc
> > > >
> > > > Fixes: 00398e1d5183 ("Bluetooth: Add support for BT_PKT_STATUS CMSG data for SCO connections")
> > > > Fixes: b96e9c671b05 ("Bluetooth: Add BT_DEFER_SETUP option to sco socket")
> > > > Reported-by: syzbot <[email protected]>
> > > > Signed-off-by: Eric Dumazet <[email protected]>
> > > > Cc: Marcel Holtmann <[email protected]>
> > > > Cc: Johan Hedberg <[email protected]>
> > > > Cc: Luiz Augusto von Dentz <[email protected]> (supporter:BLUETOOTH SUBSYSTEM)
> > > > Cc: [email protected]
> > > > ---
> > > > net/bluetooth/sco.c | 8 ++++++++
> > > > 1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> > > > index 43daf965a01e4ac5c9329150080b00dcd63c7e1c..9d013f01865fd2509f28eac3bceadf682f0a5edb 100644
> > > > --- a/net/bluetooth/sco.c
> > > > +++ b/net/bluetooth/sco.c
> > > > @@ -843,6 +843,10 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
> > > > break;
> > > > }
> > > >
> > > > + if (optlen < sizeof(u32)) {
> > > > + err = -EINVAL;
> > > > + break;
> > > > + }
> > >
> > > Usually we deal with this sort of problem by doing len =
> > > min_t(unsigned int, sizeof(u32), optlen) so we truncate the value if
> > > smaller, perhaps it would be better to have a helper function that
> > > does len check internally.
> >
> > Well, what happens if user provided optlen == 1 ?
> >
> > opt would then contain 24 bits of garbage.
>
> We might need to do a memset so if userspace just send one byte that
> would still work, not that are actually doing memset right now so you
> are right that in that respect it is still broken since it accessing
> uninitialized value which perhaps is not triggering any build errors
> but I wonder if we should attempt to fix that as well.

Historically, because of old BSD conventions, IPv4 do_ip_setsockopt()
has accepted optlen == 1,
but for instance IPv6 has been strict from day 1.

do_ipv6_setsockopt() does not accept int options in a char

Not sure why you want bluetooh to adopt lazy behavior, given that no
application could possibly
have used this so far (without risking a kernel bug)

2024-04-05 16:25:00

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH net] Bluetooth: validate setsockopt( BT_PKT_STATUS / BT_DEFER_SETUP) user input

Hi Eric,

On Fri, Apr 5, 2024 at 6:19 AM Eric Dumazet <[email protected]> wrote:
>
> On Thu, Apr 4, 2024 at 11:07 PM Luiz Augusto von Dentz
> <[email protected]> wrote:
> >
> > Hi Eric,
> >
> > On Thu, Apr 4, 2024 at 3:04 PM Eric Dumazet <[email protected]> wrote:
> > >
> > > On Thu, Apr 4, 2024 at 8:31 PM Luiz Augusto von Dentz
> > > <[email protected]> wrote:
> > > >
> > > > Hi Eric,
> > > >
> > > > On Thu, Apr 4, 2024 at 8:36 AM Eric Dumazet <[email protected]> wrote:
> > > > >
> > > > > syzbot reported sco_sock_setsockopt() is copying data without
> > > > > checking user input length.
> > > > >
> > > > > BUG: KASAN: slab-out-of-bounds in copy_from_sockptr_offset include/linux/sockptr.h:49 [inline]
> > > > > BUG: KASAN: slab-out-of-bounds in copy_from_sockptr include/linux/sockptr.h:55 [inline]
> > > > > BUG: KASAN: slab-out-of-bounds in sco_sock_setsockopt+0xc0b/0xf90 net/bluetooth/sco.c:893
> > > > > Read of size 4 at addr ffff88805f7b15a3 by task syz-executor.5/12578
> > > > >
> > > > > CPU: 1 PID: 12578 Comm: syz-executor.5 Not tainted 6.8.0-syzkaller-08951-gfe46a7dd189e #0
> > > > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
> > > > > Call Trace:
> > > > > <TASK>
> > > > > __dump_stack lib/dump_stack.c:88 [inline]
> > > > > dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
> > > > > print_address_description mm/kasan/report.c:377 [inline]
> > > > > print_report+0x169/0x550 mm/kasan/report.c:488
> > > > > kasan_report+0x143/0x180 mm/kasan/report.c:601
> > > > > copy_from_sockptr_offset include/linux/sockptr.h:49 [inline]
> > > > > copy_from_sockptr include/linux/sockptr.h:55 [inline]
> > > > > sco_sock_setsockopt+0xc0b/0xf90 net/bluetooth/sco.c:893
> > > > > do_sock_setsockopt+0x3b1/0x720 net/socket.c:2311
> > > > > __sys_setsockopt+0x1ae/0x250 net/socket.c:2334
> > > > > __do_sys_setsockopt net/socket.c:2343 [inline]
> > > > > __se_sys_setsockopt net/socket.c:2340 [inline]
> > > > > __x64_sys_setsockopt+0xb5/0xd0 net/socket.c:2340
> > > > > do_syscall_64+0xfd/0x240
> > > > > entry_SYSCALL_64_after_hwframe+0x6d/0x75
> > > > > RIP: 0033:0x7f3c2487dde9
> > > > > Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 e1 20 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48
> > > > > RSP: 002b:00007f3c256b40c8 EFLAGS: 00000246 ORIG_RAX: 0000000000000036
> > > > > RAX: ffffffffffffffda RBX: 00007f3c249abf80 RCX: 00007f3c2487dde9
> > > > > RDX: 0000000000000010 RSI: 0000000000000112 RDI: 0000000000000008
> > > > > RBP: 00007f3c248ca47a R08: 0000000000000002 R09: 0000000000000000
> > > > > R10: 0000000020000080 R11: 0000000000000246 R12: 0000000000000000
> > > > > R13: 000000000000004d R14: 00007f3c249abf80 R15: 00007fff5dcf4978
> > > > > </TASK>
> > > > >
> > > > > Allocated by task 12578:
> > > > > kasan_save_stack mm/kasan/common.c:47 [inline]
> > > > > kasan_save_track+0x3f/0x80 mm/kasan/common.c:68
> > > > > poison_kmalloc_redzone mm/kasan/common.c:370 [inline]
> > > > > __kasan_kmalloc+0x98/0xb0 mm/kasan/common.c:387
> > > > > kasan_kmalloc include/linux/kasan.h:211 [inline]
> > > > > __do_kmalloc_node mm/slub.c:3966 [inline]
> > > > > __kmalloc+0x233/0x4a0 mm/slub.c:3979
> > > > > kmalloc include/linux/slab.h:632 [inline]
> > > > > __cgroup_bpf_run_filter_setsockopt+0xd2f/0x1040 kernel/bpf/cgroup.c:1869
> > > > > do_sock_setsockopt+0x6b4/0x720 net/socket.c:2293
> > > > > __sys_setsockopt+0x1ae/0x250 net/socket.c:2334
> > > > > __do_sys_setsockopt net/socket.c:2343 [inline]
> > > > > __se_sys_setsockopt net/socket.c:2340 [inline]
> > > > > __x64_sys_setsockopt+0xb5/0xd0 net/socket.c:2340
> > > > > do_syscall_64+0xfd/0x240
> > > > > entry_SYSCALL_64_after_hwframe+0x6d/0x75
> > > > >
> > > > > The buggy address belongs to the object at ffff88805f7b15a0
> > > > > which belongs to the cache kmalloc-8 of size 8
> > > > > The buggy address is located 1 bytes to the right of
> > > > > allocated 2-byte region [ffff88805f7b15a0, ffff88805f7b15a2)
> > > > >
> > > > > The buggy address belongs to the physical page:
> > > > > page:ffffea00017dec40 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x5f7b1
> > > > > flags: 0xfff00000000800(slab|node=0|zone=1|lastcpupid=0x7ff)
> > > > > page_type: 0xffffffff()
> > > > > raw: 00fff00000000800 ffff888014c41280 ffffea0000a26d80 dead000000000002
> > > > > raw: 0000000000000000 0000000080800080 00000001ffffffff 0000000000000000
> > > > > page dumped because: kasan: bad access detected
> > > > > page_owner tracks the page as allocated
> > > > > page last allocated via order 0, migratetype Unmovable, gfp_mask 0x12cc0(GFP_KERNEL|__GFP_NOWARN|__GFP_NORETRY), pid 5091, tgid 5091 (syz-executor.3), ts 75758857522, free_ts 75730585588
> > > > > set_page_owner include/linux/page_owner.h:31 [inline]
> > > > > post_alloc_hook+0x1ea/0x210 mm/page_alloc.c:1533
> > > > > prep_new_page mm/page_alloc.c:1540 [inline]
> > > > > get_page_from_freelist+0x33ea/0x3580 mm/page_alloc.c:3311
> > > > > __alloc_pages+0x256/0x680 mm/page_alloc.c:4569
> > > > > __alloc_pages_node include/linux/gfp.h:238 [inline]
> > > > > alloc_pages_node include/linux/gfp.h:261 [inline]
> > > > > alloc_slab_page+0x5f/0x160 mm/slub.c:2175
> > > > > allocate_slab mm/slub.c:2338 [inline]
> > > > > new_slab+0x84/0x2f0 mm/slub.c:2391
> > > > > ___slab_alloc+0xc73/0x1260 mm/slub.c:3525
> > > > > __slab_alloc mm/slub.c:3610 [inline]
> > > > > __slab_alloc_node mm/slub.c:3663 [inline]
> > > > > slab_alloc_node mm/slub.c:3835 [inline]
> > > > > __do_kmalloc_node mm/slub.c:3965 [inline]
> > > > > __kmalloc_node_track_caller+0x2d6/0x4e0 mm/slub.c:3986
> > > > > kstrdup+0x3a/0x80 mm/util.c:62
> > > > > __kernfs_new_node+0x9d/0x880 fs/kernfs/dir.c:611
> > > > > kernfs_new_node+0x13a/0x240 fs/kernfs/dir.c:691
> > > > > kernfs_create_dir_ns+0x43/0x120 fs/kernfs/dir.c:1052
> > > > > sysfs_create_dir_ns+0x189/0x3a0 fs/sysfs/dir.c:59
> > > > > create_dir lib/kobject.c:73 [inline]
> > > > > kobject_add_internal+0x435/0x8d0 lib/kobject.c:240
> > > > > kobject_add_varg lib/kobject.c:374 [inline]
> > > > > kobject_init_and_add+0x124/0x190 lib/kobject.c:457
> > > > > netdev_queue_add_kobject net/core/net-sysfs.c:1786 [inline]
> > > > > netdev_queue_update_kobjects+0x1ee/0x5f0 net/core/net-sysfs.c:1838
> > > > > register_queue_kobjects net/core/net-sysfs.c:1900 [inline]
> > > > > netdev_register_kobject+0x265/0x320 net/core/net-sysfs.c:2140
> > > > > page last free pid 5103 tgid 5103 stack trace:
> > > > > reset_page_owner include/linux/page_owner.h:24 [inline]
> > > > > free_pages_prepare mm/page_alloc.c:1140 [inline]
> > > > > free_unref_page_prepare+0x968/0xa90 mm/page_alloc.c:2346
> > > > > free_unref_page_list+0x5a3/0x850 mm/page_alloc.c:2532
> > > > > release_pages+0x2744/0x2a80 mm/swap.c:1042
> > > > > tlb_batch_pages_flush mm/mmu_gather.c:98 [inline]
> > > > > tlb_flush_mmu_free mm/mmu_gather.c:293 [inline]
> > > > > tlb_flush_mmu+0x34d/0x4e0 mm/mmu_gather.c:300
> > > > > tlb_finish_mmu+0xd4/0x200 mm/mmu_gather.c:392
> > > > > exit_mmap+0x4b6/0xd40 mm/mmap.c:3300
> > > > > __mmput+0x115/0x3c0 kernel/fork.c:1345
> > > > > exit_mm+0x220/0x310 kernel/exit.c:569
> > > > > do_exit+0x99e/0x27e0 kernel/exit.c:865
> > > > > do_group_exit+0x207/0x2c0 kernel/exit.c:1027
> > > > > __do_sys_exit_group kernel/exit.c:1038 [inline]
> > > > > __se_sys_exit_group kernel/exit.c:1036 [inline]
> > > > > __x64_sys_exit_group+0x3f/0x40 kernel/exit.c:1036
> > > > > do_syscall_64+0xfd/0x240
> > > > > entry_SYSCALL_64_after_hwframe+0x6d/0x75
> > > > >
> > > > > Memory state around the buggy address:
> > > > > ffff88805f7b1480: 05 fc fc fc 05 fc fc fc fa fc fc fc 05 fc fc fc
> > > > > ffff88805f7b1500: 05 fc fc fc 05 fc fc fc 05 fc fc fc 05 fc fc fc
> > > > > >ffff88805f7b1580: 04 fc fc fc 02 fc fc fc fa fc fc fc 05 fc fc fc
> > > > > ^
> > > > > ffff88805f7b1600: fa fc fc fc 05 fc fc fc fa fc fc fc 05 fc fc fc
> > > > > ffff88805f7b1680: 05 fc fc fc 05 fc fc fc 00 fc fc fc fa fc fc fc
> > > > >
> > > > > Fixes: 00398e1d5183 ("Bluetooth: Add support for BT_PKT_STATUS CMSG data for SCO connections")
> > > > > Fixes: b96e9c671b05 ("Bluetooth: Add BT_DEFER_SETUP option to sco socket")
> > > > > Reported-by: syzbot <[email protected]>
> > > > > Signed-off-by: Eric Dumazet <[email protected]>
> > > > > Cc: Marcel Holtmann <[email protected]>
> > > > > Cc: Johan Hedberg <[email protected]>
> > > > > Cc: Luiz Augusto von Dentz <[email protected]> (supporter:BLUETOOTH SUBSYSTEM)
> > > > > Cc: [email protected]
> > > > > ---
> > > > > net/bluetooth/sco.c | 8 ++++++++
> > > > > 1 file changed, 8 insertions(+)
> > > > >
> > > > > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> > > > > index 43daf965a01e4ac5c9329150080b00dcd63c7e1c..9d013f01865fd2509f28eac3bceadf682f0a5edb 100644
> > > > > --- a/net/bluetooth/sco.c
> > > > > +++ b/net/bluetooth/sco.c
> > > > > @@ -843,6 +843,10 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
> > > > > break;
> > > > > }
> > > > >
> > > > > + if (optlen < sizeof(u32)) {
> > > > > + err = -EINVAL;
> > > > > + break;
> > > > > + }
> > > >
> > > > Usually we deal with this sort of problem by doing len =
> > > > min_t(unsigned int, sizeof(u32), optlen) so we truncate the value if
> > > > smaller, perhaps it would be better to have a helper function that
> > > > does len check internally.
> > >
> > > Well, what happens if user provided optlen == 1 ?
> > >
> > > opt would then contain 24 bits of garbage.
> >
> > We might need to do a memset so if userspace just send one byte that
> > would still work, not that are actually doing memset right now so you
> > are right that in that respect it is still broken since it accessing
> > uninitialized value which perhaps is not triggering any build errors
> > but I wonder if we should attempt to fix that as well.
>
> Historically, because of old BSD conventions, IPv4 do_ip_setsockopt()
> has accepted optlen == 1,
> but for instance IPv6 has been strict from day 1.
>
> do_ipv6_setsockopt() does not accept int options in a char
>
> Not sure why you want bluetooh to adopt lazy behavior, given that no
> application could possibly
> have used this so far (without risking a kernel bug)

Fair enough, if we don't really have any risk of breaking the API
(would result in using uninitialized memory) then I propose we do
something like this:

https://gist.github.com/Vudentz/c9092e8a3cb1e7e6a8fd384a51300eee

That said perhaps copy_from_sockptr shall really take into account
both source and destination lengths so it could incorporate the check
e.g. if (dst_size > src_size) but that might result in changing every
user of copy_from_sockptr thus I left it to be specific to bluetooth.

--
Luiz Augusto von Dentz

2024-04-05 16:30:47

by Eric Dumazet

[permalink] [raw]
Subject: Re: [PATCH net] Bluetooth: validate setsockopt( BT_PKT_STATUS / BT_DEFER_SETUP) user input

On Fri, Apr 5, 2024 at 6:24 PM Luiz Augusto von Dentz
<[email protected]> wrote:
>
ave used this so far (without risking a kernel bug)
>
> Fair enough, if we don't really have any risk of breaking the API
> (would result in using uninitialized memory) then I propose we do
> something like this:
>
> https://gist.github.com/Vudentz/c9092e8a3cb1e7e6a8fd384a51300eee
>
> That said perhaps copy_from_sockptr shall really take into account
> both source and destination lengths so it could incorporate the check
> e.g. if (dst_size > src_size) but that might result in changing every
> user of copy_from_sockptr thus I left it to be specific to bluetooth.

Make sure to return -EINVAL if the user provided length is too small,
not -EFAULT.

2024-04-05 17:39:02

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH net] Bluetooth: validate setsockopt( BT_PKT_STATUS / BT_DEFER_SETUP) user input

Hi Eric,

On Fri, Apr 5, 2024 at 12:30 PM Eric Dumazet <[email protected]> wrote:
>
> On Fri, Apr 5, 2024 at 6:24 PM Luiz Augusto von Dentz
> <[email protected]> wrote:
> >
> ave used this so far (without risking a kernel bug)
> >
> > Fair enough, if we don't really have any risk of breaking the API
> > (would result in using uninitialized memory) then I propose we do
> > something like this:
> >
> > https://gist.github.com/Vudentz/c9092e8a3cb1e7e6a8fd384a51300eee
> >
> > That said perhaps copy_from_sockptr shall really take into account
> > both source and destination lengths so it could incorporate the check
> > e.g. if (dst_size > src_size) but that might result in changing every
> > user of copy_from_sockptr thus I left it to be specific to bluetooth.
>
> Make sure to return -EINVAL if the user provided length is too small,
> not -EFAULT.

Sure, there was also a use of -EOVERFLOW and the fact we are using the
return of copy_from_sockptr so if it fails we just return -EFAULT
anyway, so if we do start returning the error from the like
bt_copy_from_sockptr then we better figure out the errors it returns
are proper.

Btw, do you want me to spin a new version containing these changes or
you would like to incorporate them into your patch and spin a v2?

--
Luiz Augusto von Dentz

2024-04-05 17:46:02

by Eric Dumazet

[permalink] [raw]
Subject: Re: [PATCH net] Bluetooth: validate setsockopt( BT_PKT_STATUS / BT_DEFER_SETUP) user input

On Fri, Apr 5, 2024 at 7:38 PM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Eric,
>
> On Fri, Apr 5, 2024 at 12:30 PM Eric Dumazet <[email protected]> wrote:
> >
> > On Fri, Apr 5, 2024 at 6:24 PM Luiz Augusto von Dentz
> > <[email protected]> wrote:
> > >
> > ave used this so far (without risking a kernel bug)
> > >
> > > Fair enough, if we don't really have any risk of breaking the API
> > > (would result in using uninitialized memory) then I propose we do
> > > something like this:
> > >
> > > https://gist.github.com/Vudentz/c9092e8a3cb1e7e6a8fd384a51300eee
> > >
> > > That said perhaps copy_from_sockptr shall really take into account
> > > both source and destination lengths so it could incorporate the check
> > > e.g. if (dst_size > src_size) but that might result in changing every
> > > user of copy_from_sockptr thus I left it to be specific to bluetooth.
> >
> > Make sure to return -EINVAL if the user provided length is too small,
> > not -EFAULT.
>
> Sure, there was also a use of -EOVERFLOW and the fact we are using the
> return of copy_from_sockptr so if it fails we just return -EFAULT
> anyway, so if we do start returning the error from the like
> bt_copy_from_sockptr then we better figure out the errors it returns
> are proper.
>
> Btw, do you want me to spin a new version containing these changes or
> you would like to incorporate them into your patch and spin a v2?

Please go ahead and submit your own patch(es), thanks !