2016-08-12 12:11:28

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH] Bluetooth: Fix bt_sock_recvmsg when MSG_TRUNC is not set

From: Luiz Augusto von Dentz <[email protected]>

Commit b5f34f9420b50c9b5876b9a2b68e96be6d629054 attempt to introduce
proper handling for MSG_TRUNC but recv and variants should still work
as read if no flag is passed, but because the code may set MSG_TRUNC to
msg->msg_flags that shall not be used as it may cause it to be behave as
if MSG_TRUNC is always, so instead of using it this changes the code to
use the flags parameter which shall contain the original flags.

Signed-off-by: Luiz Augusto von Dentz <[email protected]>
---
net/bluetooth/af_bluetooth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index b8a5caf..1d96ff3 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -251,7 +251,7 @@ int bt_sock_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,

skb_free_datagram(sk, skb);

- if (msg->msg_flags & MSG_TRUNC)
+ if (flags & MSG_TRUNC)
copied = skblen;

return err ? : copied;
--
2.7.4



2016-08-15 12:49:45

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Fix bt_sock_recvmsg when MSG_TRUNC is not set

Hi Luiz,

>>>> Commit b5f34f9420b50c9b5876b9a2b68e96be6d629054 attempt to introduce
>>>> proper handling for MSG_TRUNC but recv and variants should still work
>>>> as read if no flag is passed, but because the code may set MSG_TRUNC to
>>>> msg->msg_flags that shall not be used as it may cause it to be behave as
>>>> if MSG_TRUNC is always, so instead of using it this changes the code to
>>>> use the flags parameter which shall contain the original flags.
>>>>
>>>
>>> Taking a look at udp_recvmsg(), looks like this fix is indeed
>>> necessary. And that patch that "fixed" sdpd-server.c may not be needed
>>> at all.
>>
>> and what about hci_sock_recvmsg function? Does it need the same fix?
>
> Looks like it has the same problem, shall I send a patch or you will
> take care of it?

please send a patch for that one as well.

>> Also we should really create test cases for HCI and L2CAP/RFCOMM sockets when it comes to recv and send. I would propose to introduce a sock-tester application. Or feed it into l2cap-tester etc.
>
> Indeed it would be great to have this covered, perhaps even covering
> the testing spec when possible so we can detect regressions to
> L2CAP/RFCOMM without having to run PTS.

Can you work on that to at least cover the basic socket cases with msg_flags. I mean most important are really HCI sockets and L2CAP sockets. Since they are used most.

Regards

Marcel


2016-08-15 12:25:28

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Fix bt_sock_recvmsg when MSG_TRUNC is not set

Hi Marcel,

On Mon, Aug 15, 2016 at 3:13 PM, Marcel Holtmann <[email protected]> wrote:
> Hi Vinicius,
>
>>> Commit b5f34f9420b50c9b5876b9a2b68e96be6d629054 attempt to introduce
>>> proper handling for MSG_TRUNC but recv and variants should still work
>>> as read if no flag is passed, but because the code may set MSG_TRUNC to
>>> msg->msg_flags that shall not be used as it may cause it to be behave as
>>> if MSG_TRUNC is always, so instead of using it this changes the code to
>>> use the flags parameter which shall contain the original flags.
>>>
>>
>> Taking a look at udp_recvmsg(), looks like this fix is indeed
>> necessary. And that patch that "fixed" sdpd-server.c may not be needed
>> at all.
>
> and what about hci_sock_recvmsg function? Does it need the same fix?

Looks like it has the same problem, shall I send a patch or you will
take care of it?

> Also we should really create test cases for HCI and L2CAP/RFCOMM sockets when it comes to recv and send. I would propose to introduce a sock-tester application. Or feed it into l2cap-tester etc.

Indeed it would be great to have this covered, perhaps even covering
the testing spec when possible so we can detect regressions to
L2CAP/RFCOMM without having to run PTS.

--
Luiz Augusto von Dentz

2016-08-15 12:13:08

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Fix bt_sock_recvmsg when MSG_TRUNC is not set

Hi Vinicius,

>> Commit b5f34f9420b50c9b5876b9a2b68e96be6d629054 attempt to introduce
>> proper handling for MSG_TRUNC but recv and variants should still work
>> as read if no flag is passed, but because the code may set MSG_TRUNC to
>> msg->msg_flags that shall not be used as it may cause it to be behave as
>> if MSG_TRUNC is always, so instead of using it this changes the code to
>> use the flags parameter which shall contain the original flags.
>>
>
> Taking a look at udp_recvmsg(), looks like this fix is indeed
> necessary. And that patch that "fixed" sdpd-server.c may not be needed
> at all.

and what about hci_sock_recvmsg function? Does it need the same fix?

Also we should really create test cases for HCI and L2CAP/RFCOMM sockets when it comes to recv and send. I would propose to introduce a sock-tester application. Or feed it into l2cap-tester etc.

Regards

Marcel


2016-08-15 12:11:32

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Fix bt_sock_recvmsg when MSG_TRUNC is not set

Hi Luiz,

> Commit b5f34f9420b50c9b5876b9a2b68e96be6d629054 attempt to introduce
> proper handling for MSG_TRUNC but recv and variants should still work
> as read if no flag is passed, but because the code may set MSG_TRUNC to
> msg->msg_flags that shall not be used as it may cause it to be behave as
> if MSG_TRUNC is always, so instead of using it this changes the code to
> use the flags parameter which shall contain the original flags.
>
> Signed-off-by: Luiz Augusto von Dentz <[email protected]>
> ---
> net/bluetooth/af_bluetooth.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

patch has been applied to bluetooth-stable tree.

Regards

Marcel


2016-08-12 14:22:06

by Vinicius Costa Gomes

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Fix bt_sock_recvmsg when MSG_TRUNC is not set

Hi,

Luiz Augusto von Dentz <[email protected]> writes:

> From: Luiz Augusto von Dentz <[email protected]>
>
> Commit b5f34f9420b50c9b5876b9a2b68e96be6d629054 attempt to introduce
> proper handling for MSG_TRUNC but recv and variants should still work
> as read if no flag is passed, but because the code may set MSG_TRUNC to
> msg->msg_flags that shall not be used as it may cause it to be behave as
> if MSG_TRUNC is always, so instead of using it this changes the code to
> use the flags parameter which shall contain the original flags.
>

Taking a look at udp_recvmsg(), looks like this fix is indeed
necessary. And that patch that "fixed" sdpd-server.c may not be needed
at all.

Looks good.


Cheers,
--
Vinicius