Commit 9bf4e919ccad worked around an issue introduced after an innocuous optimisation change in LLVM main:
The same issue occurs in rfcomm in functions rfcomm_sock_getsockopt and rfcomm_sock_getsockopt_old.
Change the type of len to size_t in both rfcomm_sock_getsockopt and rfcomm_sock_getsockopt_old and replace min_t() with min().
Cc: stable@vger.kernel.org Co-authored-by: Aleksei Vetrov vvvvvv@google.com Improves: 9bf4e919ccad ("Bluetooth: Fix type of len in {l2cap,sco}_sock_getsockopt_old()") Link: https://github.com/ClangBuiltLinux/linux/issues/2007 Link: https://github.com/llvm/llvm-project/issues/85647 Signed-off-by: Andrej Shadura andrew.shadura@collabora.co.uk Reviewed-by: Nathan Chancellor nathan@kernel.org --- net/bluetooth/rfcomm/sock.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c index 37d63d768afb..5f9d370e09b1 100644 --- a/net/bluetooth/rfcomm/sock.c +++ b/net/bluetooth/rfcomm/sock.c @@ -729,7 +729,8 @@ static int rfcomm_sock_getsockopt_old(struct socket *sock, int optname, char __u struct sock *l2cap_sk; struct l2cap_conn *conn; struct rfcomm_conninfo cinfo; - int len, err = 0; + int err = 0; + size_t len; u32 opt;
BT_DBG("sk %p", sk); @@ -783,7 +784,7 @@ static int rfcomm_sock_getsockopt_old(struct socket *sock, int optname, char __u cinfo.hci_handle = conn->hcon->handle; memcpy(cinfo.dev_class, conn->hcon->dev_class, 3);
- len = min_t(unsigned int, len, sizeof(cinfo)); + len = min(len, sizeof(cinfo)); if (copy_to_user(optval, (char *) &cinfo, len)) err = -EFAULT;
@@ -802,7 +803,8 @@ static int rfcomm_sock_getsockopt(struct socket *sock, int level, int optname, c { struct sock *sk = sock->sk; struct bt_security sec; - int len, err = 0; + int err = 0; + size_t len;
BT_DBG("sk %p", sk);
@@ -827,7 +829,7 @@ static int rfcomm_sock_getsockopt(struct socket *sock, int level, int optname, c sec.level = rfcomm_pi(sk)->sec_level; sec.key_size = 0;
- len = min_t(unsigned int, len, sizeof(sec)); + len = min(len, sizeof(sec)); if (copy_to_user(optval, (char *) &sec, len)) err = -EFAULT;
From: Andrej Shadura
That can't matter because the value is a small positive integer.
Isn't there still a problem if the application passed a negative length. You are converting it to a very large unsigned value and then reducing it to the structure size. Since the structure size will be less than 2GB it makes no difference whether the '__user int optlen' is ever converted to 64bits. I think you are just hiding a bug in a different way.
Note that pretty much all the checks for 'optlen' have treated negative values as 4 since well before the min() and min_t() #defines were added. Look at the tcp code!
I bet that globally fixing the test will cause some important application that is passing 'on stack garbage' to fail.
David
- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Hi Andrej,
On Thu, Oct 17, 2024 at 9:47 AM Andrej Shadura andrew.shadura@collabora.co.uk wrote:
I was waiting to see if David had any more feedback, but if he doesn't I'm happy to merge this later today.
Hello:
This patch was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz luiz.von.dentz@intel.com:
On Wed, 9 Oct 2024 14:14:24 +0200 you wrote:
Here is the summary with links: - [v2] Bluetooth: Fix type of len in rfcomm_sock_getsockopt{,_old}() https://git.kernel.org/bluetooth/bluetooth-next/c/c440001ad70d
You are awesome, thank you!
linux-stable-mirror@lists.linaro.org