* Re: [PATCH] Bluetooth: Fix type of len in rfcomm_sock_{bind,getsockopt_old}()
2024-10-02 14:12 [PATCH] Bluetooth: Fix type of len in rfcomm_sock_{bind,getsockopt_old}() Andrej Shadura
@ 2024-10-02 15:22 ` Nathan Chancellor
2024-10-02 16:29 ` Andrej Shadura
2024-10-04 8:41 ` kernel test robot
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Nathan Chancellor @ 2024-10-02 15:22 UTC (permalink / raw)
To: Andrej Shadura
Cc: linux-bluetooth, Justin Stitt, llvm, kernel, George Burgess,
stable
Hi Andrej,
On Wed, Oct 02, 2024 at 04:12:17PM +0200, Andrej Shadura wrote:
> 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. 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_bind and
> rfcomm_sock_getsockopt_old.
Was this found by inspection or is there an actual instance of the same
warning? For what it's worth, I haven't seen a warning from this file in
any of my build tests.
> Change the type of len to size_t in both rfcomm_sock_bind and
> rfcomm_sock_getsockopt_old and replace min_t() with min().
>
> Cc: stable@vger.kernel.org
> Fixes: 9bf4e919ccad ("Bluetooth: Fix type of len in {l2cap,sco}_sock_getsockopt_old()")
I am not sure that I totally agree with this Fixes tag since I did not
have a warning to fix but I guess it makes sense to help with
backporting.
> 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..c0fe96673b3c 100644
> --- a/net/bluetooth/rfcomm/sock.c
> +++ b/net/bluetooth/rfcomm/sock.c
> @@ -328,14 +328,15 @@ static int rfcomm_sock_bind(struct socket *sock, struct sockaddr *addr, int addr
> {
> struct sockaddr_rc sa;
> struct sock *sk = sock->sk;
> - int len, err = 0;
> + int err = 0;
> + size_t len;
>
> if (!addr || addr_len < offsetofend(struct sockaddr, sa_family) ||
> addr->sa_family != AF_BLUETOOTH)
> return -EINVAL;
>
> memset(&sa, 0, sizeof(sa));
> - len = min_t(unsigned int, sizeof(sa), addr_len);
> + len = min(sizeof(sa), addr_len);
> memcpy(&sa, addr, len);
>
> BT_DBG("sk %p %pMR", sk, &sa.rc_bdaddr);
> @@ -729,7 +730,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 +785,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;
>
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Bluetooth: Fix type of len in rfcomm_sock_{bind,getsockopt_old}()
2024-10-02 15:22 ` Nathan Chancellor
@ 2024-10-02 16:29 ` Andrej Shadura
2024-10-02 16:45 ` Nathan Chancellor
0 siblings, 1 reply; 8+ messages in thread
From: Andrej Shadura @ 2024-10-02 16:29 UTC (permalink / raw)
To: Nathan Chancellor
Cc: linux-bluetooth, Justin Stitt, llvm, kernel, George Burgess,
stable
Hi Nathan,
On 02/10/2024 17:22, Nathan Chancellor wrote:
> Hi Andrej,
>
> On Wed, Oct 02, 2024 at 04:12:17PM +0200, Andrej Shadura wrote:
>> 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. 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_bind and
>> rfcomm_sock_getsockopt_old.
> Was this found by inspection or is there an actual instance of the same
> warning? For what it's worth, I haven't seen a warning from this file in
> any of my build tests.
We’ve seen build errors in rfcomm in the Chromium OS 4.14 kernel.
>> Change the type of len to size_t in both rfcomm_sock_bind and
>> rfcomm_sock_getsockopt_old and replace min_t() with min().
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 9bf4e919ccad ("Bluetooth: Fix type of len in {l2cap,sco}_sock_getsockopt_old()")
> I am not sure that I totally agree with this Fixes tag since I did not
> have a warning to fix but I guess it makes sense to help with
> backporting.
It’s a shame there isn’t a Complements: or Improves: tag :)
--
Cheers,
Andrej
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Bluetooth: Fix type of len in rfcomm_sock_{bind,getsockopt_old}()
2024-10-02 16:29 ` Andrej Shadura
@ 2024-10-02 16:45 ` Nathan Chancellor
0 siblings, 0 replies; 8+ messages in thread
From: Nathan Chancellor @ 2024-10-02 16:45 UTC (permalink / raw)
To: Andrej Shadura
Cc: linux-bluetooth, Justin Stitt, llvm, kernel, George Burgess,
stable
On Wed, Oct 02, 2024 at 06:29:22PM +0200, Andrej Shadura wrote:
> > On Wed, Oct 02, 2024 at 04:12:17PM +0200, Andrej Shadura wrote:
> >> 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. 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_bind and
> >> rfcomm_sock_getsockopt_old.
>
> > Was this found by inspection or is there an actual instance of the same
> > warning? For what it's worth, I haven't seen a warning from this file in
> > any of my build tests.
>
> We’ve seen build errors in rfcomm in the Chromium OS 4.14 kernel.
If there is any reason to send a v2 (since I don't think my nit below
really warrants one at the moment), you may consider adding this
information and the warning text that you see to the commit message for
future travelers. I definitely have to go back to look at prior commits
for understanding on current issues and I appreciate having easy things
to look for when searching.
Is the warning reproducible on mainline with the same configuration?
Just curious, I still think it is worth sending this through mainline
even if there is no warning currently, since we cannot say that an
potential future LLVM change could not make this show up there if not
properly fixed. I am just wondering what my test coverage is missing :)
> >> Change the type of len to size_t in both rfcomm_sock_bind and
> >> rfcomm_sock_getsockopt_old and replace min_t() with min().
> >>
> >> Cc: stable@vger.kernel.org
> >> Fixes: 9bf4e919ccad ("Bluetooth: Fix type of len in {l2cap,sco}_sock_getsockopt_old()")
>
> > I am not sure that I totally agree with this Fixes tag since I did not
> > have a warning to fix but I guess it makes sense to help with
> > backporting.
>
> It’s a shame there isn’t a Complements: or Improves: tag :)
Heh, who is to say we can't be the first? :)
There is some prior art with References: but eh, like I said above, I do
not think it truly matters, since it should make it easier for the
stable folks to apply it. That comment was more directed at how
overloaded Fixes: has become to me that maybe it is worth considering
expanding the type of tags for referring to previous commits but I am
sure there would be push back in some form... No worries regardless!
Cheers,
Nathan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Bluetooth: Fix type of len in rfcomm_sock_{bind,getsockopt_old}()
2024-10-02 14:12 [PATCH] Bluetooth: Fix type of len in rfcomm_sock_{bind,getsockopt_old}() Andrej Shadura
2024-10-02 15:22 ` Nathan Chancellor
@ 2024-10-04 8:41 ` kernel test robot
2024-10-04 15:13 ` kernel test robot
2024-10-07 19:49 ` Aleksei Vetrov
3 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2024-10-04 8:41 UTC (permalink / raw)
To: Andrej Shadura, linux-bluetooth
Cc: oe-kbuild-all, Nathan Chancellor, Justin Stitt, llvm, kernel,
George Burgess, stable
Hi Andrej,
kernel test robot noticed the following build errors:
[auto build test ERROR on bluetooth-next/master]
[also build test ERROR on bluetooth/master linus/master v6.12-rc1 next-20241004]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Andrej-Shadura/Bluetooth-Fix-type-of-len-in-rfcomm_sock_-bind-getsockopt_old/20241002-221656
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
patch link: https://lore.kernel.org/r/20241002141217.663070-1-andrew.shadura%40collabora.co.uk
patch subject: [PATCH] Bluetooth: Fix type of len in rfcomm_sock_{bind,getsockopt_old}()
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20241004/202410041637.iOIxEAQQ-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241004/202410041637.iOIxEAQQ-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410041637.iOIxEAQQ-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from <command-line>:
net/bluetooth/rfcomm/sock.c: In function 'rfcomm_sock_bind':
>> include/linux/compiler_types.h:510:45: error: call to '__compiletime_assert_602' declared with attribute error: min(sizeof(sa), addr_len) signedness error
510 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^
include/linux/compiler_types.h:491:25: note: in definition of macro '__compiletime_assert'
491 | prefix ## suffix(); \
| ^~~~~~
include/linux/compiler_types.h:510:9: note: in expansion of macro '_compiletime_assert'
510 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
| ^~~~~~~~~~~~~~~~~~
include/linux/minmax.h:100:9: note: in expansion of macro 'BUILD_BUG_ON_MSG'
100 | BUILD_BUG_ON_MSG(!__types_ok(x,y,ux,uy), \
| ^~~~~~~~~~~~~~~~
include/linux/minmax.h:105:9: note: in expansion of macro '__careful_cmp_once'
105 | __careful_cmp_once(op, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
| ^~~~~~~~~~~~~~~~~~
include/linux/minmax.h:129:25: note: in expansion of macro '__careful_cmp'
129 | #define min(x, y) __careful_cmp(min, x, y)
| ^~~~~~~~~~~~~
net/bluetooth/rfcomm/sock.c:339:15: note: in expansion of macro 'min'
339 | len = min(sizeof(sa), addr_len);
| ^~~
vim +/__compiletime_assert_602 +510 include/linux/compiler_types.h
eb5c2d4b45e3d2 Will Deacon 2020-07-21 496
eb5c2d4b45e3d2 Will Deacon 2020-07-21 497 #define _compiletime_assert(condition, msg, prefix, suffix) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 498 __compiletime_assert(condition, msg, prefix, suffix)
eb5c2d4b45e3d2 Will Deacon 2020-07-21 499
eb5c2d4b45e3d2 Will Deacon 2020-07-21 500 /**
eb5c2d4b45e3d2 Will Deacon 2020-07-21 501 * compiletime_assert - break build and emit msg if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21 502 * @condition: a compile-time constant condition to check
eb5c2d4b45e3d2 Will Deacon 2020-07-21 503 * @msg: a message to emit if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21 504 *
eb5c2d4b45e3d2 Will Deacon 2020-07-21 505 * In tradition of POSIX assert, this macro will break the build if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21 506 * supplied condition is *false*, emitting the supplied error message if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21 507 * compiler has support to do so.
eb5c2d4b45e3d2 Will Deacon 2020-07-21 508 */
eb5c2d4b45e3d2 Will Deacon 2020-07-21 509 #define compiletime_assert(condition, msg) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 @510 _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
eb5c2d4b45e3d2 Will Deacon 2020-07-21 511
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Bluetooth: Fix type of len in rfcomm_sock_{bind,getsockopt_old}()
2024-10-02 14:12 [PATCH] Bluetooth: Fix type of len in rfcomm_sock_{bind,getsockopt_old}() Andrej Shadura
2024-10-02 15:22 ` Nathan Chancellor
2024-10-04 8:41 ` kernel test robot
@ 2024-10-04 15:13 ` kernel test robot
2024-10-07 19:49 ` Aleksei Vetrov
3 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2024-10-04 15:13 UTC (permalink / raw)
To: Andrej Shadura, linux-bluetooth
Cc: llvm, oe-kbuild-all, Nathan Chancellor, Justin Stitt, kernel,
George Burgess, stable
Hi Andrej,
kernel test robot noticed the following build errors:
[auto build test ERROR on bluetooth-next/master]
[also build test ERROR on bluetooth/master linus/master v6.12-rc1 next-20241004]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Andrej-Shadura/Bluetooth-Fix-type-of-len-in-rfcomm_sock_-bind-getsockopt_old/20241002-221656
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
patch link: https://lore.kernel.org/r/20241002141217.663070-1-andrew.shadura%40collabora.co.uk
patch subject: [PATCH] Bluetooth: Fix type of len in rfcomm_sock_{bind,getsockopt_old}()
config: arm-randconfig-001-20241004 (https://download.01.org/0day-ci/archive/20241004/202410042221.Phncg973-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project fef3566a25ff0e34fb87339ba5e13eca17cec00f)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241004/202410042221.Phncg973-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410042221.Phncg973-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from net/bluetooth/rfcomm/sock.c:32:
In file included from include/net/bluetooth/bluetooth.h:30:
In file included from include/net/sock.h:46:
In file included from include/linux/netdevice.h:38:
In file included from include/net/net_namespace.h:43:
In file included from include/linux/skbuff.h:17:
In file included from include/linux/bvec.h:10:
In file included from include/linux/highmem.h:8:
In file included from include/linux/cacheflush.h:5:
In file included from arch/arm/include/asm/cacheflush.h:10:
In file included from include/linux/mm.h:2232:
include/linux/vmstat.h:517:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
517 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
>> net/bluetooth/rfcomm/sock.c:339:8: error: call to '__compiletime_assert_549' declared with 'error' attribute: min(sizeof(sa), addr_len) signedness error
339 | len = min(sizeof(sa), addr_len);
| ^
include/linux/minmax.h:129:19: note: expanded from macro 'min'
129 | #define min(x, y) __careful_cmp(min, x, y)
| ^
include/linux/minmax.h:105:2: note: expanded from macro '__careful_cmp'
105 | __careful_cmp_once(op, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
| ^
include/linux/minmax.h:100:2: note: expanded from macro '__careful_cmp_once'
100 | BUILD_BUG_ON_MSG(!__types_ok(x,y,ux,uy), \
| ^
note: (skipping 2 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
include/linux/compiler_types.h:498:2: note: expanded from macro '_compiletime_assert'
498 | __compiletime_assert(condition, msg, prefix, suffix)
| ^
include/linux/compiler_types.h:491:4: note: expanded from macro '__compiletime_assert'
491 | prefix ## suffix(); \
| ^
<scratch space>:174:1: note: expanded from here
174 | __compiletime_assert_549
| ^
1 warning and 1 error generated.
vim +339 net/bluetooth/rfcomm/sock.c
326
327 static int rfcomm_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_len)
328 {
329 struct sockaddr_rc sa;
330 struct sock *sk = sock->sk;
331 int err = 0;
332 size_t len;
333
334 if (!addr || addr_len < offsetofend(struct sockaddr, sa_family) ||
335 addr->sa_family != AF_BLUETOOTH)
336 return -EINVAL;
337
338 memset(&sa, 0, sizeof(sa));
> 339 len = min(sizeof(sa), addr_len);
340 memcpy(&sa, addr, len);
341
342 BT_DBG("sk %p %pMR", sk, &sa.rc_bdaddr);
343
344 lock_sock(sk);
345
346 if (sk->sk_state != BT_OPEN) {
347 err = -EBADFD;
348 goto done;
349 }
350
351 if (sk->sk_type != SOCK_STREAM) {
352 err = -EINVAL;
353 goto done;
354 }
355
356 write_lock(&rfcomm_sk_list.lock);
357
358 if (sa.rc_channel &&
359 __rfcomm_get_listen_sock_by_addr(sa.rc_channel, &sa.rc_bdaddr)) {
360 err = -EADDRINUSE;
361 } else {
362 /* Save source address */
363 bacpy(&rfcomm_pi(sk)->src, &sa.rc_bdaddr);
364 rfcomm_pi(sk)->channel = sa.rc_channel;
365 sk->sk_state = BT_BOUND;
366 }
367
368 write_unlock(&rfcomm_sk_list.lock);
369
370 done:
371 release_sock(sk);
372 return err;
373 }
374
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Bluetooth: Fix type of len in rfcomm_sock_{bind,getsockopt_old}()
2024-10-02 14:12 [PATCH] Bluetooth: Fix type of len in rfcomm_sock_{bind,getsockopt_old}() Andrej Shadura
` (2 preceding siblings ...)
2024-10-04 15:13 ` kernel test robot
@ 2024-10-07 19:49 ` Aleksei Vetrov
2024-10-09 12:01 ` Andrej Shadura
3 siblings, 1 reply; 8+ messages in thread
From: Aleksei Vetrov @ 2024-10-07 19:49 UTC (permalink / raw)
To: Andrej Shadura
Cc: linux-bluetooth, Nathan Chancellor, Justin Stitt, llvm, kernel,
George Burgess, stable
Hi Andrej,
On Wed, Oct 02, 2024 at 04:12:17PM +0200, Andrej Shadura wrote:
> Change the type of len to size_t in both rfcomm_sock_bind and
> rfcomm_sock_getsockopt_old and replace min_t() with min().
rfcomm_sock_bind doesn't use copy_to_user, are you sure it has the same
issue?
> @@ -328,14 +328,15 @@ static int rfcomm_sock_bind(struct socket *sock, struct sockaddr *addr, int addr
> {
> struct sockaddr_rc sa;
> struct sock *sk = sock->sk;
> - int len, err = 0;
> + int err = 0;
> + size_t len;
>
> if (!addr || addr_len < offsetofend(struct sockaddr, sa_family) ||
> addr->sa_family != AF_BLUETOOTH)
> return -EINVAL;
>
> memset(&sa, 0, sizeof(sa));
> - len = min_t(unsigned int, sizeof(sa), addr_len);
> + len = min(sizeof(sa), addr_len);
> memcpy(&sa, addr, len);
>
> BT_DBG("sk %p %pMR", sk, &sa.rc_bdaddr);
This change produces a compilation error around min expression, as
"kernel test robot" notices below. And I think rfcomm_sock_bind
shouldn't be touched at all, it doesn't use copy_to_user and doesn't
produce compile errors with latest Clang.
> @@ -729,7 +730,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 +785,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;
>
This looks ok. But there is the same pattern in rfcomm_sock_getsockopt
(without old prefix) and it also uses copy_to_user and produces compile
error with latest Clang.
Could you remove rfcomm_sock_bind patch and apply it to
rfcomm_sock_getsockopt instead? Or I can send my version of the patch:
we've encountered the same compile errors in rfcomm_sock_getsockopt and
rfcomm_sock_getsockopt_old after updating Clang and would like to get it
fixed.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Bluetooth: Fix type of len in rfcomm_sock_{bind,getsockopt_old}()
2024-10-07 19:49 ` Aleksei Vetrov
@ 2024-10-09 12:01 ` Andrej Shadura
0 siblings, 0 replies; 8+ messages in thread
From: Andrej Shadura @ 2024-10-09 12:01 UTC (permalink / raw)
To: Aleksei Vetrov
Cc: linux-bluetooth, Nathan Chancellor, Justin Stitt, llvm, kernel,
George Burgess
On 07/10/2024 21:49, Aleksei Vetrov wrote:
> Hi Andrej,
>
> On Wed, Oct 02, 2024 at 04:12:17PM +0200, Andrej Shadura wrote:
>> Change the type of len to size_t in both rfcomm_sock_bind and
>> rfcomm_sock_getsockopt_old and replace min_t() with min().
> rfcomm_sock_bind doesn't use copy_to_user, are you sure it has the same
> issue?
> This change produces a compilation error around min expression, as
> "kernel test robot" notices below. And I think rfcomm_sock_bind
> shouldn't be touched at all, it doesn't use copy_to_user and doesn't
> produce compile errors with latest Clang.
Thanks, you’re right, I went a bit too far here :)
Rebuilding with a different verbosity setting revealed it here as well.
> This looks ok. But there is the same pattern in rfcomm_sock_getsockopt
> (without old prefix) and it also uses copy_to_user and produces compile
> error with latest Clang.
>
> Could you remove rfcomm_sock_bind patch and apply it to
> rfcomm_sock_getsockopt instead? Or I can send my version of the patch:
> we've encountered the same compile errors in rfcomm_sock_getsockopt and
> rfcomm_sock_getsockopt_old after updating Clang and would like to get it
> fixed.
Interestingly, it didn’t produce an error on my clang version, but sure,
I am amending the patch and will submit it shortly.
--
Cheers,
Andrej
^ permalink raw reply [flat|nested] 8+ messages in thread