diff mbox series

[net] Bluetooth: validate setsockopt( BT_PKT_STATUS / BT_DEFER_SETUP) user input

Message ID 20240404123602.2369488-1-edumazet@google.com (mailing list archive)
State New, archived
Headers show
Series [net] Bluetooth: validate setsockopt( BT_PKT_STATUS / BT_DEFER_SETUP) user input | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch warning 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 <syzkaller@googlegroups.com> Signed-off-by: Eric Dumazet <edumazet@google.com> 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.
tedd_an/GitLint fail 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 <luiz.dentz@gmail.com> (supporter:BLUETOOTH SUBSYSTEM)"
tedd_an/SubjectPrefix success Gitlint PASS
tedd_an/BuildKernel success BuildKernel PASS
tedd_an/CheckAllWarning success CheckAllWarning PASS
tedd_an/CheckSparse warning CheckSparse WARNING net/bluetooth/sco.c: note: in included file:./include/net/bluetooth/hci_core.h:150:35: warning: array of flexible structures
tedd_an/CheckSmatch fail CheckSparse: FAIL: 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
tedd_an/BuildKernel32 success BuildKernel32 PASS
tedd_an/TestRunnerSetup success TestRunnerSetup PASS
tedd_an/TestRunner_l2cap-tester success TestRunner PASS
tedd_an/TestRunner_iso-tester success TestRunner PASS
tedd_an/TestRunner_bnep-tester success TestRunner PASS
tedd_an/TestRunner_mgmt-tester success TestRunner PASS
tedd_an/TestRunner_rfcomm-tester success TestRunner PASS
tedd_an/TestRunner_sco-tester success TestRunner PASS
tedd_an/TestRunner_ioctl-tester success TestRunner PASS
tedd_an/TestRunner_mesh-tester success TestRunner PASS
tedd_an/TestRunner_smp-tester success TestRunner PASS
tedd_an/TestRunner_userchan-tester success TestRunner PASS
tedd_an/IncrementalBuild success Incremental Build PASS

Commit Message

Eric Dumazet April 4, 2024, 12:36 p.m. UTC
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 <syzkaller@googlegroups.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Cc: Luiz Augusto von Dentz <luiz.dentz@gmail.com> (supporter:BLUETOOTH SUBSYSTEM)
Cc: linux-bluetooth@vger.kernel.org
---
 net/bluetooth/sco.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

bluez.test.bot@gmail.com April 4, 2024, 1:07 p.m. UTC | #1
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 <syzkaller@googlegroups.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>

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 <luiz.dentz@gmail.com> (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
Luiz Augusto von Dentz April 4, 2024, 6:31 p.m. UTC | #2
Hi Eric,

On Thu, Apr 4, 2024 at 8:36 AM Eric Dumazet <edumazet@google.com> 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 <syzkaller@googlegroups.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Marcel Holtmann <marcel@holtmann.org>
> Cc: Johan Hedberg <johan.hedberg@gmail.com>
> Cc: Luiz Augusto von Dentz <luiz.dentz@gmail.com> (supporter:BLUETOOTH SUBSYSTEM)
> Cc: linux-bluetooth@vger.kernel.org
> ---
>  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
>
Eric Dumazet April 4, 2024, 7:04 p.m. UTC | #3
On Thu, Apr 4, 2024 at 8:31 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Eric,
>
> On Thu, Apr 4, 2024 at 8:36 AM Eric Dumazet <edumazet@google.com> 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 <syzkaller@googlegroups.com>
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Cc: Marcel Holtmann <marcel@holtmann.org>
> > Cc: Johan Hedberg <johan.hedberg@gmail.com>
> > Cc: Luiz Augusto von Dentz <luiz.dentz@gmail.com> (supporter:BLUETOOTH SUBSYSTEM)
> > Cc: linux-bluetooth@vger.kernel.org
> > ---
> >  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.
Luiz Augusto von Dentz April 4, 2024, 9:07 p.m. UTC | #4
Hi Eric,

On Thu, Apr 4, 2024 at 3:04 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Thu, Apr 4, 2024 at 8:31 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Eric,
> >
> > On Thu, Apr 4, 2024 at 8:36 AM Eric Dumazet <edumazet@google.com> 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 <syzkaller@googlegroups.com>
> > > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > > Cc: Marcel Holtmann <marcel@holtmann.org>
> > > Cc: Johan Hedberg <johan.hedberg@gmail.com>
> > > Cc: Luiz Augusto von Dentz <luiz.dentz@gmail.com> (supporter:BLUETOOTH SUBSYSTEM)
> > > Cc: linux-bluetooth@vger.kernel.org
> > > ---
> > >  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.
Eric Dumazet April 5, 2024, 10:18 a.m. UTC | #5
On Thu, Apr 4, 2024 at 11:07 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Eric,
>
> On Thu, Apr 4, 2024 at 3:04 PM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Thu, Apr 4, 2024 at 8:31 PM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > Hi Eric,
> > >
> > > On Thu, Apr 4, 2024 at 8:36 AM Eric Dumazet <edumazet@google.com> 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 <syzkaller@googlegroups.com>
> > > > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > > > Cc: Marcel Holtmann <marcel@holtmann.org>
> > > > Cc: Johan Hedberg <johan.hedberg@gmail.com>
> > > > Cc: Luiz Augusto von Dentz <luiz.dentz@gmail.com> (supporter:BLUETOOTH SUBSYSTEM)
> > > > Cc: linux-bluetooth@vger.kernel.org
> > > > ---
> > > >  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)
Luiz Augusto von Dentz April 5, 2024, 4:24 p.m. UTC | #6
Hi Eric,

On Fri, Apr 5, 2024 at 6:19 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Thu, Apr 4, 2024 at 11:07 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Eric,
> >
> > On Thu, Apr 4, 2024 at 3:04 PM Eric Dumazet <edumazet@google.com> wrote:
> > >
> > > On Thu, Apr 4, 2024 at 8:31 PM Luiz Augusto von Dentz
> > > <luiz.dentz@gmail.com> wrote:
> > > >
> > > > Hi Eric,
> > > >
> > > > On Thu, Apr 4, 2024 at 8:36 AM Eric Dumazet <edumazet@google.com> 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 <syzkaller@googlegroups.com>
> > > > > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > > > > Cc: Marcel Holtmann <marcel@holtmann.org>
> > > > > Cc: Johan Hedberg <johan.hedberg@gmail.com>
> > > > > Cc: Luiz Augusto von Dentz <luiz.dentz@gmail.com> (supporter:BLUETOOTH SUBSYSTEM)
> > > > > Cc: linux-bluetooth@vger.kernel.org
> > > > > ---
> > > > >  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.
Eric Dumazet April 5, 2024, 4:30 p.m. UTC | #7
On Fri, Apr 5, 2024 at 6:24 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> 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.
Luiz Augusto von Dentz April 5, 2024, 5:38 p.m. UTC | #8
Hi Eric,

On Fri, Apr 5, 2024 at 12:30 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Fri, Apr 5, 2024 at 6:24 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> 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?
Eric Dumazet April 5, 2024, 5:45 p.m. UTC | #9
On Fri, Apr 5, 2024 at 7:38 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Eric,
>
> On Fri, Apr 5, 2024 at 12:30 PM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Fri, Apr 5, 2024 at 6:24 PM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> 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 !
diff mbox series

Patch

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;