[PATCH v5.10-v5.15] Bluetooth: RFCOMM: Fix not validating setsockopt user input

Keerthana K posted 1 patch 1 year ago
There is a newer version of this series
net/bluetooth/rfcomm/sock.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
[PATCH v5.10-v5.15] Bluetooth: RFCOMM: Fix not validating setsockopt user input
Posted by Keerthana K 1 year ago
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

[ Upstream commit a97de7bff13b1cc825c1b1344eaed8d6c2d3e695 ]

syzbot reported rfcomm_sock_setsockopt_old() 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 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

Fixes: 9f2c8a03fbb3 ("Bluetooth: Replace RFCOMM link mode with security level")
Fixes: bb23c0ab8246 ("Bluetooth: Add support for deferring RFCOMM connection setup")
Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Keerthana K <keerthana.kalyanasundaram@broadcom.com>
---
 net/bluetooth/rfcomm/sock.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 1db441db4..2dcb70f49 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -631,7 +631,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 (bt_copy_from_sockptr(&opt, sizeof(opt), optval, optlen)) {
 			err = -EFAULT;
 			break;
 		}
@@ -666,7 +666,6 @@ static int rfcomm_sock_setsockopt(struct socket *sock, int level, int optname,
 	struct sock *sk = sock->sk;
 	struct bt_security sec;
 	int err = 0;
-	size_t len;
 	u32 opt;
 
 	BT_DBG("sk %p", sk);
@@ -688,11 +687,9 @@ static int rfcomm_sock_setsockopt(struct socket *sock, int level, int optname,
 
 		sec.level = BT_SECURITY_LOW;
 
-		len = min_t(unsigned int, sizeof(sec), optlen);
-		if (copy_from_sockptr(&sec, optval, len)) {
-			err = -EFAULT;
+		err = bt_copy_from_sockptr(&sec, sizeof(sec), optval, optlen);
+		if (err)
 			break;
-		}
 
 		if (sec.level > BT_SECURITY_HIGH) {
 			err = -EINVAL;
@@ -708,10 +705,9 @@ static int rfcomm_sock_setsockopt(struct socket *sock, int level, int optname,
 			break;
 		}
 
-		if (copy_from_sockptr(&opt, optval, sizeof(u32))) {
-			err = -EFAULT;
+		err = bt_copy_from_sockptr(&opt, sizeof(opt), optval, optlen);
+		if (err)
 			break;
-		}
 
 		if (opt)
 			set_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
-- 
2.39.4
Re: [PATCH v5.10-v5.15] Bluetooth: RFCOMM: Fix not validating setsockopt user input
Posted by Greg KH 1 year ago
On Mon, Jan 20, 2025 at 06:46:47AM +0000, Keerthana K wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> [ Upstream commit a97de7bff13b1cc825c1b1344eaed8d6c2d3e695 ]
> 
> syzbot reported rfcomm_sock_setsockopt_old() 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 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
> 
> Fixes: 9f2c8a03fbb3 ("Bluetooth: Replace RFCOMM link mode with security level")
> Fixes: bb23c0ab8246 ("Bluetooth: Add support for deferring RFCOMM connection setup")
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> Signed-off-by: Keerthana K <keerthana.kalyanasundaram@broadcom.com>
> ---
>  net/bluetooth/rfcomm/sock.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)

This breaks the build on 5.15.y systems, did you test it?

I'm dropping both patches now, please be more careful.

greg k-h
Re: [PATCH v5.10-v5.15] Bluetooth: RFCOMM: Fix not validating setsockopt user input
Posted by Keerthana Kalyanasundaram 1 year ago
On Mon, Jan 20, 2025 at 9:11 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, Jan 20, 2025 at 06:46:47AM +0000, Keerthana K wrote:
> > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >
> > [ Upstream commit a97de7bff13b1cc825c1b1344eaed8d6c2d3e695 ]
> >
> > syzbot reported rfcomm_sock_setsockopt_old() 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 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
> >
> > Fixes: 9f2c8a03fbb3 ("Bluetooth: Replace RFCOMM link mode with security level")
> > Fixes: bb23c0ab8246 ("Bluetooth: Add support for deferring RFCOMM connection setup")
> > Reported-by: syzbot <syzkaller@googlegroups.com>
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > Signed-off-by: Sasha Levin <sashal@kernel.org>
> > Signed-off-by: Keerthana K <keerthana.kalyanasundaram@broadcom.com>
> > ---
> >  net/bluetooth/rfcomm/sock.c | 14 +++++---------
> >  1 file changed, 5 insertions(+), 9 deletions(-)
>
> This breaks the build on 5.15.y systems, did you test it?
>
> I'm dropping both patches now, please be more careful.
>
Apologies for the build breakage. I will be more careful in the future.
v5.15.y:
one patch is missing in v5.15.y. I have added that patch
https://lore.kernel.org/stable/20250124053306.5028-1-keerthana.kalyanasundaram@broadcom.com/T/#t
v5.10.y:
No changes needed. you can pick the same patch from the email chain for v5.10.y

- Keerthana K
Re: [PATCH v5.10-v5.15] Bluetooth: RFCOMM: Fix not validating setsockopt user input
Posted by Greg KH 1 year ago
On Fri, Jan 24, 2025 at 11:13:53AM +0530, Keerthana Kalyanasundaram wrote:
> On Mon, Jan 20, 2025 at 9:11 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Mon, Jan 20, 2025 at 06:46:47AM +0000, Keerthana K wrote:
> > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > >
> > > [ Upstream commit a97de7bff13b1cc825c1b1344eaed8d6c2d3e695 ]
> > >
> > > syzbot reported rfcomm_sock_setsockopt_old() 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 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
> > >
> > > Fixes: 9f2c8a03fbb3 ("Bluetooth: Replace RFCOMM link mode with security level")
> > > Fixes: bb23c0ab8246 ("Bluetooth: Add support for deferring RFCOMM connection setup")
> > > Reported-by: syzbot <syzkaller@googlegroups.com>
> > > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > Signed-off-by: Sasha Levin <sashal@kernel.org>
> > > Signed-off-by: Keerthana K <keerthana.kalyanasundaram@broadcom.com>
> > > ---
> > >  net/bluetooth/rfcomm/sock.c | 14 +++++---------
> > >  1 file changed, 5 insertions(+), 9 deletions(-)
> >
> > This breaks the build on 5.15.y systems, did you test it?
> >
> > I'm dropping both patches now, please be more careful.
> >
> Apologies for the build breakage. I will be more careful in the future.
> v5.15.y:
> one patch is missing in v5.15.y. I have added that patch
> https://lore.kernel.org/stable/20250124053306.5028-1-keerthana.kalyanasundaram@broadcom.com/T/#t
> v5.10.y:
> No changes needed. you can pick the same patch from the email chain for v5.10.y

From what "email chain"?  Please just send a v5.10.y patch as well to
make it obvious what we are supposed to do here.

confused,

greg k-h