From: Andrej Shadura

Sent: 09 October 2024 13:14

Commit 9bf4e919ccad worked around an issue introduced after an innocuous optimisation change in LLVM main:

...

len is defined as an 'int' because it is assigned from '__user int *optlen'. However, it is clamped against the result of sizeof(), which has a type of 'size_t' ('unsigned long' for 64-bit platforms). This is done with min_t() because min() requires compatible types, which results in both len and the result of sizeof() being casted to 'unsigned int', meaning len changes signs and the result of sizeof() is truncated. From there, len is passed to copy_to_user(), which has a third parameter type of 'unsigned long', so it is widened and changes signs again.

That can't matter because the value is a small positive integer.

This excessive casting in combination with the KCSAN

...

instrumentation causes LLVM to fail to eliminate the __bad_copy_from() call, failing the build.

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().

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

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;

-- 2.43.0

- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)