1. Patchew
  2. linux
  3. View series

[PATCH] net/socket: the length value of the input socket option parameter is too small

Edward Adam Davis posted 1 patch 1 year, 10 months ago 
net/socket.c | 3 +++
1 file changed, 3 insertions(+)
[PATCH] net/socket: the length value of the input socket option parameter is too small
Posted by Edward Adam Davis 1 year, 10 months ago 
[Syzbot reported]
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 rfcomm_sock_setsockopt_old net/bluetooth/rfcomm/sock.c:632 [inline]
BUG: KASAN: slab-out-of-bounds in rfcomm_sock_setsockopt+0x893/0xa70 net/bluetooth/rfcomm/sock.c:673
Read of size 4 at addr ffff8880209a8bc3 by task syz-executor632/5064

CPU: 0 PID: 5064 Comm: syz-executor632 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]
 rfcomm_sock_setsockopt_old net/bluetooth/rfcomm/sock.c:632 [inline]
 rfcomm_sock_setsockopt+0x893/0xa70 net/bluetooth/rfcomm/sock.c:673
 do_sock_setsockopt+0x3af/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+0xfb/0x240
 entry_SYSCALL_64_after_hwframe+0x6d/0x75
RIP: 0033:0x7f36ff898dc9
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 91 18 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 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffe010c2208 EFLAGS: 00000246 ORIG_RAX: 0000000000000036
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00007f36ff898dc9
RDX: 0000000000000003 RSI: 0000000000000012 RDI: 0000000000000006
RBP: 0000000000000006 R08: 0000000000000002 R09: 0000000000000000
R10: 00000000200000c0 R11: 0000000000000246 R12: 0000555567399338
R13: 000000000000000e R14: 0000000000000000 R15: 0000000000000000
 </TASK>

Allocated by task 5064:
 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+0xfb/0x240
 entry_SYSCALL_64_after_hwframe+0x6d/0x75

The buggy address belongs to the object at ffff8880209a8bc0
 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 [ffff8880209a8bc0, ffff8880209a8bc2)
[Fix]
The optlen value passed by syzbot to _sys_setsockopt() is 2, which results in
only 2 bytes being allocated when allocating memory to kernel_optval, and the
optval size passed when calling the function copy_from_sockptr() is 4 bytes.
Here, optlen is determined uniformly in the entry function __sys_setsockopt(). 
If its value is less than 4, the parameter is considered invalid.

Reported-by: syzbot+d4ecae01a53fd9b42e7d@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
 net/socket.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/socket.c b/net/socket.c
index e5f3af49a8b6..ac8fd4f6ebfe 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2327,6 +2327,9 @@ int __sys_setsockopt(int fd, int level, int optname, char __user *user_optval,
 	int err, fput_needed;
 	struct socket *sock;
 
+	if (optlen < sizeof(int))
+		return -EINVAL;
+
 	sock = sockfd_lookup_light(fd, &err, &fput_needed);
 	if (!sock)
 		return err;
-- 
2.43.0
Re: [PATCH] net/socket: the length value of the input socket option parameter is too small
Posted by Eric Dumazet 1 year, 10 months ago 
On 4/5/24 12:16, Edward Adam Davis wrote:
> [Syzbot reported]
> 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 rfcomm_sock_setsockopt_old net/bluetooth/rfcomm/sock.c:632 [inline]
> BUG: KASAN: slab-out-of-bounds in rfcomm_sock_setsockopt+0x893/0xa70 net/bluetooth/rfcomm/sock.c:673
> Read of size 4 at addr ffff8880209a8bc3 by task syz-executor632/5064
>
> CPU: 0 PID: 5064 Comm: syz-executor632 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]
>   rfcomm_sock_setsockopt_old net/bluetooth/rfcomm/sock.c:632 [inline]
>   rfcomm_sock_setsockopt+0x893/0xa70 net/bluetooth/rfcomm/sock.c:673
>   do_sock_setsockopt+0x3af/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+0xfb/0x240
>   entry_SYSCALL_64_after_hwframe+0x6d/0x75
> RIP: 0033:0x7f36ff898dc9
> Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 91 18 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 b8 ff ff ff f7 d8 64 89 01 48
> RSP: 002b:00007ffe010c2208 EFLAGS: 00000246 ORIG_RAX: 0000000000000036
> RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00007f36ff898dc9
> RDX: 0000000000000003 RSI: 0000000000000012 RDI: 0000000000000006
> RBP: 0000000000000006 R08: 0000000000000002 R09: 0000000000000000
> R10: 00000000200000c0 R11: 0000000000000246 R12: 0000555567399338
> R13: 000000000000000e R14: 0000000000000000 R15: 0000000000000000
>   </TASK>
>
> Allocated by task 5064:
>   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+0xfb/0x240
>   entry_SYSCALL_64_after_hwframe+0x6d/0x75
>
> The buggy address belongs to the object at ffff8880209a8bc0
>   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 [ffff8880209a8bc0, ffff8880209a8bc2)
> [Fix]
> The optlen value passed by syzbot to _sys_setsockopt() is 2, which results in
> only 2 bytes being allocated when allocating memory to kernel_optval, and the
> optval size passed when calling the function copy_from_sockptr() is 4 bytes.
> Here, optlen is determined uniformly in the entry function __sys_setsockopt().
> If its value is less than 4, the parameter is considered invalid.
>
> Reported-by: syzbot+d4ecae01a53fd9b42e7d@syzkaller.appspotmail.com
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
>   net/socket.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/net/socket.c b/net/socket.c
> index e5f3af49a8b6..ac8fd4f6ebfe 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -2327,6 +2327,9 @@ int __sys_setsockopt(int fd, int level, int optname, char __user *user_optval,
>   	int err, fput_needed;
>   	struct socket *sock;
>   
> +	if (optlen < sizeof(int))
> +		return -EINVAL;
> +


Please cc netdev@ for core networking patches.

This patch is not good, please fix net/bluetooth/rfcomm/sock.c instead

I think I did this yesterday already :

https://lore.kernel.org/netdev/20240404124723.2429464-1-edumazet@google.com/T/


>   	sock = sockfd_lookup_light(fd, &err, &fput_needed);
>   	if (!sock)
>   		return err;
Re: [PATCH] net/socket: the length value of the input socket option parameter is too small
Posted by Paul Menzel 1 year, 10 months ago 
Dear Edward,


Thank you very much for looking into this and sending a patch. Should 
you resent, I’d make the summary about the change and not the issue. Maybe:

net/socket: Ensure length of input socket option param >= sizeof(int)


Kind regards,

Paul
[PATCH] net/socket: Ensure length of input socket option param >= sizeof(int)
Posted by Edward Adam Davis 1 year, 10 months ago 
The optlen value passed by syzbot to _sys_setsockopt() is 2, which results in
only 2 bytes being allocated when allocating memory to kernel_optval, and the
optval size passed when calling the function copy_from_sockptr() is 4 bytes.
Here, optlen is determined uniformly in the entry function __sys_setsockopt(). 
If its value is less than 4, the parameter is considered invalid.

Reported-by: syzbot+837ba09d9db969068367@syzkaller.appspotmail.com
Reported-by: syzbot+b71011ec0a23f4d15625@syzkaller.appspotmail.com
Reported-by: syzbot+d4ecae01a53fd9b42e7d@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
 net/socket.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/socket.c b/net/socket.c
index e5f3af49a8b6..ac8fd4f6ebfe 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2327,6 +2327,9 @@ int __sys_setsockopt(int fd, int level, int optname, char __user *user_optval,
 	int err, fput_needed;
 	struct socket *sock;
 
+	if (optlen < sizeof(int))
+		return -EINVAL;
+
 	sock = sockfd_lookup_light(fd, &err, &fput_needed);
 	if (!sock)
 		return err;
-- 
2.43.0
Re: [PATCH] net/socket: Ensure length of input socket option param >= sizeof(int)
Posted by Eric Dumazet 1 year, 10 months ago 
On 4/9/24 14:15, Edward Adam Davis wrote:
> The optlen value passed by syzbot to _sys_setsockopt() is 2, which results in
> only 2 bytes being allocated when allocating memory to kernel_optval, and the
> optval size passed when calling the function copy_from_sockptr() is 4 bytes.
> Here, optlen is determined uniformly in the entry function __sys_setsockopt().
> If its value is less than 4, the parameter is considered invalid.
>
> Reported-by: syzbot+837ba09d9db969068367@syzkaller.appspotmail.com
> Reported-by: syzbot+b71011ec0a23f4d15625@syzkaller.appspotmail.com
> Reported-by: syzbot+d4ecae01a53fd9b42e7d@syzkaller.appspotmail.com
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>


I think I gave my feedback already.

Please do not ignore maintainers feedback.

This patch is absolutely wrong.

Some setsockopt() deal with optlen == 1 just fine, thank you very much.
Re: [PATCH] net/socket: Ensure length of input socket option param >= sizeof(int)
Posted by Edward Adam Davis 1 year, 10 months ago 
On Tue, 9 Apr 2024 15:07:41 +0200, Eric Dumazet wrote:
> > The optlen value passed by syzbot to _sys_setsockopt() is 2, which results in
> > only 2 bytes being allocated when allocating memory to kernel_optval, and the
> > optval size passed when calling the function copy_from_sockptr() is 4 bytes.
> > Here, optlen is determined uniformly in the entry function __sys_setsockopt().
> > If its value is less than 4, the parameter is considered invalid.
> >
> > Reported-by: syzbot+837ba09d9db969068367@syzkaller.appspotmail.com
> > Reported-by: syzbot+b71011ec0a23f4d15625@syzkaller.appspotmail.com
> > Reported-by: syzbot+d4ecae01a53fd9b42e7d@syzkaller.appspotmail.com
> > Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> 
> 
> I think I gave my feedback already.
> 
> Please do not ignore maintainers feedback.
> 
> This patch is absolutely wrong.
> 
> Some setsockopt() deal with optlen == 1 just fine, thank you very much.
It's better to use evidence to support your claim, rather than your "maintainer" title.
Re: [PATCH] net/socket: Ensure length of input socket option param >= sizeof(int)
Posted by Jakub Kicinski 1 year, 10 months ago 
On Tue,  9 Apr 2024 22:01:45 +0800 Edward Adam Davis wrote:
> > I think I gave my feedback already.
> > 
> > Please do not ignore maintainers feedback.
> > 
> > This patch is absolutely wrong.
> > 
> > Some setsockopt() deal with optlen == 1 just fine, thank you very much.  
>
> It's better to use evidence to support your claim, rather than your "maintainer" title.

Run selftests.
Re: [PATCH] net/socket: Ensure length of input socket option param >= sizeof(int)
Posted by Eric Dumazet 1 year, 10 months ago 
On Tue, Apr 9, 2024 at 4:02 PM Edward Adam Davis <eadavis@qq.com> wrote:
>
> On Tue, 9 Apr 2024 15:07:41 +0200, Eric Dumazet wrote:
> > > The optlen value passed by syzbot to _sys_setsockopt() is 2, which results in
> > > only 2 bytes being allocated when allocating memory to kernel_optval, and the
> > > optval size passed when calling the function copy_from_sockptr() is 4 bytes.
> > > Here, optlen is determined uniformly in the entry function __sys_setsockopt().
> > > If its value is less than 4, the parameter is considered invalid.
> > >
> > > Reported-by: syzbot+837ba09d9db969068367@syzkaller.appspotmail.com
> > > Reported-by: syzbot+b71011ec0a23f4d15625@syzkaller.appspotmail.com
> > > Reported-by: syzbot+d4ecae01a53fd9b42e7d@syzkaller.appspotmail.com
> > > Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> >
> >
> > I think I gave my feedback already.
> >
> > Please do not ignore maintainers feedback.
> >
> > This patch is absolutely wrong.
> >
> > Some setsockopt() deal with optlen == 1 just fine, thank you very much.
> It's better to use evidence to support your claim, rather than your "maintainer" title.

I will answer since you ask so nicely,
but if you plan sending linux kernel patches, I suggest you look in
the source code.

Look at do_ip_setsockopt(), which is one of the most used setsockopt()
in the world.

The code is at least 20 years old.

It even supports optlen == 0

               if (optlen >= sizeof(int)) {
                       if (copy_from_sockptr(&val, optval, sizeof(val)))
                               return -EFAULT;
               } else if (optlen >= sizeof(char)) {
                       unsigned char ucval;

                       if (copy_from_sockptr(&ucval, optval, sizeof(ucval)))
                               return -EFAULT;
                       val = (int) ucval;
               }
       }

       /* If optlen==0, it is equivalent to val == 0 */
Re: [PATCH] net/socket: Ensure length of input socket option param >= sizeof(int)
Posted by Luiz Augusto von Dentz 1 year, 10 months ago 
Hi,

On Tue, Apr 9, 2024 at 9:07 AM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
> On 4/9/24 14:15, Edward Adam Davis wrote:
> > The optlen value passed by syzbot to _sys_setsockopt() is 2, which results in
> > only 2 bytes being allocated when allocating memory to kernel_optval, and the
> > optval size passed when calling the function copy_from_sockptr() is 4 bytes.
> > Here, optlen is determined uniformly in the entry function __sys_setsockopt().
> > If its value is less than 4, the parameter is considered invalid.
> >
> > Reported-by: syzbot+837ba09d9db969068367@syzkaller.appspotmail.com
> > Reported-by: syzbot+b71011ec0a23f4d15625@syzkaller.appspotmail.com
> > Reported-by: syzbot+d4ecae01a53fd9b42e7d@syzkaller.appspotmail.com
> > Signed-off-by: Edward Adam Davis <eadavis@qq.com>
>
>
> I think I gave my feedback already.
>
> Please do not ignore maintainers feedback.
>
> This patch is absolutely wrong.
>
> Some setsockopt() deal with optlen == 1 just fine, thank you very much.

+1, I don't think the setsockopt interface has a fixed minimum of
sizeof(int), so this is a nak from me as well.

-- 
Luiz Augusto von Dentz
[PATCH] Bluetooth: fix oob in rfcomm_sock_setsockopt
Posted by Edward Adam Davis 1 year, 10 months ago 
If optlen < sizeof(u32) it will trigger oob, so take the min of them.

Reported-by: syzbot+d4ecae01a53fd9b42e7d@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
 net/bluetooth/rfcomm/sock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index b54e8a530f55..42c55c756b51 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -629,7 +629,7 @@ static int rfcomm_sock_setsockopt_old(struct socket *sock, int optname,
 
 	switch (optname) {
 	case RFCOMM_LM:
-		if (copy_from_sockptr(&opt, optval, sizeof(u32))) {
+		if (copy_from_sockptr(&opt, optval, min_t(int, sizeof(u32), optlen))) {
 			err = -EFAULT;
 			break;
 		}
-- 
2.43.0
Re: [PATCH] Bluetooth: fix oob in rfcomm_sock_setsockopt
Posted by Luiz Augusto von Dentz 1 year, 10 months ago 
Hi Edward,

On Tue, Apr 9, 2024 at 9:36 AM Edward Adam Davis <eadavis@qq.com> wrote:
>
> If optlen < sizeof(u32) it will trigger oob, so take the min of them.
>
> Reported-by: syzbot+d4ecae01a53fd9b42e7d@syzkaller.appspotmail.com
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
>  net/bluetooth/rfcomm/sock.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
> index b54e8a530f55..42c55c756b51 100644
> --- a/net/bluetooth/rfcomm/sock.c
> +++ b/net/bluetooth/rfcomm/sock.c
> @@ -629,7 +629,7 @@ static int rfcomm_sock_setsockopt_old(struct socket *sock, int optname,
>
>         switch (optname) {
>         case RFCOMM_LM:
> -               if (copy_from_sockptr(&opt, optval, sizeof(u32))) {
> +               if (copy_from_sockptr(&opt, optval, min_t(int, sizeof(u32), optlen))) {
>                         err = -EFAULT;
>                         break;
>                 }
> --
> 2.43.0

This has been dealt with already:

https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=ee77912bc0bbd78fceb785a81cc9108fa954982f


-- 
Luiz Augusto von Dentz

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.