* Re: [RFC PATCH 0/4] net/io_uring: pass a kernel pointer via optlen_t to proto[_ops].getsockopt()
From: Linus Torvalds @ 2025-04-02 0:40 UTC (permalink / raw)
To: Stefan Metzmacher
Cc: Jens Axboe, Pavel Begunkov, Breno Leitao, Jakub Kicinski,
Christoph Hellwig, Karsten Keil, Ayush Sawal, Andrew Lunn,
David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman,
Kuniyuki Iwashima, Willem de Bruijn, David Ahern,
Marcelo Ricardo Leitner, Xin Long, Neal Cardwell, Joerg Reuter,
Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
Oliver Hartkopp, Marc Kleine-Budde, Robin van der Gracht,
Oleksij Rempel, kernel, Alexander Aring, Stefan Schmidt,
Miquel Raynal, Alexandra Winter, Thorsten Winkler, James Chapman,
Jeremy Kerr, Matt Johnston, Matthieu Baerts, Mat Martineau,
Geliang Tang, Krzysztof Kozlowski, Remi Denis-Courmont,
Allison Henderson, David Howells, Marc Dionne, Wenjia Zhang,
Jan Karcher, D. Wythe, Tony Lu, Wen Gu, Jon Maloy, Boris Pismenny,
John Fastabend, Stefano Garzarella, Martin Schiller,
Björn Töpel, Magnus Karlsson, Maciej Fijalkowski,
Jonathan Lemon, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, netdev, linux-kernel, linux-sctp,
linux-hams, linux-bluetooth, linux-can, dccp, linux-wpan,
linux-s390, mptcp, linux-rdma, rds-devel, linux-afs,
tipc-discussion, virtualization, linux-x25, bpf, isdn4linux,
io-uring
In-Reply-To: <cover.1743449872.git.metze@samba.org>
"
On Mon, 31 Mar 2025 at 13:11, Stefan Metzmacher <metze@samba.org> wrote:
>
> But as Linus don't like 'sockptr_t' I used a different approach.
So the sockptr_t thing has already happened. I hate it, and I think
it's ugly as hell, but it is what it is.
I think it's a complete hack and having that "kernel or user" pointer
flag is disgusting.
Making things worse, the naming is disgusting too, talking about some
random "socket pointer", when it has absolutely nothing to do with
socket, and isn't even a pointer. It's something else.
It's literally called "socket" not because it has anything to do with
sockets, but because it's a socket-specific hack that isn't acceptable
anywhere else in the kernel.
So that "socket" part of the name is literally shorthand for "only
sockets are disgusting enough to use this, and nobody else should ever
touch this crap".
At least so far that part has mostly worked, even if there's some
"sockptr_t" use in the crypto code. I didn't look closer, because I
didn't want to lose my lunch.
I don't understand why the networking code uses that thing.
If you have a "fat pointer", you should damn well make it have the
size of the area too, and do things *right*.
Instead of doing what sockptr_t does, which is a complete hack to just
pass a kernel/user flag, and then passes the length *separately*
because the socket code couldn't be arsed to do the right thing.
So I do still think "sockptr_t" should die.
As Stanislav says, if you actually want that "user or kernel" thing,
just use an "iov_iter".
No, an "iov_iter" isn't exactly a pretty thing either, but at least
it's the standard way to say "this pointer can have multiple different
kinds of sources".
And it keeps the size of the thing it points to around, so it's at
least a fat pointer with proper ranges, even if it isn't exactly "type
safe" (yes, it's type safe in the sense that it stays as a "iov_iter",
but it's still basically a "random pointer").
> @Linus, would that optlen_t approach fit better for you?
The optlen_t thing is slightly better mainly because it's more
type-safe. At least it's not a "random misnamed
user-or-kernel-pointer" thing where the name is about how nothing else
is so broken as to use it.
So it's better because it's more limited, and it's better in that at
least it has a type-safe pointer rather than a "void *" with no size
or type associated with it.
That said, I don't think it's exactly great.
It's just another case of "networking can't just do it right, and uses
a random hack with special flag values".
So I do think that it would be better to actually get rid of
"sockptr_t optval, unsigned int optlen" ENTIRELY, and replace that
with iov_iter and just make networking bite the bullet and do the
RightThing(tm).
In fact, to make it *really* typesafe, it might be a good idea to wrap
the iov_iter in another struct, something like
typedef struct sockopt {
struct iov_iter iter;
} sockopt_t;
and make the networking functions make the typing very clear, and end
up with an interface something like
int do_tcp_setsockopt(struct sock *sk,
int level, int optname,
sockopt_t *val);
where that "sockopt_t *val" replaces not just the "sockptr_t optval",
but also the "unsigned int optlen" thing.
And no, I didn't look at how much churn that would be. Probably a lot.
Maybe more than people are willing to do - even if I think some of it
could be automated with coccinelle or whatever.
Linus
^ permalink raw reply
* Re: [RFC PATCH 0/4] net/io_uring: pass a kernel pointer via optlen_t to proto[_ops].getsockopt()
From: Stefan Metzmacher @ 2025-04-01 22:53 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Breno Leitao, Linus Torvalds, Jens Axboe, Pavel Begunkov,
Jakub Kicinski, Christoph Hellwig, Karsten Keil, Ayush Sawal,
Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
Simon Horman, Kuniyuki Iwashima, Willem de Bruijn, David Ahern,
Marcelo Ricardo Leitner, Xin Long, Neal Cardwell, Joerg Reuter,
Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
Oliver Hartkopp, Marc Kleine-Budde, Robin van der Gracht,
Oleksij Rempel, kernel, Alexander Aring, Stefan Schmidt,
Miquel Raynal, Alexandra Winter, Thorsten Winkler, James Chapman,
Jeremy Kerr, Matt Johnston, Matthieu Baerts, Mat Martineau,
Geliang Tang, Krzysztof Kozlowski, Remi Denis-Courmont,
Allison Henderson, David Howells, Marc Dionne, Wenjia Zhang,
Jan Karcher, D. Wythe, Tony Lu, Wen Gu, Jon Maloy, Boris Pismenny,
John Fastabend, Stefano Garzarella, Martin Schiller,
Björn Töpel, Magnus Karlsson, Maciej Fijalkowski,
Jonathan Lemon, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, netdev, linux-kernel, linux-sctp,
linux-hams, linux-bluetooth, linux-can, dccp, linux-wpan,
linux-s390, mptcp, linux-rdma, rds-devel, linux-afs,
tipc-discussion, virtualization, linux-x25, bpf, isdn4linux,
io-uring
In-Reply-To: <Z-xi7TH83upf-E3q@mini-arch>
Am 02.04.25 um 00:04 schrieb Stanislav Fomichev:
> On 04/01, Stefan Metzmacher wrote:
>> Am 01.04.25 um 17:45 schrieb Stanislav Fomichev:
>>> On 04/01, Breno Leitao wrote:
>>>> On Tue, Apr 01, 2025 at 03:48:58PM +0200, Stefan Metzmacher wrote:
>>>>> Am 01.04.25 um 15:37 schrieb Stefan Metzmacher:
>>>>>> Am 01.04.25 um 10:19 schrieb Stefan Metzmacher:
>>>>>>> Am 31.03.25 um 23:04 schrieb Stanislav Fomichev:
>>>>>>>> On 03/31, Stefan Metzmacher wrote:
>>>>>>>>> The motivation for this is to remove the SOL_SOCKET limitation
>>>>>>>>> from io_uring_cmd_getsockopt().
>>>>>>>>>
>>>>>>>>> The reason for this limitation is that io_uring_cmd_getsockopt()
>>>>>>>>> passes a kernel pointer as optlen to do_sock_getsockopt()
>>>>>>>>> and can't reach the ops->getsockopt() path.
>>>>>>>>>
>>>>>>>>> The first idea would be to change the optval and optlen arguments
>>>>>>>>> to the protocol specific hooks also to sockptr_t, as that
>>>>>>>>> is already used for setsockopt() and also by do_sock_getsockopt()
>>>>>>>>> sk_getsockopt() and BPF_CGROUP_RUN_PROG_GETSOCKOPT().
>>>>>>>>>
>>>>>>>>> But as Linus don't like 'sockptr_t' I used a different approach.
>>>>>>>>>
>>>>>>>>> @Linus, would that optlen_t approach fit better for you?
>>>>>>>>
>>>>>>>> [..]
>>>>>>>>
>>>>>>>>> Instead of passing the optlen as user or kernel pointer,
>>>>>>>>> we only ever pass a kernel pointer and do the
>>>>>>>>> translation from/to userspace in do_sock_getsockopt().
>>>>>>>>
>>>>>>>> At this point why not just fully embrace iov_iter? You have the size
>>>>>>>> now + the user (or kernel) pointer. Might as well do
>>>>>>>> s/sockptr_t/iov_iter/ conversion?
>>>>>>>
>>>>>>> I think that would only be possible if we introduce
>>>>>>> proto[_ops].getsockopt_iter() and then convert the implementations
>>>>>>> step by step. Doing it all in one go has a lot of potential to break
>>>>>>> the uapi. I could try to convert things like socket, ip and tcp myself, but
>>>>>>> the rest needs to be converted by the maintainer of the specific protocol,
>>>>>>> as it needs to be tested. As there are crazy things happening in the existing
>>>>>>> implementations, e.g. some getsockopt() implementations use optval as in and out
>>>>>>> buffer.
>>>>>>>
>>>>>>> I first tried to convert both optval and optlen of getsockopt to sockptr_t,
>>>>>>> and that showed that touching the optval part starts to get complex very soon,
>>>>>>> see https://git.samba.org/?p=metze/linux/wip.git;a=commitdiff;h=141912166473bf8843ec6ace76dc9c6945adafd1
>>>>>>> (note it didn't converted everything, I gave up after hitting
>>>>>>> sctp_getsockopt_peer_addrs and sctp_getsockopt_local_addrs.
>>>>>>> sctp_getsockopt_context, sctp_getsockopt_maxseg, sctp_getsockopt_associnfo and maybe
>>>>>>> more are the ones also doing both copy_from_user and copy_to_user on optval)
>>>>>>>
>>>>>>> I come also across one implementation that returned -ERANGE because *optlen was
>>>>>>> too short and put the required length into *optlen, which means the returned
>>>>>>> *optlen is larger than the optval buffer given from userspace.
>>>>>>>
>>>>>>> Because of all these strange things I tried to do a minimal change
>>>>>>> in order to get rid of the io_uring limitation and only converted
>>>>>>> optlen and leave optval as is.
>>>>>>>
>>>>>>> In order to have a patchset that has a low risk to cause regressions.
>>>>>>>
>>>>>>> But as alternative introducing a prototype like this:
>>>>>>>
>>>>>>> int (*getsockopt_iter)(struct socket *sock, int level, int optname,
>>>>>>> struct iov_iter *optval_iter);
>>>>>>>
>>>>>>> That returns a non-negative value which can be placed into *optlen
>>>>>>> or negative value as error and *optlen will not be changed on error.
>>>>>>> optval_iter will get direction ITER_DEST, so it can only be written to.
>>>>>>>
>>>>>>> Implementations could then opt in for the new interface and
>>>>>>> allow do_sock_getsockopt() work also for the io_uring case,
>>>>>>> while all others would still get -EOPNOTSUPP.
>>>>>>>
>>>>>>> So what should be the way to go?
>>>>>>
>>>>>> Ok, I've added the infrastructure for getsockopt_iter, see below,
>>>>>> but the first part I wanted to convert was
>>>>>> tcp_ao_copy_mkts_to_user() and that also reads from userspace before
>>>>>> writing.
>>>>>>
>>>>>> So we could go with the optlen_t approach, or we need
>>>>>> logic for ITER_BOTH or pass two iov_iters one with ITER_SRC and one
>>>>>> with ITER_DEST...
>>>>>>
>>>>>> So who wants to decide?
>>>>>
>>>>> I just noticed that it's even possible in same cases
>>>>> to pass in a short buffer to optval, but have a longer value in optlen,
>>>>> hci_sock_getsockopt() with SOL_BLUETOOTH completely ignores optlen.
>>>>>
>>>>> This makes it really hard to believe that trying to use iov_iter for this
>>>>> is a good idea :-(
>>>>
>>>> That was my finding as well a while ago, when I was planning to get the
>>>> __user pointers converted to iov_iter. There are some weird ways of
>>>> using optlen and optval, which makes them non-trivial to covert to
>>>> iov_iter.
>>>
>>> Can we ignore all non-ip/tcp/udp cases for now? This should cover +90%
>>> of useful socket opts. See if there are any obvious problems with them
>>> and if not, try converting. The rest we can cover separately when/if
>>> needed.
>>
>> That's what I tried, but it fails with
>> tcp_getsockopt ->
>> do_tcp_getsockopt ->
>> tcp_ao_get_mkts ->
>> tcp_ao_copy_mkts_to_user ->
>> copy_struct_from_sockptr
>> tcp_ao_get_sock_info ->
>> copy_struct_from_sockptr
>>
>> That's not possible with a ITER_DEST iov_iter.
>>
>> metze
>
> Can we create two iterators over the same memory? One for ITER_SOURCE and
> another for ITER_DEST. And then make getsockopt_iter accept optval_in and
> optval_out. We can also use optval_out position (iov_offset) as optlen output
> value. Don't see why it won't work, but I agree that's gonna be a messy
> conversion so let's see if someone else has better suggestions.
Yes, that might work, but it would be good to get some feedback
if this would be the way to go:
int (*getsockopt_iter)(struct socket *sock,
int level, int optname,
struct iov_iter *optval_in,
struct iov_iter *optval_out);
And *optlen = optval_out->iov_offset;
Any objection or better ideas? Linus would that be what you had in mind?
Thanks!
metze
^ permalink raw reply
* Re: [RFC PATCH 0/4] net/io_uring: pass a kernel pointer via optlen_t to proto[_ops].getsockopt()
From: Stanislav Fomichev @ 2025-04-01 22:04 UTC (permalink / raw)
To: Stefan Metzmacher
Cc: Breno Leitao, Linus Torvalds, Jens Axboe, Pavel Begunkov,
Jakub Kicinski, Christoph Hellwig, Karsten Keil, Ayush Sawal,
Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
Simon Horman, Kuniyuki Iwashima, Willem de Bruijn, David Ahern,
Marcelo Ricardo Leitner, Xin Long, Neal Cardwell, Joerg Reuter,
Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
Oliver Hartkopp, Marc Kleine-Budde, Robin van der Gracht,
Oleksij Rempel, kernel, Alexander Aring, Stefan Schmidt,
Miquel Raynal, Alexandra Winter, Thorsten Winkler, James Chapman,
Jeremy Kerr, Matt Johnston, Matthieu Baerts, Mat Martineau,
Geliang Tang, Krzysztof Kozlowski, Remi Denis-Courmont,
Allison Henderson, David Howells, Marc Dionne, Wenjia Zhang,
Jan Karcher, D. Wythe, Tony Lu, Wen Gu, Jon Maloy, Boris Pismenny,
John Fastabend, Stefano Garzarella, Martin Schiller,
Björn Töpel, Magnus Karlsson, Maciej Fijalkowski,
Jonathan Lemon, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, netdev, linux-kernel, linux-sctp,
linux-hams, linux-bluetooth, linux-can, dccp, linux-wpan,
linux-s390, mptcp, linux-rdma, rds-devel, linux-afs,
tipc-discussion, virtualization, linux-x25, bpf, isdn4linux,
io-uring
In-Reply-To: <0f0f9cfd-77be-4988-8043-0d1bd0d157e7@samba.org>
On 04/01, Stefan Metzmacher wrote:
> Am 01.04.25 um 17:45 schrieb Stanislav Fomichev:
> > On 04/01, Breno Leitao wrote:
> > > On Tue, Apr 01, 2025 at 03:48:58PM +0200, Stefan Metzmacher wrote:
> > > > Am 01.04.25 um 15:37 schrieb Stefan Metzmacher:
> > > > > Am 01.04.25 um 10:19 schrieb Stefan Metzmacher:
> > > > > > Am 31.03.25 um 23:04 schrieb Stanislav Fomichev:
> > > > > > > On 03/31, Stefan Metzmacher wrote:
> > > > > > > > The motivation for this is to remove the SOL_SOCKET limitation
> > > > > > > > from io_uring_cmd_getsockopt().
> > > > > > > >
> > > > > > > > The reason for this limitation is that io_uring_cmd_getsockopt()
> > > > > > > > passes a kernel pointer as optlen to do_sock_getsockopt()
> > > > > > > > and can't reach the ops->getsockopt() path.
> > > > > > > >
> > > > > > > > The first idea would be to change the optval and optlen arguments
> > > > > > > > to the protocol specific hooks also to sockptr_t, as that
> > > > > > > > is already used for setsockopt() and also by do_sock_getsockopt()
> > > > > > > > sk_getsockopt() and BPF_CGROUP_RUN_PROG_GETSOCKOPT().
> > > > > > > >
> > > > > > > > But as Linus don't like 'sockptr_t' I used a different approach.
> > > > > > > >
> > > > > > > > @Linus, would that optlen_t approach fit better for you?
> > > > > > >
> > > > > > > [..]
> > > > > > >
> > > > > > > > Instead of passing the optlen as user or kernel pointer,
> > > > > > > > we only ever pass a kernel pointer and do the
> > > > > > > > translation from/to userspace in do_sock_getsockopt().
> > > > > > >
> > > > > > > At this point why not just fully embrace iov_iter? You have the size
> > > > > > > now + the user (or kernel) pointer. Might as well do
> > > > > > > s/sockptr_t/iov_iter/ conversion?
> > > > > >
> > > > > > I think that would only be possible if we introduce
> > > > > > proto[_ops].getsockopt_iter() and then convert the implementations
> > > > > > step by step. Doing it all in one go has a lot of potential to break
> > > > > > the uapi. I could try to convert things like socket, ip and tcp myself, but
> > > > > > the rest needs to be converted by the maintainer of the specific protocol,
> > > > > > as it needs to be tested. As there are crazy things happening in the existing
> > > > > > implementations, e.g. some getsockopt() implementations use optval as in and out
> > > > > > buffer.
> > > > > >
> > > > > > I first tried to convert both optval and optlen of getsockopt to sockptr_t,
> > > > > > and that showed that touching the optval part starts to get complex very soon,
> > > > > > see https://git.samba.org/?p=metze/linux/wip.git;a=commitdiff;h=141912166473bf8843ec6ace76dc9c6945adafd1
> > > > > > (note it didn't converted everything, I gave up after hitting
> > > > > > sctp_getsockopt_peer_addrs and sctp_getsockopt_local_addrs.
> > > > > > sctp_getsockopt_context, sctp_getsockopt_maxseg, sctp_getsockopt_associnfo and maybe
> > > > > > more are the ones also doing both copy_from_user and copy_to_user on optval)
> > > > > >
> > > > > > I come also across one implementation that returned -ERANGE because *optlen was
> > > > > > too short and put the required length into *optlen, which means the returned
> > > > > > *optlen is larger than the optval buffer given from userspace.
> > > > > >
> > > > > > Because of all these strange things I tried to do a minimal change
> > > > > > in order to get rid of the io_uring limitation and only converted
> > > > > > optlen and leave optval as is.
> > > > > >
> > > > > > In order to have a patchset that has a low risk to cause regressions.
> > > > > >
> > > > > > But as alternative introducing a prototype like this:
> > > > > >
> > > > > > int (*getsockopt_iter)(struct socket *sock, int level, int optname,
> > > > > > struct iov_iter *optval_iter);
> > > > > >
> > > > > > That returns a non-negative value which can be placed into *optlen
> > > > > > or negative value as error and *optlen will not be changed on error.
> > > > > > optval_iter will get direction ITER_DEST, so it can only be written to.
> > > > > >
> > > > > > Implementations could then opt in for the new interface and
> > > > > > allow do_sock_getsockopt() work also for the io_uring case,
> > > > > > while all others would still get -EOPNOTSUPP.
> > > > > >
> > > > > > So what should be the way to go?
> > > > >
> > > > > Ok, I've added the infrastructure for getsockopt_iter, see below,
> > > > > but the first part I wanted to convert was
> > > > > tcp_ao_copy_mkts_to_user() and that also reads from userspace before
> > > > > writing.
> > > > >
> > > > > So we could go with the optlen_t approach, or we need
> > > > > logic for ITER_BOTH or pass two iov_iters one with ITER_SRC and one
> > > > > with ITER_DEST...
> > > > >
> > > > > So who wants to decide?
> > > >
> > > > I just noticed that it's even possible in same cases
> > > > to pass in a short buffer to optval, but have a longer value in optlen,
> > > > hci_sock_getsockopt() with SOL_BLUETOOTH completely ignores optlen.
> > > >
> > > > This makes it really hard to believe that trying to use iov_iter for this
> > > > is a good idea :-(
> > >
> > > That was my finding as well a while ago, when I was planning to get the
> > > __user pointers converted to iov_iter. There are some weird ways of
> > > using optlen and optval, which makes them non-trivial to covert to
> > > iov_iter.
> >
> > Can we ignore all non-ip/tcp/udp cases for now? This should cover +90%
> > of useful socket opts. See if there are any obvious problems with them
> > and if not, try converting. The rest we can cover separately when/if
> > needed.
>
> That's what I tried, but it fails with
> tcp_getsockopt ->
> do_tcp_getsockopt ->
> tcp_ao_get_mkts ->
> tcp_ao_copy_mkts_to_user ->
> copy_struct_from_sockptr
> tcp_ao_get_sock_info ->
> copy_struct_from_sockptr
>
> That's not possible with a ITER_DEST iov_iter.
>
> metze
Can we create two iterators over the same memory? One for ITER_SOURCE and
another for ITER_DEST. And then make getsockopt_iter accept optval_in and
optval_out. We can also use optval_out position (iov_offset) as optlen output
value. Don't see why it won't work, but I agree that's gonna be a messy
conversion so let's see if someone else has better suggestions.
^ permalink raw reply
* Re: [PATCH BlueZ v2] shared/bap: Fix swallowing states transitions
From: patchwork-bot+bluetooth @ 2025-04-01 21:40 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <20250401135115.3735533-1-luiz.dentz@gmail.com>
Hello:
This patch was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:
On Tue, 1 Apr 2025 09:51:15 -0400 you wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> In certain cases (e.g ASCS_Enable) a Control Point operation may change
> states multiple times causing states not to be notified.
>
> To fix this attempts to queue states if timeout is pending (state_id)
> and then proceed to notify them ahead of the last state set in the ASE
> so the remote side is informed of all the state transitions.
>
> [...]
Here is the summary with links:
- [BlueZ,v2] shared/bap: Fix swallowing states transitions
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=6d20a300642f
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH BlueZ v1] dbus: Fix condition for invalidating path
From: patchwork-bot+bluetooth @ 2025-04-01 21:40 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <20250401134904.3721187-1-luiz.dentz@gmail.com>
Hello:
This patch was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:
On Tue, 1 Apr 2025 09:49:04 -0400 you wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> This fixes the condition introduced in cdd02afbb7ef
> ("dbus: Fix add invalid memory during interface removal") which was
> reversed while applying the original fix.
>
> Fixes: https://github.com/bluez/bluez/issues/1155
>
> [...]
Here is the summary with links:
- [BlueZ,v1] dbus: Fix condition for invalidating path
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=da5846c096cd
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [RFC PATCH 0/4] net/io_uring: pass a kernel pointer via optlen_t to proto[_ops].getsockopt()
From: Stefan Metzmacher @ 2025-04-01 21:20 UTC (permalink / raw)
To: Stanislav Fomichev, Breno Leitao
Cc: Linus Torvalds, Jens Axboe, Pavel Begunkov, Jakub Kicinski,
Christoph Hellwig, Karsten Keil, Ayush Sawal, Andrew Lunn,
David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman,
Kuniyuki Iwashima, Willem de Bruijn, David Ahern,
Marcelo Ricardo Leitner, Xin Long, Neal Cardwell, Joerg Reuter,
Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
Oliver Hartkopp, Marc Kleine-Budde, Robin van der Gracht,
Oleksij Rempel, kernel, Alexander Aring, Stefan Schmidt,
Miquel Raynal, Alexandra Winter, Thorsten Winkler, James Chapman,
Jeremy Kerr, Matt Johnston, Matthieu Baerts, Mat Martineau,
Geliang Tang, Krzysztof Kozlowski, Remi Denis-Courmont,
Allison Henderson, David Howells, Marc Dionne, Wenjia Zhang,
Jan Karcher, D. Wythe, Tony Lu, Wen Gu, Jon Maloy, Boris Pismenny,
John Fastabend, Stefano Garzarella, Martin Schiller,
Björn Töpel, Magnus Karlsson, Maciej Fijalkowski,
Jonathan Lemon, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, netdev, linux-kernel, linux-sctp,
linux-hams, linux-bluetooth, linux-can, dccp, linux-wpan,
linux-s390, mptcp, linux-rdma, rds-devel, linux-afs,
tipc-discussion, virtualization, linux-x25, bpf, isdn4linux,
io-uring
In-Reply-To: <Z-wKI1rQGSgrsjbl@mini-arch>
Am 01.04.25 um 17:45 schrieb Stanislav Fomichev:
> On 04/01, Breno Leitao wrote:
>> On Tue, Apr 01, 2025 at 03:48:58PM +0200, Stefan Metzmacher wrote:
>>> Am 01.04.25 um 15:37 schrieb Stefan Metzmacher:
>>>> Am 01.04.25 um 10:19 schrieb Stefan Metzmacher:
>>>>> Am 31.03.25 um 23:04 schrieb Stanislav Fomichev:
>>>>>> On 03/31, Stefan Metzmacher wrote:
>>>>>>> The motivation for this is to remove the SOL_SOCKET limitation
>>>>>>> from io_uring_cmd_getsockopt().
>>>>>>>
>>>>>>> The reason for this limitation is that io_uring_cmd_getsockopt()
>>>>>>> passes a kernel pointer as optlen to do_sock_getsockopt()
>>>>>>> and can't reach the ops->getsockopt() path.
>>>>>>>
>>>>>>> The first idea would be to change the optval and optlen arguments
>>>>>>> to the protocol specific hooks also to sockptr_t, as that
>>>>>>> is already used for setsockopt() and also by do_sock_getsockopt()
>>>>>>> sk_getsockopt() and BPF_CGROUP_RUN_PROG_GETSOCKOPT().
>>>>>>>
>>>>>>> But as Linus don't like 'sockptr_t' I used a different approach.
>>>>>>>
>>>>>>> @Linus, would that optlen_t approach fit better for you?
>>>>>>
>>>>>> [..]
>>>>>>
>>>>>>> Instead of passing the optlen as user or kernel pointer,
>>>>>>> we only ever pass a kernel pointer and do the
>>>>>>> translation from/to userspace in do_sock_getsockopt().
>>>>>>
>>>>>> At this point why not just fully embrace iov_iter? You have the size
>>>>>> now + the user (or kernel) pointer. Might as well do
>>>>>> s/sockptr_t/iov_iter/ conversion?
>>>>>
>>>>> I think that would only be possible if we introduce
>>>>> proto[_ops].getsockopt_iter() and then convert the implementations
>>>>> step by step. Doing it all in one go has a lot of potential to break
>>>>> the uapi. I could try to convert things like socket, ip and tcp myself, but
>>>>> the rest needs to be converted by the maintainer of the specific protocol,
>>>>> as it needs to be tested. As there are crazy things happening in the existing
>>>>> implementations, e.g. some getsockopt() implementations use optval as in and out
>>>>> buffer.
>>>>>
>>>>> I first tried to convert both optval and optlen of getsockopt to sockptr_t,
>>>>> and that showed that touching the optval part starts to get complex very soon,
>>>>> see https://git.samba.org/?p=metze/linux/wip.git;a=commitdiff;h=141912166473bf8843ec6ace76dc9c6945adafd1
>>>>> (note it didn't converted everything, I gave up after hitting
>>>>> sctp_getsockopt_peer_addrs and sctp_getsockopt_local_addrs.
>>>>> sctp_getsockopt_context, sctp_getsockopt_maxseg, sctp_getsockopt_associnfo and maybe
>>>>> more are the ones also doing both copy_from_user and copy_to_user on optval)
>>>>>
>>>>> I come also across one implementation that returned -ERANGE because *optlen was
>>>>> too short and put the required length into *optlen, which means the returned
>>>>> *optlen is larger than the optval buffer given from userspace.
>>>>>
>>>>> Because of all these strange things I tried to do a minimal change
>>>>> in order to get rid of the io_uring limitation and only converted
>>>>> optlen and leave optval as is.
>>>>>
>>>>> In order to have a patchset that has a low risk to cause regressions.
>>>>>
>>>>> But as alternative introducing a prototype like this:
>>>>>
>>>>> int (*getsockopt_iter)(struct socket *sock, int level, int optname,
>>>>> struct iov_iter *optval_iter);
>>>>>
>>>>> That returns a non-negative value which can be placed into *optlen
>>>>> or negative value as error and *optlen will not be changed on error.
>>>>> optval_iter will get direction ITER_DEST, so it can only be written to.
>>>>>
>>>>> Implementations could then opt in for the new interface and
>>>>> allow do_sock_getsockopt() work also for the io_uring case,
>>>>> while all others would still get -EOPNOTSUPP.
>>>>>
>>>>> So what should be the way to go?
>>>>
>>>> Ok, I've added the infrastructure for getsockopt_iter, see below,
>>>> but the first part I wanted to convert was
>>>> tcp_ao_copy_mkts_to_user() and that also reads from userspace before
>>>> writing.
>>>>
>>>> So we could go with the optlen_t approach, or we need
>>>> logic for ITER_BOTH or pass two iov_iters one with ITER_SRC and one
>>>> with ITER_DEST...
>>>>
>>>> So who wants to decide?
>>>
>>> I just noticed that it's even possible in same cases
>>> to pass in a short buffer to optval, but have a longer value in optlen,
>>> hci_sock_getsockopt() with SOL_BLUETOOTH completely ignores optlen.
>>>
>>> This makes it really hard to believe that trying to use iov_iter for this
>>> is a good idea :-(
>>
>> That was my finding as well a while ago, when I was planning to get the
>> __user pointers converted to iov_iter. There are some weird ways of
>> using optlen and optval, which makes them non-trivial to covert to
>> iov_iter.
>
> Can we ignore all non-ip/tcp/udp cases for now? This should cover +90%
> of useful socket opts. See if there are any obvious problems with them
> and if not, try converting. The rest we can cover separately when/if
> needed.
That's what I tried, but it fails with
tcp_getsockopt ->
do_tcp_getsockopt ->
tcp_ao_get_mkts ->
tcp_ao_copy_mkts_to_user ->
copy_struct_from_sockptr
tcp_ao_get_sock_info ->
copy_struct_from_sockptr
That's not possible with a ITER_DEST iov_iter.
metze
^ permalink raw reply
* [bluez/bluez] da5846: dbus: Fix condition for invalidating path
From: Luiz Augusto von Dentz @ 2025-04-01 21:13 UTC (permalink / raw)
To: linux-bluetooth
Branch: refs/heads/master
Home: https://github.com/bluez/bluez
Commit: da5846c096cd1006d512bbdbc466fc46a61417b8
https://github.com/bluez/bluez/commit/da5846c096cd1006d512bbdbc466fc46a61417b8
Author: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Date: 2025-04-01 (Tue, 01 Apr 2025)
Changed paths:
M gdbus/object.c
Log Message:
-----------
dbus: Fix condition for invalidating path
This fixes the condition introduced in cdd02afbb7ef
("dbus: Fix add invalid memory during interface removal") which was
reversed while applying the original fix.
Fixes: https://github.com/bluez/bluez/issues/1155
Commit: 6d20a300642f312290af0bc9869a0e1b416c58dc
https://github.com/bluez/bluez/commit/6d20a300642f312290af0bc9869a0e1b416c58dc
Author: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Date: 2025-04-01 (Tue, 01 Apr 2025)
Changed paths:
M src/shared/bap.c
Log Message:
-----------
shared/bap: Fix swallowing states transitions
In certain cases (e.g ASCS_Enable) a Control Point operation may change
states multiple times causing states not to be notified.
To fix this attempts to queue states if timeout is pending (state_id)
and then proceed to notify them ahead of the last state set in the ASE
so the remote side is informed of all the state transitions.
Compare: https://github.com/bluez/bluez/compare/47e5d3491d37...6d20a300642f
To unsubscribe from these emails, change your notification settings at https://github.com/bluez/bluez/settings/notifications
^ permalink raw reply
* RE: [v1] Bluetooth: hci_event: Fix sending MGMT_EV_DEVICE_FOUND for invalid address
From: bluez.test.bot @ 2025-04-01 17:32 UTC (permalink / raw)
To: linux-bluetooth, luiz.dentz
In-Reply-To: <20250401171013.3785788-1-luiz.dentz@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2230 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=949043
---Test result---
Test Summary:
CheckPatch PENDING 0.29 seconds
GitLint PENDING 0.20 seconds
SubjectPrefix PASS 0.13 seconds
BuildKernel PASS 24.76 seconds
CheckAllWarning PASS 27.30 seconds
CheckSparse WARNING 31.23 seconds
BuildKernel32 PASS 24.66 seconds
TestRunnerSetup PASS 437.15 seconds
TestRunner_l2cap-tester PASS 21.70 seconds
TestRunner_iso-tester PASS 35.11 seconds
TestRunner_bnep-tester PASS 4.87 seconds
TestRunner_mgmt-tester FAIL 121.19 seconds
TestRunner_rfcomm-tester PASS 8.05 seconds
TestRunner_sco-tester PASS 12.83 seconds
TestRunner_ioctl-tester PASS 8.55 seconds
TestRunner_mesh-tester PASS 6.24 seconds
TestRunner_smp-tester PASS 7.49 seconds
TestRunner_userchan-tester PASS 5.13 seconds
IncrementalBuild PENDING 0.94 seconds
Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:
##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:
##############################
Test: CheckSparse - WARNING
Desc: Run sparse tool with linux kernel
Output:
net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 490, Passed: 484 (98.8%), Failed: 2, Not Run: 4
Failed Test Cases
LL Privacy - Add Device 3 (AL is full) Failed 0.202 seconds
LL Privacy - Set Flags 3 (2 Devices to RL) Failed 0.176 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:
---
Regards,
Linux Bluetooth
^ permalink raw reply
* [PATCH v1] Bluetooth: hci_event: Fix sending MGMT_EV_DEVICE_FOUND for invalid address
From: Luiz Augusto von Dentz @ 2025-04-01 17:10 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This fixes sending MGMT_EV_DEVICE_FOUND for invalid address
(00:00:00:00:00:00) which is a regression introduced by
a2ec905d1e16 ("Bluetooth: fix kernel oops in store_pending_adv_report")
since in the attempt to skip storing data for extended advertisement it
actually made the code to skip the entire if statement supposed to send
MGMT_EV_DEVICE_FOUND without attempting to use the last_addr_adv which
is garanteed to be invalid for extended advertisement since we never
store anything on it.
Link: https://github.com/bluez/bluez/issues/1157
Link: https://github.com/bluez/bluez/issues/1149#issuecomment-2767215658
Fixes: a2ec905d1e16 ("Bluetooth: fix kernel oops in store_pending_adv_report")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
net/bluetooth/hci_event.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 1d8616f2e740..5f808f0b0e9a 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -6160,11 +6160,12 @@ static void process_adv_report(struct hci_dev *hdev, u8 type, bdaddr_t *bdaddr,
* event or send an immediate device found event if the data
* should not be stored for later.
*/
- if (!ext_adv && !has_pending_adv_report(hdev)) {
+ if (!has_pending_adv_report(hdev)) {
/* If the report will trigger a SCAN_REQ store it for
* later merging.
*/
- if (type == LE_ADV_IND || type == LE_ADV_SCAN_IND) {
+ if (!ext_adv && (type == LE_ADV_IND ||
+ type == LE_ADV_SCAN_IND)) {
store_pending_adv_report(hdev, bdaddr, bdaddr_type,
rssi, flags, data, len);
return;
--
2.48.1
^ permalink raw reply related
* Re: [RFC PATCH 0/4] net/io_uring: pass a kernel pointer via optlen_t to proto[_ops].getsockopt()
From: Stanislav Fomichev @ 2025-04-01 15:45 UTC (permalink / raw)
To: Breno Leitao
Cc: Stefan Metzmacher, Linus Torvalds, Jens Axboe, Pavel Begunkov,
Jakub Kicinski, Christoph Hellwig, Karsten Keil, Ayush Sawal,
Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
Simon Horman, Kuniyuki Iwashima, Willem de Bruijn, David Ahern,
Marcelo Ricardo Leitner, Xin Long, Neal Cardwell, Joerg Reuter,
Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
Oliver Hartkopp, Marc Kleine-Budde, Robin van der Gracht,
Oleksij Rempel, kernel, Alexander Aring, Stefan Schmidt,
Miquel Raynal, Alexandra Winter, Thorsten Winkler, James Chapman,
Jeremy Kerr, Matt Johnston, Matthieu Baerts, Mat Martineau,
Geliang Tang, Krzysztof Kozlowski, Remi Denis-Courmont,
Allison Henderson, David Howells, Marc Dionne, Wenjia Zhang,
Jan Karcher, D. Wythe, Tony Lu, Wen Gu, Jon Maloy, Boris Pismenny,
John Fastabend, Stefano Garzarella, Martin Schiller,
Björn Töpel, Magnus Karlsson, Maciej Fijalkowski,
Jonathan Lemon, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, netdev, linux-kernel, linux-sctp,
linux-hams, linux-bluetooth, linux-can, dccp, linux-wpan,
linux-s390, mptcp, linux-rdma, rds-devel, linux-afs,
tipc-discussion, virtualization, linux-x25, bpf, isdn4linux,
io-uring
In-Reply-To: <Z+wH1oYOr1dlKeyN@gmail.com>
On 04/01, Breno Leitao wrote:
> On Tue, Apr 01, 2025 at 03:48:58PM +0200, Stefan Metzmacher wrote:
> > Am 01.04.25 um 15:37 schrieb Stefan Metzmacher:
> > > Am 01.04.25 um 10:19 schrieb Stefan Metzmacher:
> > > > Am 31.03.25 um 23:04 schrieb Stanislav Fomichev:
> > > > > On 03/31, Stefan Metzmacher wrote:
> > > > > > The motivation for this is to remove the SOL_SOCKET limitation
> > > > > > from io_uring_cmd_getsockopt().
> > > > > >
> > > > > > The reason for this limitation is that io_uring_cmd_getsockopt()
> > > > > > passes a kernel pointer as optlen to do_sock_getsockopt()
> > > > > > and can't reach the ops->getsockopt() path.
> > > > > >
> > > > > > The first idea would be to change the optval and optlen arguments
> > > > > > to the protocol specific hooks also to sockptr_t, as that
> > > > > > is already used for setsockopt() and also by do_sock_getsockopt()
> > > > > > sk_getsockopt() and BPF_CGROUP_RUN_PROG_GETSOCKOPT().
> > > > > >
> > > > > > But as Linus don't like 'sockptr_t' I used a different approach.
> > > > > >
> > > > > > @Linus, would that optlen_t approach fit better for you?
> > > > >
> > > > > [..]
> > > > >
> > > > > > Instead of passing the optlen as user or kernel pointer,
> > > > > > we only ever pass a kernel pointer and do the
> > > > > > translation from/to userspace in do_sock_getsockopt().
> > > > >
> > > > > At this point why not just fully embrace iov_iter? You have the size
> > > > > now + the user (or kernel) pointer. Might as well do
> > > > > s/sockptr_t/iov_iter/ conversion?
> > > >
> > > > I think that would only be possible if we introduce
> > > > proto[_ops].getsockopt_iter() and then convert the implementations
> > > > step by step. Doing it all in one go has a lot of potential to break
> > > > the uapi. I could try to convert things like socket, ip and tcp myself, but
> > > > the rest needs to be converted by the maintainer of the specific protocol,
> > > > as it needs to be tested. As there are crazy things happening in the existing
> > > > implementations, e.g. some getsockopt() implementations use optval as in and out
> > > > buffer.
> > > >
> > > > I first tried to convert both optval and optlen of getsockopt to sockptr_t,
> > > > and that showed that touching the optval part starts to get complex very soon,
> > > > see https://git.samba.org/?p=metze/linux/wip.git;a=commitdiff;h=141912166473bf8843ec6ace76dc9c6945adafd1
> > > > (note it didn't converted everything, I gave up after hitting
> > > > sctp_getsockopt_peer_addrs and sctp_getsockopt_local_addrs.
> > > > sctp_getsockopt_context, sctp_getsockopt_maxseg, sctp_getsockopt_associnfo and maybe
> > > > more are the ones also doing both copy_from_user and copy_to_user on optval)
> > > >
> > > > I come also across one implementation that returned -ERANGE because *optlen was
> > > > too short and put the required length into *optlen, which means the returned
> > > > *optlen is larger than the optval buffer given from userspace.
> > > >
> > > > Because of all these strange things I tried to do a minimal change
> > > > in order to get rid of the io_uring limitation and only converted
> > > > optlen and leave optval as is.
> > > >
> > > > In order to have a patchset that has a low risk to cause regressions.
> > > >
> > > > But as alternative introducing a prototype like this:
> > > >
> > > > int (*getsockopt_iter)(struct socket *sock, int level, int optname,
> > > > struct iov_iter *optval_iter);
> > > >
> > > > That returns a non-negative value which can be placed into *optlen
> > > > or negative value as error and *optlen will not be changed on error.
> > > > optval_iter will get direction ITER_DEST, so it can only be written to.
> > > >
> > > > Implementations could then opt in for the new interface and
> > > > allow do_sock_getsockopt() work also for the io_uring case,
> > > > while all others would still get -EOPNOTSUPP.
> > > >
> > > > So what should be the way to go?
> > >
> > > Ok, I've added the infrastructure for getsockopt_iter, see below,
> > > but the first part I wanted to convert was
> > > tcp_ao_copy_mkts_to_user() and that also reads from userspace before
> > > writing.
> > >
> > > So we could go with the optlen_t approach, or we need
> > > logic for ITER_BOTH or pass two iov_iters one with ITER_SRC and one
> > > with ITER_DEST...
> > >
> > > So who wants to decide?
> >
> > I just noticed that it's even possible in same cases
> > to pass in a short buffer to optval, but have a longer value in optlen,
> > hci_sock_getsockopt() with SOL_BLUETOOTH completely ignores optlen.
> >
> > This makes it really hard to believe that trying to use iov_iter for this
> > is a good idea :-(
>
> That was my finding as well a while ago, when I was planning to get the
> __user pointers converted to iov_iter. There are some weird ways of
> using optlen and optval, which makes them non-trivial to covert to
> iov_iter.
Can we ignore all non-ip/tcp/udp cases for now? This should cover +90%
of useful socket opts. See if there are any obvious problems with them
and if not, try converting. The rest we can cover separately when/if
needed.
^ permalink raw reply
* Re: [RFC PATCH 0/4] net/io_uring: pass a kernel pointer via optlen_t to proto[_ops].getsockopt()
From: Breno Leitao @ 2025-04-01 15:35 UTC (permalink / raw)
To: Stefan Metzmacher
Cc: Stanislav Fomichev, Linus Torvalds, Jens Axboe, Pavel Begunkov,
Jakub Kicinski, Christoph Hellwig, Karsten Keil, Ayush Sawal,
Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
Simon Horman, Kuniyuki Iwashima, Willem de Bruijn, David Ahern,
Marcelo Ricardo Leitner, Xin Long, Neal Cardwell, Joerg Reuter,
Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
Oliver Hartkopp, Marc Kleine-Budde, Robin van der Gracht,
Oleksij Rempel, kernel, Alexander Aring, Stefan Schmidt,
Miquel Raynal, Alexandra Winter, Thorsten Winkler, James Chapman,
Jeremy Kerr, Matt Johnston, Matthieu Baerts, Mat Martineau,
Geliang Tang, Krzysztof Kozlowski, Remi Denis-Courmont,
Allison Henderson, David Howells, Marc Dionne, Wenjia Zhang,
Jan Karcher, D. Wythe, Tony Lu, Wen Gu, Jon Maloy, Boris Pismenny,
John Fastabend, Stefano Garzarella, Martin Schiller,
Björn Töpel, Magnus Karlsson, Maciej Fijalkowski,
Jonathan Lemon, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, netdev, linux-kernel, linux-sctp,
linux-hams, linux-bluetooth, linux-can, dccp, linux-wpan,
linux-s390, mptcp, linux-rdma, rds-devel, linux-afs,
tipc-discussion, virtualization, linux-x25, bpf, isdn4linux,
io-uring
In-Reply-To: <ed2038b1-0331-43d6-ac15-fd7e004ab27e@samba.org>
On Tue, Apr 01, 2025 at 03:48:58PM +0200, Stefan Metzmacher wrote:
> Am 01.04.25 um 15:37 schrieb Stefan Metzmacher:
> > Am 01.04.25 um 10:19 schrieb Stefan Metzmacher:
> > > Am 31.03.25 um 23:04 schrieb Stanislav Fomichev:
> > > > On 03/31, Stefan Metzmacher wrote:
> > > > > The motivation for this is to remove the SOL_SOCKET limitation
> > > > > from io_uring_cmd_getsockopt().
> > > > >
> > > > > The reason for this limitation is that io_uring_cmd_getsockopt()
> > > > > passes a kernel pointer as optlen to do_sock_getsockopt()
> > > > > and can't reach the ops->getsockopt() path.
> > > > >
> > > > > The first idea would be to change the optval and optlen arguments
> > > > > to the protocol specific hooks also to sockptr_t, as that
> > > > > is already used for setsockopt() and also by do_sock_getsockopt()
> > > > > sk_getsockopt() and BPF_CGROUP_RUN_PROG_GETSOCKOPT().
> > > > >
> > > > > But as Linus don't like 'sockptr_t' I used a different approach.
> > > > >
> > > > > @Linus, would that optlen_t approach fit better for you?
> > > >
> > > > [..]
> > > >
> > > > > Instead of passing the optlen as user or kernel pointer,
> > > > > we only ever pass a kernel pointer and do the
> > > > > translation from/to userspace in do_sock_getsockopt().
> > > >
> > > > At this point why not just fully embrace iov_iter? You have the size
> > > > now + the user (or kernel) pointer. Might as well do
> > > > s/sockptr_t/iov_iter/ conversion?
> > >
> > > I think that would only be possible if we introduce
> > > proto[_ops].getsockopt_iter() and then convert the implementations
> > > step by step. Doing it all in one go has a lot of potential to break
> > > the uapi. I could try to convert things like socket, ip and tcp myself, but
> > > the rest needs to be converted by the maintainer of the specific protocol,
> > > as it needs to be tested. As there are crazy things happening in the existing
> > > implementations, e.g. some getsockopt() implementations use optval as in and out
> > > buffer.
> > >
> > > I first tried to convert both optval and optlen of getsockopt to sockptr_t,
> > > and that showed that touching the optval part starts to get complex very soon,
> > > see https://git.samba.org/?p=metze/linux/wip.git;a=commitdiff;h=141912166473bf8843ec6ace76dc9c6945adafd1
> > > (note it didn't converted everything, I gave up after hitting
> > > sctp_getsockopt_peer_addrs and sctp_getsockopt_local_addrs.
> > > sctp_getsockopt_context, sctp_getsockopt_maxseg, sctp_getsockopt_associnfo and maybe
> > > more are the ones also doing both copy_from_user and copy_to_user on optval)
> > >
> > > I come also across one implementation that returned -ERANGE because *optlen was
> > > too short and put the required length into *optlen, which means the returned
> > > *optlen is larger than the optval buffer given from userspace.
> > >
> > > Because of all these strange things I tried to do a minimal change
> > > in order to get rid of the io_uring limitation and only converted
> > > optlen and leave optval as is.
> > >
> > > In order to have a patchset that has a low risk to cause regressions.
> > >
> > > But as alternative introducing a prototype like this:
> > >
> > > int (*getsockopt_iter)(struct socket *sock, int level, int optname,
> > > struct iov_iter *optval_iter);
> > >
> > > That returns a non-negative value which can be placed into *optlen
> > > or negative value as error and *optlen will not be changed on error.
> > > optval_iter will get direction ITER_DEST, so it can only be written to.
> > >
> > > Implementations could then opt in for the new interface and
> > > allow do_sock_getsockopt() work also for the io_uring case,
> > > while all others would still get -EOPNOTSUPP.
> > >
> > > So what should be the way to go?
> >
> > Ok, I've added the infrastructure for getsockopt_iter, see below,
> > but the first part I wanted to convert was
> > tcp_ao_copy_mkts_to_user() and that also reads from userspace before
> > writing.
> >
> > So we could go with the optlen_t approach, or we need
> > logic for ITER_BOTH or pass two iov_iters one with ITER_SRC and one
> > with ITER_DEST...
> >
> > So who wants to decide?
>
> I just noticed that it's even possible in same cases
> to pass in a short buffer to optval, but have a longer value in optlen,
> hci_sock_getsockopt() with SOL_BLUETOOTH completely ignores optlen.
>
> This makes it really hard to believe that trying to use iov_iter for this
> is a good idea :-(
That was my finding as well a while ago, when I was planning to get the
__user pointers converted to iov_iter. There are some weird ways of
using optlen and optval, which makes them non-trivial to covert to
iov_iter.
^ permalink raw reply
* RE: Bluetooth: qca: fix NV variant for one of WCN3950 SoCs
From: bluez.test.bot @ 2025-04-01 15:34 UTC (permalink / raw)
To: linux-bluetooth, dmitry.baryshkov
In-Reply-To: <20250401-fix-rb1-bt-v1-1-0d140c583a06@oss.qualcomm.com>
[-- Attachment #1: Type: text/plain, Size: 2029 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=948999
---Test result---
Test Summary:
CheckPatch PENDING 0.28 seconds
GitLint PENDING 0.20 seconds
SubjectPrefix PASS 0.13 seconds
BuildKernel PASS 24.59 seconds
CheckAllWarning PASS 26.75 seconds
CheckSparse PASS 29.99 seconds
BuildKernel32 PASS 23.96 seconds
TestRunnerSetup PASS 430.98 seconds
TestRunner_l2cap-tester PASS 21.07 seconds
TestRunner_iso-tester PASS 33.11 seconds
TestRunner_bnep-tester PASS 5.63 seconds
TestRunner_mgmt-tester FAIL 120.89 seconds
TestRunner_rfcomm-tester PASS 7.87 seconds
TestRunner_sco-tester PASS 12.49 seconds
TestRunner_ioctl-tester PASS 8.33 seconds
TestRunner_mesh-tester PASS 6.05 seconds
TestRunner_smp-tester PASS 7.20 seconds
TestRunner_userchan-tester PASS 4.95 seconds
IncrementalBuild PENDING 0.54 seconds
Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:
##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 490, Passed: 484 (98.8%), Failed: 2, Not Run: 4
Failed Test Cases
LL Privacy - Set Flags 3 (2 Devices to RL) Failed 0.169 seconds
LL Privacy - Set Device Flag 1 (Device Privacy) Failed 0.142 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:
---
Regards,
Linux Bluetooth
^ permalink raw reply
* Re: [PATCH] Bluetooth: Add driver command BTUSB_DRV_CMD_SWITCH_ALT_SETTING
From: Hsin-chen Chuang @ 2025-04-01 15:26 UTC (permalink / raw)
To: Luiz Augusto von Dentz
Cc: Hsin-chen Chuang, chromeos-bluetooth-upstreaming, David S. Miller,
Eric Dumazet, Jakub Kicinski, Johan Hedberg, Marcel Holtmann,
Paolo Abeni, Simon Horman, Ying Hsu, linux-bluetooth,
linux-kernel, netdev
In-Reply-To: <CABBYNZKu_jRm4b-gBT=DRtn0c_svgxyM7tc_u3HDRCUAwvABnQ@mail.gmail.com>
Hi Luiz,
On Tue, Apr 1, 2025 at 11:03 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Hsin-chen,
>
> On Tue, Apr 1, 2025 at 9:44 AM Hsin-chen Chuang <chharry@google.com> wrote:
> >
> > From: Hsin-chen Chuang <chharry@chromium.org>
> >
> > Although commit 75ddcd5ad40e ("Bluetooth: btusb: Configure altsetting
> > for HCI_USER_CHANNEL") has enabled the HCI_USER_CHANNEL user to send out
> > SCO data through USB Bluetooth chips, it's observed that with the patch
> > HFP is flaky on most of the existing USB Bluetooth controllers: Intel
> > chips sometimes send out no packet for Transparent codec; MTK chips may
> > generate SCO data with a wrong handle for CVSD codec; RTK could split
> > the data with a wrong packet size for Transparent codec; ... etc.
> >
> > To address the issue above one needs to reset the altsetting back to
> > zero when there is no active SCO connection, which is the same as the
> > BlueZ behavior, and another benefit is the bus doesn't need to reserve
> > bandwidth when no SCO connection.
> >
> > This patch introduces a fundamental solution that lets the user space
> > program to configure the altsetting freely:
> > - Define the new packet type HCI_DRV_PKT which is specifically used for
> > communication between the user space program and the Bluetooth drviers
> > - Define the btusb driver command BTUSB_DRV_CMD_SWITCH_ALT_SETTING which
> > indicates the expected altsetting from the user space program
> > - btusb intercepts the command and adjusts the Isoc endpoint
> > correspondingly
> >
> > This patch is tested on ChromeOS devices. The USB Bluetooth models
> > (CVSD, TRANS alt3, and TRANS alt6) could pass the stress HFP test narrow
> > band speech and wide band speech.
> >
> > Cc: chromeos-bluetooth-upstreaming@chromium.org
> > Fixes: b16b327edb4d ("Bluetooth: btusb: add sysfs attribute to control USB alt setting")
> > Signed-off-by: Hsin-chen Chuang <chharry@chromium.org>
> > ---
> >
> > drivers/bluetooth/btusb.c | 67 +++++++++++++++++++++++++++++++++
> > include/net/bluetooth/hci.h | 1 +
> > include/net/bluetooth/hci_mon.h | 2 +
> > net/bluetooth/hci_core.c | 2 +
> > net/bluetooth/hci_sock.c | 14 +++++--
> > 5 files changed, 83 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index 5012b5ff92c8..a7bc64e86661 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -2151,6 +2151,67 @@ static int submit_or_queue_tx_urb(struct hci_dev *hdev, struct urb *urb)
> > return 0;
> > }
> >
> > +static struct sk_buff *btusb_drv_response(u8 opcode, size_t data_len)
> > +{
> > + struct sk_buff *skb;
> > +
> > + /* btusb driver response starts with 1 oct of the opcode,
> > + * and followed by the command specific data.
> > + */
> > + skb = bt_skb_alloc(1 + data_len, GFP_KERNEL);
> > + if (!skb)
> > + return NULL;
> > +
> > + skb_put_u8(skb, opcode);
> > + hci_skb_pkt_type(skb) = HCI_DRV_PKT;
> > +
> > + return skb;
> > +}
> > +
> > +static int btusb_switch_alt_setting(struct hci_dev *hdev, int new_alts);
> > +
> > +#define BTUSB_DRV_CMD_SWITCH_ALT_SETTING 0x35
>
> Any particular reason why you are starting with 0x35? We may need to
> add something like Read Supported Driver Commands to begin with.
Um, it's just my lucky number. No particular reason in terms of the design.
And sure Read Supported Driver Commands seems good, but does that
indicate that the meaning of the opcodes is shared across different
driver modules? That's fine but we would need to move these
definitions somewhere else.
>
> > +static int btusb_drv_cmd(struct hci_dev *hdev, struct sk_buff *skb)
> > +{
> > + /* btusb driver command starts with 1 oct of the opcode,
> > + * and followed by the command specific data.
> > + */
> > + if (!skb->len)
> > + return -EILSEQ;
>
> We might need to define a struct header, and I'd definitely recommend
> using skb_pull_data for parsing.
So far the header has only 1 oct of the opcode. Would you suggest
something similar to HCI command e.g. 2 oct of the opcode + 1 oct of
the param length?
>
> > + switch (skb->data[0]) {
> > + case BTUSB_DRV_CMD_SWITCH_ALT_SETTING: {
> > + struct sk_buff *resp;
> > + int status;
> > +
> > + /* Response data: Total 1 Oct
> > + * Status: 1 Oct
> > + * 0 = Success
> > + * 1 = Invalid command
> > + * 2 = Other errors
> > + */
> > + resp = btusb_drv_response(BTUSB_DRV_CMD_SWITCH_ALT_SETTING, 1);
> > + if (!resp)
> > + return -ENOMEM;
> > +
> > + if (skb->len != 2 || skb->data[1] > 6) {
> > + status = 1;
> > + } else {
> > + status = btusb_switch_alt_setting(hdev, skb->data[1]);
> > + if (status)
> > + status = 2;
> > + }
> > + skb_put_u8(resp, status);
> > +
> > + kfree_skb(skb);
> > + return hci_recv_frame(hdev, resp);
> > + }
> > + }
> > +
> > + return -EILSEQ;
> > +}
> > +
> > static int btusb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
> > {
> > struct urb *urb;
> > @@ -2192,6 +2253,9 @@ static int btusb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
> > return PTR_ERR(urb);
> >
> > return submit_or_queue_tx_urb(hdev, urb);
> > +
> > + case HCI_DRV_PKT:
> > + return btusb_drv_cmd(hdev, skb);
> > }
> >
> > return -EILSEQ;
> > @@ -2669,6 +2733,9 @@ static int btusb_send_frame_intel(struct hci_dev *hdev, struct sk_buff *skb)
> > return PTR_ERR(urb);
> >
> > return submit_or_queue_tx_urb(hdev, urb);
> > +
> > + case HCI_DRV_PKT:
> > + return btusb_drv_cmd(hdev, skb);
> > }
> >
> > return -EILSEQ;
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index a8586c3058c7..e297b312d2b7 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -494,6 +494,7 @@ enum {
> > #define HCI_EVENT_PKT 0x04
> > #define HCI_ISODATA_PKT 0x05
> > #define HCI_DIAG_PKT 0xf0
> > +#define HCI_DRV_PKT 0xf1
> > #define HCI_VENDOR_PKT 0xff
> >
> > /* HCI packet types */
> > diff --git a/include/net/bluetooth/hci_mon.h b/include/net/bluetooth/hci_mon.h
> > index 082f89531b88..bbd752494ef9 100644
> > --- a/include/net/bluetooth/hci_mon.h
> > +++ b/include/net/bluetooth/hci_mon.h
> > @@ -51,6 +51,8 @@ struct hci_mon_hdr {
> > #define HCI_MON_CTRL_EVENT 17
> > #define HCI_MON_ISO_TX_PKT 18
> > #define HCI_MON_ISO_RX_PKT 19
> > +#define HCI_MON_DRV_TX_PKT 20
> > +#define HCI_MON_DRV_RX_PKT 21
>
> Are you planning to write some btmon decoding for these packets?
Yeah, I believe this is helpful for debugging. ChromeOS still uses
btmon for btsnoop capturing.
>
> > struct hci_mon_new_index {
> > __u8 type;
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index 5eb0600bbd03..bb4e1721edc2 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -2911,6 +2911,8 @@ int hci_recv_frame(struct hci_dev *hdev, struct sk_buff *skb)
> > break;
> > case HCI_ISODATA_PKT:
> > break;
> > + case HCI_DRV_PKT:
> > + break;
> > default:
> > kfree_skb(skb);
> > return -EINVAL;
> > diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
> > index 022b86797acd..0bc4f77ed17b 100644
> > --- a/net/bluetooth/hci_sock.c
> > +++ b/net/bluetooth/hci_sock.c
> > @@ -234,7 +234,8 @@ void hci_send_to_sock(struct hci_dev *hdev, struct sk_buff *skb)
> > if (hci_skb_pkt_type(skb) != HCI_EVENT_PKT &&
> > hci_skb_pkt_type(skb) != HCI_ACLDATA_PKT &&
> > hci_skb_pkt_type(skb) != HCI_SCODATA_PKT &&
> > - hci_skb_pkt_type(skb) != HCI_ISODATA_PKT)
> > + hci_skb_pkt_type(skb) != HCI_ISODATA_PKT &&
> > + hci_skb_pkt_type(skb) != HCI_DRV_PKT)
> > continue;
> > } else {
> > /* Don't send frame to other channel types */
> > @@ -391,6 +392,12 @@ void hci_send_to_monitor(struct hci_dev *hdev, struct sk_buff *skb)
> > else
> > opcode = cpu_to_le16(HCI_MON_ISO_TX_PKT);
> > break;
> > + case HCI_DRV_PKT:
> > + if (bt_cb(skb)->incoming)
> > + opcode = cpu_to_le16(HCI_MON_DRV_RX_PKT);
> > + else
> > + opcode = cpu_to_le16(HCI_MON_DRV_TX_PKT);
> > + break;
> > case HCI_DIAG_PKT:
> > opcode = cpu_to_le16(HCI_MON_VENDOR_DIAG);
> > break;
> > @@ -1806,7 +1813,7 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg,
> > if (flags & ~(MSG_DONTWAIT | MSG_NOSIGNAL | MSG_ERRQUEUE | MSG_CMSG_COMPAT))
> > return -EINVAL;
> >
> > - if (len < 4 || len > hci_pi(sk)->mtu)
> > + if (len > hci_pi(sk)->mtu)
> > return -EINVAL;
> >
> > skb = bt_skb_sendmsg(sk, msg, len, len, 0, 0);
> > @@ -1860,7 +1867,8 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg,
> > if (hci_skb_pkt_type(skb) != HCI_COMMAND_PKT &&
> > hci_skb_pkt_type(skb) != HCI_ACLDATA_PKT &&
> > hci_skb_pkt_type(skb) != HCI_SCODATA_PKT &&
> > - hci_skb_pkt_type(skb) != HCI_ISODATA_PKT) {
> > + hci_skb_pkt_type(skb) != HCI_ISODATA_PKT &&
> > + hci_skb_pkt_type(skb) != HCI_DRV_PKT) {
> > err = -EINVAL;
> > goto drop;
> > }
> > --
> > 2.49.0.472.ge94155a9ec-goog
> >
>
>
> --
> Luiz Augusto von Dentz
--
Best Regards,
Hsin-chen
^ permalink raw reply
* RE: [BlueZ,v2] shared/bap: Fix swallowing states transitions
From: bluez.test.bot @ 2025-04-01 15:11 UTC (permalink / raw)
To: linux-bluetooth, luiz.dentz
In-Reply-To: <20250401135115.3735533-1-luiz.dentz@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1863 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=948958
---Test result---
Test Summary:
CheckPatch PENDING 0.27 seconds
GitLint PENDING 0.26 seconds
BuildEll PASS 20.49 seconds
BluezMake PASS 1498.49 seconds
MakeCheck PASS 13.27 seconds
MakeDistcheck PASS 157.52 seconds
CheckValgrind PASS 217.93 seconds
CheckSmatch WARNING 285.93 seconds
bluezmakeextell PASS 98.39 seconds
IncrementalBuild PENDING 0.25 seconds
ScanBuild PASS 869.65 seconds
Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:
##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
src/shared/bap.c:314:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:314:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:314:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structures
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:
---
Regards,
Linux Bluetooth
^ permalink raw reply
* RE: [BlueZ,v1] dbus: Fix condition for invalidating path
From: bluez.test.bot @ 2025-04-01 15:11 UTC (permalink / raw)
To: linux-bluetooth, luiz.dentz
In-Reply-To: <20250401134904.3721187-1-luiz.dentz@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1260 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=948957
---Test result---
Test Summary:
CheckPatch PENDING 0.39 seconds
GitLint PENDING 0.32 seconds
BuildEll PASS 20.49 seconds
BluezMake PASS 1501.79 seconds
MakeCheck PASS 12.93 seconds
MakeDistcheck PASS 157.66 seconds
CheckValgrind PASS 216.69 seconds
CheckSmatch PASS 283.78 seconds
bluezmakeextell PASS 97.95 seconds
IncrementalBuild PENDING 0.34 seconds
ScanBuild PASS 868.85 seconds
Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:
##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:
---
Regards,
Linux Bluetooth
^ permalink raw reply
* [PATCH] Bluetooth: qca: fix NV variant for one of WCN3950 SoCs
From: Dmitry Baryshkov @ 2025-04-01 15:04 UTC (permalink / raw)
To: Marcel Holtmann, Luiz Augusto von Dentz, Dmitry Baryshkov
Cc: Luiz Augusto von Dentz, linux-bluetooth, linux-kernel,
Wojciech Slenska
The QCA_WCN3950_SOC_ID_S should be using qca/cmnv13s.bin, rather than
qca/cmnv13u.bin file. Correct the variant suffix to be used for this SoC
ID.
Fixes: 3b0e0839d9f2 ("Bluetooth: qca: add WCN3950 support")
Reported-by: Wojciech Slenska <wsl@trackunit.com>
Closes: https://github.com/qualcomm-linux/meta-qcom/pull/817#discussion_r2022866431
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/bluetooth/btqca.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index 3d6778b95e0058beda3f0500b21caaef8e797d32..edefb9dc76aa1a019dc1e0ae06f4545b4ad3f96a 100644
--- a/drivers/bluetooth/btqca.c
+++ b/drivers/bluetooth/btqca.c
@@ -889,7 +889,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
if (le32_to_cpu(ver.soc_id) == QCA_WCN3950_SOC_ID_T)
variant = "t";
else if (le32_to_cpu(ver.soc_id) == QCA_WCN3950_SOC_ID_S)
- variant = "u";
+ variant = "s";
snprintf(config.fwname, sizeof(config.fwname),
"qca/cmnv%02x%s.bin", rom_ver, variant);
---
base-commit: eb4bc4b07f66f01618d9cb1aa4eaef59b1188415
change-id: 20250401-fix-rb1-bt-8b2f4dd7b711
Best regards,
--
Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
^ permalink raw reply related
* Re: [PATCH] Bluetooth: Add driver command BTUSB_DRV_CMD_SWITCH_ALT_SETTING
From: Luiz Augusto von Dentz @ 2025-04-01 15:02 UTC (permalink / raw)
To: Hsin-chen Chuang
Cc: Hsin-chen Chuang, chromeos-bluetooth-upstreaming, David S. Miller,
Eric Dumazet, Jakub Kicinski, Johan Hedberg, Marcel Holtmann,
Paolo Abeni, Simon Horman, Ying Hsu, linux-bluetooth,
linux-kernel, netdev
In-Reply-To: <20250401134424.3725875-1-chharry@google.com>
Hi Hsin-chen,
On Tue, Apr 1, 2025 at 9:44 AM Hsin-chen Chuang <chharry@google.com> wrote:
>
> From: Hsin-chen Chuang <chharry@chromium.org>
>
> Although commit 75ddcd5ad40e ("Bluetooth: btusb: Configure altsetting
> for HCI_USER_CHANNEL") has enabled the HCI_USER_CHANNEL user to send out
> SCO data through USB Bluetooth chips, it's observed that with the patch
> HFP is flaky on most of the existing USB Bluetooth controllers: Intel
> chips sometimes send out no packet for Transparent codec; MTK chips may
> generate SCO data with a wrong handle for CVSD codec; RTK could split
> the data with a wrong packet size for Transparent codec; ... etc.
>
> To address the issue above one needs to reset the altsetting back to
> zero when there is no active SCO connection, which is the same as the
> BlueZ behavior, and another benefit is the bus doesn't need to reserve
> bandwidth when no SCO connection.
>
> This patch introduces a fundamental solution that lets the user space
> program to configure the altsetting freely:
> - Define the new packet type HCI_DRV_PKT which is specifically used for
> communication between the user space program and the Bluetooth drviers
> - Define the btusb driver command BTUSB_DRV_CMD_SWITCH_ALT_SETTING which
> indicates the expected altsetting from the user space program
> - btusb intercepts the command and adjusts the Isoc endpoint
> correspondingly
>
> This patch is tested on ChromeOS devices. The USB Bluetooth models
> (CVSD, TRANS alt3, and TRANS alt6) could pass the stress HFP test narrow
> band speech and wide band speech.
>
> Cc: chromeos-bluetooth-upstreaming@chromium.org
> Fixes: b16b327edb4d ("Bluetooth: btusb: add sysfs attribute to control USB alt setting")
> Signed-off-by: Hsin-chen Chuang <chharry@chromium.org>
> ---
>
> drivers/bluetooth/btusb.c | 67 +++++++++++++++++++++++++++++++++
> include/net/bluetooth/hci.h | 1 +
> include/net/bluetooth/hci_mon.h | 2 +
> net/bluetooth/hci_core.c | 2 +
> net/bluetooth/hci_sock.c | 14 +++++--
> 5 files changed, 83 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 5012b5ff92c8..a7bc64e86661 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -2151,6 +2151,67 @@ static int submit_or_queue_tx_urb(struct hci_dev *hdev, struct urb *urb)
> return 0;
> }
>
> +static struct sk_buff *btusb_drv_response(u8 opcode, size_t data_len)
> +{
> + struct sk_buff *skb;
> +
> + /* btusb driver response starts with 1 oct of the opcode,
> + * and followed by the command specific data.
> + */
> + skb = bt_skb_alloc(1 + data_len, GFP_KERNEL);
> + if (!skb)
> + return NULL;
> +
> + skb_put_u8(skb, opcode);
> + hci_skb_pkt_type(skb) = HCI_DRV_PKT;
> +
> + return skb;
> +}
> +
> +static int btusb_switch_alt_setting(struct hci_dev *hdev, int new_alts);
> +
> +#define BTUSB_DRV_CMD_SWITCH_ALT_SETTING 0x35
Any particular reason why you are starting with 0x35? We may need to
add something like Read Supported Driver Commands to begin with.
> +static int btusb_drv_cmd(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> + /* btusb driver command starts with 1 oct of the opcode,
> + * and followed by the command specific data.
> + */
> + if (!skb->len)
> + return -EILSEQ;
We might need to define a struct header, and I'd definitely recommend
using skb_pull_data for parsing.
> + switch (skb->data[0]) {
> + case BTUSB_DRV_CMD_SWITCH_ALT_SETTING: {
> + struct sk_buff *resp;
> + int status;
> +
> + /* Response data: Total 1 Oct
> + * Status: 1 Oct
> + * 0 = Success
> + * 1 = Invalid command
> + * 2 = Other errors
> + */
> + resp = btusb_drv_response(BTUSB_DRV_CMD_SWITCH_ALT_SETTING, 1);
> + if (!resp)
> + return -ENOMEM;
> +
> + if (skb->len != 2 || skb->data[1] > 6) {
> + status = 1;
> + } else {
> + status = btusb_switch_alt_setting(hdev, skb->data[1]);
> + if (status)
> + status = 2;
> + }
> + skb_put_u8(resp, status);
> +
> + kfree_skb(skb);
> + return hci_recv_frame(hdev, resp);
> + }
> + }
> +
> + return -EILSEQ;
> +}
> +
> static int btusb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
> {
> struct urb *urb;
> @@ -2192,6 +2253,9 @@ static int btusb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
> return PTR_ERR(urb);
>
> return submit_or_queue_tx_urb(hdev, urb);
> +
> + case HCI_DRV_PKT:
> + return btusb_drv_cmd(hdev, skb);
> }
>
> return -EILSEQ;
> @@ -2669,6 +2733,9 @@ static int btusb_send_frame_intel(struct hci_dev *hdev, struct sk_buff *skb)
> return PTR_ERR(urb);
>
> return submit_or_queue_tx_urb(hdev, urb);
> +
> + case HCI_DRV_PKT:
> + return btusb_drv_cmd(hdev, skb);
> }
>
> return -EILSEQ;
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index a8586c3058c7..e297b312d2b7 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -494,6 +494,7 @@ enum {
> #define HCI_EVENT_PKT 0x04
> #define HCI_ISODATA_PKT 0x05
> #define HCI_DIAG_PKT 0xf0
> +#define HCI_DRV_PKT 0xf1
> #define HCI_VENDOR_PKT 0xff
>
> /* HCI packet types */
> diff --git a/include/net/bluetooth/hci_mon.h b/include/net/bluetooth/hci_mon.h
> index 082f89531b88..bbd752494ef9 100644
> --- a/include/net/bluetooth/hci_mon.h
> +++ b/include/net/bluetooth/hci_mon.h
> @@ -51,6 +51,8 @@ struct hci_mon_hdr {
> #define HCI_MON_CTRL_EVENT 17
> #define HCI_MON_ISO_TX_PKT 18
> #define HCI_MON_ISO_RX_PKT 19
> +#define HCI_MON_DRV_TX_PKT 20
> +#define HCI_MON_DRV_RX_PKT 21
Are you planning to write some btmon decoding for these packets?
> struct hci_mon_new_index {
> __u8 type;
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 5eb0600bbd03..bb4e1721edc2 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2911,6 +2911,8 @@ int hci_recv_frame(struct hci_dev *hdev, struct sk_buff *skb)
> break;
> case HCI_ISODATA_PKT:
> break;
> + case HCI_DRV_PKT:
> + break;
> default:
> kfree_skb(skb);
> return -EINVAL;
> diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
> index 022b86797acd..0bc4f77ed17b 100644
> --- a/net/bluetooth/hci_sock.c
> +++ b/net/bluetooth/hci_sock.c
> @@ -234,7 +234,8 @@ void hci_send_to_sock(struct hci_dev *hdev, struct sk_buff *skb)
> if (hci_skb_pkt_type(skb) != HCI_EVENT_PKT &&
> hci_skb_pkt_type(skb) != HCI_ACLDATA_PKT &&
> hci_skb_pkt_type(skb) != HCI_SCODATA_PKT &&
> - hci_skb_pkt_type(skb) != HCI_ISODATA_PKT)
> + hci_skb_pkt_type(skb) != HCI_ISODATA_PKT &&
> + hci_skb_pkt_type(skb) != HCI_DRV_PKT)
> continue;
> } else {
> /* Don't send frame to other channel types */
> @@ -391,6 +392,12 @@ void hci_send_to_monitor(struct hci_dev *hdev, struct sk_buff *skb)
> else
> opcode = cpu_to_le16(HCI_MON_ISO_TX_PKT);
> break;
> + case HCI_DRV_PKT:
> + if (bt_cb(skb)->incoming)
> + opcode = cpu_to_le16(HCI_MON_DRV_RX_PKT);
> + else
> + opcode = cpu_to_le16(HCI_MON_DRV_TX_PKT);
> + break;
> case HCI_DIAG_PKT:
> opcode = cpu_to_le16(HCI_MON_VENDOR_DIAG);
> break;
> @@ -1806,7 +1813,7 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg,
> if (flags & ~(MSG_DONTWAIT | MSG_NOSIGNAL | MSG_ERRQUEUE | MSG_CMSG_COMPAT))
> return -EINVAL;
>
> - if (len < 4 || len > hci_pi(sk)->mtu)
> + if (len > hci_pi(sk)->mtu)
> return -EINVAL;
>
> skb = bt_skb_sendmsg(sk, msg, len, len, 0, 0);
> @@ -1860,7 +1867,8 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg,
> if (hci_skb_pkt_type(skb) != HCI_COMMAND_PKT &&
> hci_skb_pkt_type(skb) != HCI_ACLDATA_PKT &&
> hci_skb_pkt_type(skb) != HCI_SCODATA_PKT &&
> - hci_skb_pkt_type(skb) != HCI_ISODATA_PKT) {
> + hci_skb_pkt_type(skb) != HCI_ISODATA_PKT &&
> + hci_skb_pkt_type(skb) != HCI_DRV_PKT) {
> err = -EINVAL;
> goto drop;
> }
> --
> 2.49.0.472.ge94155a9ec-goog
>
--
Luiz Augusto von Dentz
^ permalink raw reply
* RE: Bluetooth: Add driver command BTUSB_DRV_CMD_SWITCH_ALT_SETTING
From: bluez.test.bot @ 2025-04-01 14:33 UTC (permalink / raw)
To: linux-bluetooth, chharry
In-Reply-To: <20250401134424.3725875-1-chharry@google.com>
[-- Attachment #1: Type: text/plain, Size: 2218 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=948951
---Test result---
Test Summary:
CheckPatch PENDING 0.40 seconds
GitLint PENDING 0.26 seconds
SubjectPrefix PASS 0.11 seconds
BuildKernel PASS 24.01 seconds
CheckAllWarning PASS 26.43 seconds
CheckSparse PASS 30.05 seconds
BuildKernel32 PASS 24.08 seconds
TestRunnerSetup PASS 428.46 seconds
TestRunner_l2cap-tester FAIL 44.90 seconds
TestRunner_iso-tester PASS 34.68 seconds
TestRunner_bnep-tester PASS 4.73 seconds
TestRunner_mgmt-tester FAIL 119.51 seconds
TestRunner_rfcomm-tester PASS 7.76 seconds
TestRunner_sco-tester PASS 12.47 seconds
TestRunner_ioctl-tester PASS 12.65 seconds
TestRunner_mesh-tester PASS 6.04 seconds
TestRunner_smp-tester PASS 7.09 seconds
TestRunner_userchan-tester PASS 4.93 seconds
IncrementalBuild PENDING 0.50 seconds
Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:
##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:
##############################
Test: TestRunner_l2cap-tester - FAIL
Desc: Run l2cap-tester with test-runner
Output:
Total: 62, Passed: 61 (98.4%), Failed: 1, Not Run: 0
Failed Test Cases
L2CAP LE Client - Close socket 2 Timed out 23.647 seconds
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 490, Passed: 485 (99.0%), Failed: 1, Not Run: 4
Failed Test Cases
LL Privacy - Set Device Flag 1 (Device Privacy) Failed 0.154 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:
---
Regards,
Linux Bluetooth
^ permalink raw reply
* [bluez/bluez]
From: BluezTestBot @ 2025-04-01 14:16 UTC (permalink / raw)
To: linux-bluetooth
Branch: refs/tags/5.81
Home: https://github.com/bluez/bluez
To unsubscribe from these emails, change your notification settings at https://github.com/bluez/bluez/settings/notifications
^ permalink raw reply
* [bluez/bluez] 47e5d3: Release 5.81
From: Marcel Holtmann @ 2025-04-01 14:14 UTC (permalink / raw)
To: linux-bluetooth
Branch: refs/heads/master
Home: https://github.com/bluez/bluez
Commit: 47e5d3491d373944c82704b48b3005443807fb40
https://github.com/bluez/bluez/commit/47e5d3491d373944c82704b48b3005443807fb40
Author: Marcel Holtmann <marcel@holtmann.org>
Date: 2025-04-01 (Tue, 01 Apr 2025)
Changed paths:
M ChangeLog
M configure.ac
Log Message:
-----------
Release 5.81
To unsubscribe from these emails, change your notification settings at https://github.com/bluez/bluez/settings/notifications
^ permalink raw reply
* [PATCH BlueZ v2] shared/bap: Fix swallowing states transitions
From: Luiz Augusto von Dentz @ 2025-04-01 13:51 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
In certain cases (e.g ASCS_Enable) a Control Point operation may change
states multiple times causing states not to be notified.
To fix this attempts to queue states if timeout is pending (state_id)
and then proceed to notify them ahead of the last state set in the ASE
so the remote side is informed of all the state transitions.
---
src/shared/bap.c | 39 ++++++++++++++++++++++++++++-----------
1 file changed, 28 insertions(+), 11 deletions(-)
diff --git a/src/shared/bap.c b/src/shared/bap.c
index 5d4d69d2925b..650bea2f4e8d 100644
--- a/src/shared/bap.c
+++ b/src/shared/bap.c
@@ -293,6 +293,7 @@ struct bt_bap_stream {
uint8_t old_state;
uint8_t state;
unsigned int state_id;
+ struct queue *pending_states;
bool client;
void *user_data;
};
@@ -1205,6 +1206,7 @@ static void bap_stream_free(void *data)
struct bt_bap_stream *stream = data;
timeout_remove(stream->state_id);
+ queue_destroy(stream->pending_states, NULL);
if (stream->ep)
stream->ep->stream = NULL;
@@ -1705,19 +1707,11 @@ static bool bap_queue_req(struct bt_bap *bap, struct bt_bap_req *req)
return true;
}
-static bool stream_notify_state(void *data)
+static void stream_notify(struct bt_bap_stream *stream, uint8_t state)
{
- struct bt_bap_stream *stream = data;
- struct bt_bap_endpoint *ep = stream->ep;
+ DBG(stream->bap, "stream %p state %d", stream, state);
- DBG(stream->bap, "stream %p status %d", stream, ep->state);
-
- if (stream->state_id) {
- timeout_remove(stream->state_id);
- stream->state_id = 0;
- }
-
- switch (ep->state) {
+ switch (state) {
case BT_ASCS_ASE_STATE_IDLE:
break;
case BT_ASCS_ASE_STATE_CONFIG:
@@ -1735,6 +1729,24 @@ static bool stream_notify_state(void *data)
stream_notify_release(stream);
break;
}
+}
+
+static bool stream_notify_state(void *data)
+{
+ struct bt_bap_stream *stream = data;
+ struct bt_bap_endpoint *ep = stream->ep;
+ uint8_t state;
+
+ if (stream->state_id) {
+ timeout_remove(stream->state_id);
+ stream->state_id = 0;
+ }
+
+ /* Notify any pending states before notifying ep->state */
+ while ((state = PTR_TO_UINT(queue_pop_head(stream->pending_states))))
+ stream_notify(stream, state);
+
+ stream_notify(stream, ep->state);
return false;
}
@@ -1760,6 +1772,10 @@ static void bap_ucast_set_state(struct bt_bap_stream *stream, uint8_t state)
stream->state_id = timeout_add(BAP_PROCESS_TIMEOUT,
stream_notify_state,
stream, NULL);
+ else /* If a state_id is already pending then queue the old one */
+ queue_push_tail(stream->pending_states,
+ UINT_TO_PTR(ep->old_state));
+
done:
bap_stream_state_changed(stream);
@@ -2716,6 +2732,7 @@ static struct bt_bap_stream *bap_stream_new(struct bt_bap *bap,
stream->cc = util_iov_dup(data, 1);
stream->client = client;
stream->ops = bap_stream_new_ops(stream);
+ stream->pending_states = queue_new();
queue_push_tail(bap->streams, stream);
--
2.48.1
^ permalink raw reply related
* [PATCH BlueZ v1] dbus: Fix condition for invalidating path
From: Luiz Augusto von Dentz @ 2025-04-01 13:49 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This fixes the condition introduced in cdd02afbb7ef
("dbus: Fix add invalid memory during interface removal") which was
reversed while applying the original fix.
Fixes: https://github.com/bluez/bluez/issues/1155
---
gdbus/object.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gdbus/object.c b/gdbus/object.c
index 54e04b983a98..f8c694aaffdf 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -810,7 +810,7 @@ static struct generic_data *invalidate_parent_data(DBusConnection *conn,
if (child == NULL || g_slist_find(data->objects, child) != NULL)
goto done;
- if (!g_slist_find(parent->objects, child))
+ if (g_slist_find(parent->objects, child))
goto done;
data->objects = g_slist_prepend(data->objects, child);
--
2.48.1
^ permalink raw reply related
* Re: [RFC PATCH 0/4] net/io_uring: pass a kernel pointer via optlen_t to proto[_ops].getsockopt()
From: Stefan Metzmacher @ 2025-04-01 13:48 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Linus Torvalds, Jens Axboe, Pavel Begunkov, Breno Leitao,
Jakub Kicinski, Christoph Hellwig, Karsten Keil, Ayush Sawal,
Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
Simon Horman, Kuniyuki Iwashima, Willem de Bruijn, David Ahern,
Marcelo Ricardo Leitner, Xin Long, Neal Cardwell, Joerg Reuter,
Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
Oliver Hartkopp, Marc Kleine-Budde, Robin van der Gracht,
Oleksij Rempel, kernel, Alexander Aring, Stefan Schmidt,
Miquel Raynal, Alexandra Winter, Thorsten Winkler, James Chapman,
Jeremy Kerr, Matt Johnston, Matthieu Baerts, Mat Martineau,
Geliang Tang, Krzysztof Kozlowski, Remi Denis-Courmont,
Allison Henderson, David Howells, Marc Dionne, Wenjia Zhang,
Jan Karcher, D. Wythe, Tony Lu, Wen Gu, Jon Maloy, Boris Pismenny,
John Fastabend, Stefano Garzarella, Martin Schiller,
Björn Töpel, Magnus Karlsson, Maciej Fijalkowski,
Jonathan Lemon, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, netdev, linux-kernel, linux-sctp,
linux-hams, linux-bluetooth, linux-can, dccp, linux-wpan,
linux-s390, mptcp, linux-rdma, rds-devel, linux-afs,
tipc-discussion, virtualization, linux-x25, bpf, isdn4linux,
io-uring
In-Reply-To: <407c1a05-24a7-430b-958c-0ca78c467c07@samba.org>
Am 01.04.25 um 15:37 schrieb Stefan Metzmacher:
> Am 01.04.25 um 10:19 schrieb Stefan Metzmacher:
>> Am 31.03.25 um 23:04 schrieb Stanislav Fomichev:
>>> On 03/31, Stefan Metzmacher wrote:
>>>> The motivation for this is to remove the SOL_SOCKET limitation
>>>> from io_uring_cmd_getsockopt().
>>>>
>>>> The reason for this limitation is that io_uring_cmd_getsockopt()
>>>> passes a kernel pointer as optlen to do_sock_getsockopt()
>>>> and can't reach the ops->getsockopt() path.
>>>>
>>>> The first idea would be to change the optval and optlen arguments
>>>> to the protocol specific hooks also to sockptr_t, as that
>>>> is already used for setsockopt() and also by do_sock_getsockopt()
>>>> sk_getsockopt() and BPF_CGROUP_RUN_PROG_GETSOCKOPT().
>>>>
>>>> But as Linus don't like 'sockptr_t' I used a different approach.
>>>>
>>>> @Linus, would that optlen_t approach fit better for you?
>>>
>>> [..]
>>>
>>>> Instead of passing the optlen as user or kernel pointer,
>>>> we only ever pass a kernel pointer and do the
>>>> translation from/to userspace in do_sock_getsockopt().
>>>
>>> At this point why not just fully embrace iov_iter? You have the size
>>> now + the user (or kernel) pointer. Might as well do
>>> s/sockptr_t/iov_iter/ conversion?
>>
>> I think that would only be possible if we introduce
>> proto[_ops].getsockopt_iter() and then convert the implementations
>> step by step. Doing it all in one go has a lot of potential to break
>> the uapi. I could try to convert things like socket, ip and tcp myself, but
>> the rest needs to be converted by the maintainer of the specific protocol,
>> as it needs to be tested. As there are crazy things happening in the existing
>> implementations, e.g. some getsockopt() implementations use optval as in and out
>> buffer.
>>
>> I first tried to convert both optval and optlen of getsockopt to sockptr_t,
>> and that showed that touching the optval part starts to get complex very soon,
>> see https://git.samba.org/?p=metze/linux/wip.git;a=commitdiff;h=141912166473bf8843ec6ace76dc9c6945adafd1
>> (note it didn't converted everything, I gave up after hitting
>> sctp_getsockopt_peer_addrs and sctp_getsockopt_local_addrs.
>> sctp_getsockopt_context, sctp_getsockopt_maxseg, sctp_getsockopt_associnfo and maybe
>> more are the ones also doing both copy_from_user and copy_to_user on optval)
>>
>> I come also across one implementation that returned -ERANGE because *optlen was
>> too short and put the required length into *optlen, which means the returned
>> *optlen is larger than the optval buffer given from userspace.
>>
>> Because of all these strange things I tried to do a minimal change
>> in order to get rid of the io_uring limitation and only converted
>> optlen and leave optval as is.
>>
>> In order to have a patchset that has a low risk to cause regressions.
>>
>> But as alternative introducing a prototype like this:
>>
>> int (*getsockopt_iter)(struct socket *sock, int level, int optname,
>> struct iov_iter *optval_iter);
>>
>> That returns a non-negative value which can be placed into *optlen
>> or negative value as error and *optlen will not be changed on error.
>> optval_iter will get direction ITER_DEST, so it can only be written to.
>>
>> Implementations could then opt in for the new interface and
>> allow do_sock_getsockopt() work also for the io_uring case,
>> while all others would still get -EOPNOTSUPP.
>>
>> So what should be the way to go?
>
> Ok, I've added the infrastructure for getsockopt_iter, see below,
> but the first part I wanted to convert was
> tcp_ao_copy_mkts_to_user() and that also reads from userspace before
> writing.
>
> So we could go with the optlen_t approach, or we need
> logic for ITER_BOTH or pass two iov_iters one with ITER_SRC and one
> with ITER_DEST...
>
> So who wants to decide?
I just noticed that it's even possible in same cases
to pass in a short buffer to optval, but have a longer value in optlen,
hci_sock_getsockopt() with SOL_BLUETOOTH completely ignores optlen.
This makes it really hard to believe that trying to use iov_iter for this
is a good idea :-(
Any ideas beside just going with optlen_t?
metze
^ permalink raw reply
* [PATCH BlueZ v1] dbus: Fix condition for invalidating path
From: Luiz Augusto von Dentz @ 2025-04-01 13:48 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This fixes the condition introduced in cdd02afbb7ef
("dbus: Fix add invalid memory during interface removal") which was
reversed while applying the original fix.
---
gdbus/object.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gdbus/object.c b/gdbus/object.c
index 54e04b983a98..f8c694aaffdf 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -810,7 +810,7 @@ static struct generic_data *invalidate_parent_data(DBusConnection *conn,
if (child == NULL || g_slist_find(data->objects, child) != NULL)
goto done;
- if (!g_slist_find(parent->objects, child))
+ if (g_slist_find(parent->objects, child))
goto done;
data->objects = g_slist_prepend(data->objects, child);
--
2.48.1
^ permalink raw reply related
* [PATCH] Bluetooth: Add driver command BTUSB_DRV_CMD_SWITCH_ALT_SETTING
From: Hsin-chen Chuang @ 2025-04-01 13:44 UTC (permalink / raw)
To: luiz.dentz
Cc: Hsin-chen Chuang, chromeos-bluetooth-upstreaming, David S. Miller,
Eric Dumazet, Jakub Kicinski, Johan Hedberg, Marcel Holtmann,
Paolo Abeni, Simon Horman, Ying Hsu, linux-bluetooth,
linux-kernel, netdev
From: Hsin-chen Chuang <chharry@chromium.org>
Although commit 75ddcd5ad40e ("Bluetooth: btusb: Configure altsetting
for HCI_USER_CHANNEL") has enabled the HCI_USER_CHANNEL user to send out
SCO data through USB Bluetooth chips, it's observed that with the patch
HFP is flaky on most of the existing USB Bluetooth controllers: Intel
chips sometimes send out no packet for Transparent codec; MTK chips may
generate SCO data with a wrong handle for CVSD codec; RTK could split
the data with a wrong packet size for Transparent codec; ... etc.
To address the issue above one needs to reset the altsetting back to
zero when there is no active SCO connection, which is the same as the
BlueZ behavior, and another benefit is the bus doesn't need to reserve
bandwidth when no SCO connection.
This patch introduces a fundamental solution that lets the user space
program to configure the altsetting freely:
- Define the new packet type HCI_DRV_PKT which is specifically used for
communication between the user space program and the Bluetooth drviers
- Define the btusb driver command BTUSB_DRV_CMD_SWITCH_ALT_SETTING which
indicates the expected altsetting from the user space program
- btusb intercepts the command and adjusts the Isoc endpoint
correspondingly
This patch is tested on ChromeOS devices. The USB Bluetooth models
(CVSD, TRANS alt3, and TRANS alt6) could pass the stress HFP test narrow
band speech and wide band speech.
Cc: chromeos-bluetooth-upstreaming@chromium.org
Fixes: b16b327edb4d ("Bluetooth: btusb: add sysfs attribute to control USB alt setting")
Signed-off-by: Hsin-chen Chuang <chharry@chromium.org>
---
drivers/bluetooth/btusb.c | 67 +++++++++++++++++++++++++++++++++
include/net/bluetooth/hci.h | 1 +
include/net/bluetooth/hci_mon.h | 2 +
net/bluetooth/hci_core.c | 2 +
net/bluetooth/hci_sock.c | 14 +++++--
5 files changed, 83 insertions(+), 3 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 5012b5ff92c8..a7bc64e86661 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2151,6 +2151,67 @@ static int submit_or_queue_tx_urb(struct hci_dev *hdev, struct urb *urb)
return 0;
}
+static struct sk_buff *btusb_drv_response(u8 opcode, size_t data_len)
+{
+ struct sk_buff *skb;
+
+ /* btusb driver response starts with 1 oct of the opcode,
+ * and followed by the command specific data.
+ */
+ skb = bt_skb_alloc(1 + data_len, GFP_KERNEL);
+ if (!skb)
+ return NULL;
+
+ skb_put_u8(skb, opcode);
+ hci_skb_pkt_type(skb) = HCI_DRV_PKT;
+
+ return skb;
+}
+
+static int btusb_switch_alt_setting(struct hci_dev *hdev, int new_alts);
+
+#define BTUSB_DRV_CMD_SWITCH_ALT_SETTING 0x35
+
+static int btusb_drv_cmd(struct hci_dev *hdev, struct sk_buff *skb)
+{
+ /* btusb driver command starts with 1 oct of the opcode,
+ * and followed by the command specific data.
+ */
+ if (!skb->len)
+ return -EILSEQ;
+
+ switch (skb->data[0]) {
+ case BTUSB_DRV_CMD_SWITCH_ALT_SETTING: {
+ struct sk_buff *resp;
+ int status;
+
+ /* Response data: Total 1 Oct
+ * Status: 1 Oct
+ * 0 = Success
+ * 1 = Invalid command
+ * 2 = Other errors
+ */
+ resp = btusb_drv_response(BTUSB_DRV_CMD_SWITCH_ALT_SETTING, 1);
+ if (!resp)
+ return -ENOMEM;
+
+ if (skb->len != 2 || skb->data[1] > 6) {
+ status = 1;
+ } else {
+ status = btusb_switch_alt_setting(hdev, skb->data[1]);
+ if (status)
+ status = 2;
+ }
+ skb_put_u8(resp, status);
+
+ kfree_skb(skb);
+ return hci_recv_frame(hdev, resp);
+ }
+ }
+
+ return -EILSEQ;
+}
+
static int btusb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
{
struct urb *urb;
@@ -2192,6 +2253,9 @@ static int btusb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
return PTR_ERR(urb);
return submit_or_queue_tx_urb(hdev, urb);
+
+ case HCI_DRV_PKT:
+ return btusb_drv_cmd(hdev, skb);
}
return -EILSEQ;
@@ -2669,6 +2733,9 @@ static int btusb_send_frame_intel(struct hci_dev *hdev, struct sk_buff *skb)
return PTR_ERR(urb);
return submit_or_queue_tx_urb(hdev, urb);
+
+ case HCI_DRV_PKT:
+ return btusb_drv_cmd(hdev, skb);
}
return -EILSEQ;
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index a8586c3058c7..e297b312d2b7 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -494,6 +494,7 @@ enum {
#define HCI_EVENT_PKT 0x04
#define HCI_ISODATA_PKT 0x05
#define HCI_DIAG_PKT 0xf0
+#define HCI_DRV_PKT 0xf1
#define HCI_VENDOR_PKT 0xff
/* HCI packet types */
diff --git a/include/net/bluetooth/hci_mon.h b/include/net/bluetooth/hci_mon.h
index 082f89531b88..bbd752494ef9 100644
--- a/include/net/bluetooth/hci_mon.h
+++ b/include/net/bluetooth/hci_mon.h
@@ -51,6 +51,8 @@ struct hci_mon_hdr {
#define HCI_MON_CTRL_EVENT 17
#define HCI_MON_ISO_TX_PKT 18
#define HCI_MON_ISO_RX_PKT 19
+#define HCI_MON_DRV_TX_PKT 20
+#define HCI_MON_DRV_RX_PKT 21
struct hci_mon_new_index {
__u8 type;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 5eb0600bbd03..bb4e1721edc2 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2911,6 +2911,8 @@ int hci_recv_frame(struct hci_dev *hdev, struct sk_buff *skb)
break;
case HCI_ISODATA_PKT:
break;
+ case HCI_DRV_PKT:
+ break;
default:
kfree_skb(skb);
return -EINVAL;
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 022b86797acd..0bc4f77ed17b 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -234,7 +234,8 @@ void hci_send_to_sock(struct hci_dev *hdev, struct sk_buff *skb)
if (hci_skb_pkt_type(skb) != HCI_EVENT_PKT &&
hci_skb_pkt_type(skb) != HCI_ACLDATA_PKT &&
hci_skb_pkt_type(skb) != HCI_SCODATA_PKT &&
- hci_skb_pkt_type(skb) != HCI_ISODATA_PKT)
+ hci_skb_pkt_type(skb) != HCI_ISODATA_PKT &&
+ hci_skb_pkt_type(skb) != HCI_DRV_PKT)
continue;
} else {
/* Don't send frame to other channel types */
@@ -391,6 +392,12 @@ void hci_send_to_monitor(struct hci_dev *hdev, struct sk_buff *skb)
else
opcode = cpu_to_le16(HCI_MON_ISO_TX_PKT);
break;
+ case HCI_DRV_PKT:
+ if (bt_cb(skb)->incoming)
+ opcode = cpu_to_le16(HCI_MON_DRV_RX_PKT);
+ else
+ opcode = cpu_to_le16(HCI_MON_DRV_TX_PKT);
+ break;
case HCI_DIAG_PKT:
opcode = cpu_to_le16(HCI_MON_VENDOR_DIAG);
break;
@@ -1806,7 +1813,7 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg,
if (flags & ~(MSG_DONTWAIT | MSG_NOSIGNAL | MSG_ERRQUEUE | MSG_CMSG_COMPAT))
return -EINVAL;
- if (len < 4 || len > hci_pi(sk)->mtu)
+ if (len > hci_pi(sk)->mtu)
return -EINVAL;
skb = bt_skb_sendmsg(sk, msg, len, len, 0, 0);
@@ -1860,7 +1867,8 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg,
if (hci_skb_pkt_type(skb) != HCI_COMMAND_PKT &&
hci_skb_pkt_type(skb) != HCI_ACLDATA_PKT &&
hci_skb_pkt_type(skb) != HCI_SCODATA_PKT &&
- hci_skb_pkt_type(skb) != HCI_ISODATA_PKT) {
+ hci_skb_pkt_type(skb) != HCI_ISODATA_PKT &&
+ hci_skb_pkt_type(skb) != HCI_DRV_PKT) {
err = -EINVAL;
goto drop;
}
--
2.49.0.472.ge94155a9ec-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox