[PATCH] Bluetooth: fix incorrect nonblock bitmask in bt_sock_wait_ready()

[PATCH] Bluetooth: fix incorrect nonblock bitmask in bt_sock_wait_ready()

[gavin]

From: Gavin Li <gavin@matician.com>

Callers pass msg->msg_flags as flags, which contains MSG_DONTWAIT
instead of O_NONBLOCK.

Signed-off-by: Gavin Li <gavin@matician.com>
---
 include/net/bluetooth/bluetooth.h | 2 +-
 net/bluetooth/af_bluetooth.c      | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index a647e5fabdbd6..87f0bba39b0f7 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -343,7 +343,7 @@ int  bt_sock_stream_recvmsg(struct socket *sock, struct msghdr *msg,
 __poll_t bt_sock_poll(struct file *file, struct socket *sock, poll_table *wait);
 int  bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
 int  bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo);
-int  bt_sock_wait_ready(struct sock *sk, unsigned long flags);
+int  bt_sock_wait_ready(struct sock *sk, unsigned int flags);
 
 void bt_accept_enqueue(struct sock *parent, struct sock *sk, bool bh);
 void bt_accept_unlink(struct sock *sk);
diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index ee319779781e6..69374321130e4 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -568,7 +568,7 @@ int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo)
 EXPORT_SYMBOL(bt_sock_wait_state);
 
 /* This function expects the sk lock to be held when called */
-int bt_sock_wait_ready(struct sock *sk, unsigned long flags)
+int bt_sock_wait_ready(struct sock *sk, unsigned int flags)
 {
 	DECLARE_WAITQUEUE(wait, current);
 	unsigned long timeo;
@@ -576,7 +576,7 @@ int bt_sock_wait_ready(struct sock *sk, unsigned long flags)
 
 	BT_DBG("sk %p", sk);
 
-	timeo = sock_sndtimeo(sk, flags & O_NONBLOCK);
+	timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);
 
 	add_wait_queue(sk_sleep(sk), &wait);
 	set_current_state(TASK_INTERRUPTIBLE);
-- 
2.34.1

RE: Bluetooth: fix incorrect nonblock bitmask in bt_sock_wait_ready()

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1097 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=617495

---Test result---

Test Summary:
CheckPatch                    PASS      1.94 seconds
GitLint                       PASS      1.00 seconds
SubjectPrefix                 PASS      0.83 seconds
BuildKernel                   PASS      36.68 seconds
BuildKernel32                 PASS      32.49 seconds
Incremental Build with patchesPASS      46.70 seconds
TestRunner: Setup             PASS      556.83 seconds
TestRunner: l2cap-tester      PASS      15.42 seconds
TestRunner: bnep-tester       PASS      7.09 seconds
TestRunner: mgmt-tester       PASS      118.87 seconds
TestRunner: rfcomm-tester     PASS      9.20 seconds
TestRunner: sco-tester        PASS      9.41 seconds
TestRunner: smp-tester        PASS      9.20 seconds
TestRunner: userchan-tester   PASS      7.50 seconds



---
Regards,
Linux Bluetooth

Re: [PATCH] Bluetooth: fix incorrect nonblock bitmask in bt_sock_wait_ready()

[Marcel Holtmann]

Hi Gavin,

> Callers pass msg->msg_flags as flags, which contains MSG_DONTWAIT
> instead of O_NONBLOCK.
> 
> Signed-off-by: Gavin Li <gavin@matician.com>
> ---
> include/net/bluetooth/bluetooth.h | 2 +-
> net/bluetooth/af_bluetooth.c      | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index a647e5fabdbd6..87f0bba39b0f7 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -343,7 +343,7 @@ int  bt_sock_stream_recvmsg(struct socket *sock, struct msghdr *msg,
> __poll_t bt_sock_poll(struct file *file, struct socket *sock, poll_table *wait);
> int  bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
> int  bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo);
> -int  bt_sock_wait_ready(struct sock *sk, unsigned long flags);
> +int  bt_sock_wait_ready(struct sock *sk, unsigned int flags);
> 
> void bt_accept_enqueue(struct sock *parent, struct sock *sk, bool bh);
> void bt_accept_unlink(struct sock *sk);
> diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
> index ee319779781e6..69374321130e4 100644
> --- a/net/bluetooth/af_bluetooth.c
> +++ b/net/bluetooth/af_bluetooth.c
> @@ -568,7 +568,7 @@ int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo)
> EXPORT_SYMBOL(bt_sock_wait_state);
> 
> /* This function expects the sk lock to be held when called */
> -int bt_sock_wait_ready(struct sock *sk, unsigned long flags)
> +int bt_sock_wait_ready(struct sock *sk, unsigned int flags)

can we then also do s/flags/msg_flags/ then.

> {
> 	DECLARE_WAITQUEUE(wait, current);
> 	unsigned long timeo;
> @@ -576,7 +576,7 @@ int bt_sock_wait_ready(struct sock *sk, unsigned long flags)
> 
> 	BT_DBG("sk %p", sk);
> 
> -	timeo = sock_sndtimeo(sk, flags & O_NONBLOCK);
> +	timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);

Since sock_sndtimeo() is taking a bool. This might be better !!(flags & MSG_DONTWAIT).

Regards

Marcel

Re: [PATCH] Bluetooth: fix incorrect nonblock bitmask in bt_sock_wait_ready()

[Gavin Li]

Hi Marcel,

Thanks for reviewing this quickly.

> > diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
> > index ee319779781e6..69374321130e4 100644
> > --- a/net/bluetooth/af_bluetooth.c
> > +++ b/net/bluetooth/af_bluetooth.c
> > @@ -568,7 +568,7 @@ int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo)
> > EXPORT_SYMBOL(bt_sock_wait_state);
> >
> > /* This function expects the sk lock to be held when called */
> > -int bt_sock_wait_ready(struct sock *sk, unsigned long flags)
> > +int bt_sock_wait_ready(struct sock *sk, unsigned int flags)
>
> can we then also do s/flags/msg_flags/ then.
I prefer keeping it as flags because all other net code also uses
flags, msg_flags only appears
in msg->msg_flags.

> > @@ -576,7 +576,7 @@ int bt_sock_wait_ready(struct sock *sk, unsigned long flags)
> >
> >       BT_DBG("sk %p", sk);
> >
> > -     timeo = sock_sndtimeo(sk, flags & O_NONBLOCK);
> > +     timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);
>
> Since sock_sndtimeo() is taking a bool. This might be better !!(flags & MSG_DONTWAIT).
It appears to be well-known in the net code that sock_sndtimeo takes a
bool, since no other
uses of it do the "!!" conversion.

Let me know what you think. I can make the changes if needed but I was
just trying my best
to match the currently existing convention.

Best,
Gavin

Re: [PATCH] Bluetooth: fix incorrect nonblock bitmask in bt_sock_wait_ready()

[Gavin Li]

Hi Marcel,

Please let me know what you think with regards to the comments above.

Best,
Gavin


On Thu, Feb 24, 2022 at 12:56 PM Gavin Li <gavin@matician.com> wrote:
>
> Hi Marcel,
>
> Thanks for reviewing this quickly.
>
> > > diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
> > > index ee319779781e6..69374321130e4 100644
> > > --- a/net/bluetooth/af_bluetooth.c
> > > +++ b/net/bluetooth/af_bluetooth.c
> > > @@ -568,7 +568,7 @@ int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo)
> > > EXPORT_SYMBOL(bt_sock_wait_state);
> > >
> > > /* This function expects the sk lock to be held when called */
> > > -int bt_sock_wait_ready(struct sock *sk, unsigned long flags)
> > > +int bt_sock_wait_ready(struct sock *sk, unsigned int flags)
> >
> > can we then also do s/flags/msg_flags/ then.
> I prefer keeping it as flags because all other net code also uses
> flags, msg_flags only appears
> in msg->msg_flags.
>
> > > @@ -576,7 +576,7 @@ int bt_sock_wait_ready(struct sock *sk, unsigned long flags)
> > >
> > >       BT_DBG("sk %p", sk);
> > >
> > > -     timeo = sock_sndtimeo(sk, flags & O_NONBLOCK);
> > > +     timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);
> >
> > Since sock_sndtimeo() is taking a bool. This might be better !!(flags & MSG_DONTWAIT).
> It appears to be well-known in the net code that sock_sndtimeo takes a
> bool, since no other
> uses of it do the "!!" conversion.
>
> Let me know what you think. I can make the changes if needed but I was
> just trying my best
> to match the currently existing convention.
>
> Best,
> Gavin

Re: [PATCH] Bluetooth: fix incorrect nonblock bitmask in bt_sock_wait_ready()

[Marcel Holtmann]

Hi Gavin,

>>> diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
>>> index ee319779781e6..69374321130e4 100644
>>> --- a/net/bluetooth/af_bluetooth.c
>>> +++ b/net/bluetooth/af_bluetooth.c
>>> @@ -568,7 +568,7 @@ int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo)
>>> EXPORT_SYMBOL(bt_sock_wait_state);
>>> 
>>> /* This function expects the sk lock to be held when called */
>>> -int bt_sock_wait_ready(struct sock *sk, unsigned long flags)
>>> +int bt_sock_wait_ready(struct sock *sk, unsigned int flags)
>> 
>> can we then also do s/flags/msg_flags/ then.
> I prefer keeping it as flags because all other net code also uses
> flags, msg_flags only appears
> in msg->msg_flags.

while that might be true, I find it a lot clearer if the variable is msg_flags.

>>> @@ -576,7 +576,7 @@ int bt_sock_wait_ready(struct sock *sk, unsigned long flags)
>>> 
>>>      BT_DBG("sk %p", sk);
>>> 
>>> -     timeo = sock_sndtimeo(sk, flags & O_NONBLOCK);
>>> +     timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);
>> 
>> Since sock_sndtimeo() is taking a bool. This might be better !!(flags & MSG_DONTWAIT).
> It appears to be well-known in the net code that sock_sndtimeo takes a
> bool, since no other
> uses of it do the "!!" conversion.
> 
> Let me know what you think. I can make the changes if needed but I was
> just trying my best
> to match the currently existing convention.

And other code in the kernel makes sure to clearly turn something into a bool. You get 0x00 and 0x40.

Regards

Marcel