2022-06-08 14:25:41

by Soenke Huster

[permalink] [raw]
Subject: [PATCH] Bluetooth: RFCOMM: Use skb_trim to trim checksum

Use the skb helper instead of direct manipulation. This fixes the
following page fault, when connecting my Android phone:

BUG: unable to handle page fault for address: ffffed1021de29ff
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
RIP: 0010:rfcomm_run+0x831/0x4040 (net/bluetooth/rfcomm/core.c:1751)

Signed-off-by: Soenke Huster <[email protected]>
---
net/bluetooth/rfcomm/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 7324764384b6..7360e905d045 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -1747,8 +1747,8 @@ static struct rfcomm_session *rfcomm_recv_frame(struct rfcomm_session *s,
type = __get_type(hdr->ctrl);

/* Trim FCS */
- skb->len--; skb->tail--;
- fcs = *(u8 *)skb_tail_pointer(skb);
+ skb_trim(skb, skb->len - 1);
+ fcs = *(skb->data + skb->len);

if (__check_fcs(skb->data, type, fcs)) {
BT_ERR("bad checksum in packet");
--
2.36.1


2022-06-08 14:49:52

by bluez.test.bot

[permalink] [raw]
Subject: RE: Bluetooth: RFCOMM: Use skb_trim to trim checksum

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=648480

---Test result---

Test Summary:
CheckPatch PASS 1.66 seconds
GitLint PASS 0.94 seconds
SubjectPrefix PASS 0.90 seconds
BuildKernel PASS 34.48 seconds
BuildKernel32 PASS 29.81 seconds
Incremental Build with patchesPASS 41.25 seconds
TestRunner: Setup PASS 508.23 seconds
TestRunner: l2cap-tester PASS 17.90 seconds
TestRunner: bnep-tester PASS 6.34 seconds
TestRunner: mgmt-tester PASS 107.06 seconds
TestRunner: rfcomm-tester PASS 10.63 seconds
TestRunner: sco-tester PASS 10.42 seconds
TestRunner: smp-tester PASS 10.39 seconds
TestRunner: userchan-tester PASS 6.76 seconds



---
Regards,
Linux Bluetooth

2022-06-08 16:03:34

by Eric Dumazet

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: RFCOMM: Use skb_trim to trim checksum

On Wed, Jun 8, 2022 at 6:51 AM Soenke Huster <[email protected]> wrote:
>
> Use the skb helper instead of direct manipulation. This fixes the
> following page fault, when connecting my Android phone:
>
> BUG: unable to handle page fault for address: ffffed1021de29ff
> #PF: supervisor read access in kernel mode
> #PF: error_code(0x0000) - not-present page
> RIP: 0010:rfcomm_run+0x831/0x4040 (net/bluetooth/rfcomm/core.c:1751)
>
> Signed-off-by: Soenke Huster <[email protected]>
> ---
> net/bluetooth/rfcomm/core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
> index 7324764384b6..7360e905d045 100644
> --- a/net/bluetooth/rfcomm/core.c
> +++ b/net/bluetooth/rfcomm/core.c
> @@ -1747,8 +1747,8 @@ static struct rfcomm_session *rfcomm_recv_frame(struct rfcomm_session *s,
> type = __get_type(hdr->ctrl);
>
> /* Trim FCS */
> - skb->len--; skb->tail--;
> - fcs = *(u8 *)skb_tail_pointer(skb);
> + skb_trim(skb, skb->len - 1);
> + fcs = *(skb->data + skb->len);
>

Hmmm... I do not see any difference before/after in term of memory
dereference to get fcs.

I think you should give more details on how exactly the bug triggers.

2022-06-09 08:28:03

by Soenke Huster

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: RFCOMM: Use skb_trim to trim checksum

Hi Eric,

On 08.06.22 17:33, Eric Dumazet wrote:
> On Wed, Jun 8, 2022 at 6:51 AM Soenke Huster <[email protected]> wrote:
>>
>> Use the skb helper instead of direct manipulation. This fixes the
>> following page fault, when connecting my Android phone:
>>
>> BUG: unable to handle page fault for address: ffffed1021de29ff
>> #PF: supervisor read access in kernel mode
>> #PF: error_code(0x0000) - not-present page
>> RIP: 0010:rfcomm_run+0x831/0x4040 (net/bluetooth/rfcomm/core.c:1751)
>>
>> Signed-off-by: Soenke Huster <[email protected]>
>> ---
>> net/bluetooth/rfcomm/core.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
>> index 7324764384b6..7360e905d045 100644
>> --- a/net/bluetooth/rfcomm/core.c
>> +++ b/net/bluetooth/rfcomm/core.c
>> @@ -1747,8 +1747,8 @@ static struct rfcomm_session *rfcomm_recv_frame(struct rfcomm_session *s,
>> type = __get_type(hdr->ctrl);
>>
>> /* Trim FCS */
>> - skb->len--; skb->tail--;
>> - fcs = *(u8 *)skb_tail_pointer(skb);
>> + skb_trim(skb, skb->len - 1);
>> + fcs = *(skb->data + skb->len);
>>
>
> Hmmm... I do not see any difference before/after in term of memory
> dereference to get fcs.
>
> I think you should give more details on how exactly the bug triggers.

Sorry, yesterday I was not able to track down why exactly it crashes,
but by now I think I figured it out.

The crash happens when using Bluetooth in a virtual machine.
On connecting my Android phone to the physical controller which I use
inside the virtual machine via the VirtIO driver, after some seconds
the crash occurs.

Before the trimming step, I examined the skb in gdb and saw, that
skb->tail is zero. Thus, skb->tail--; modifies the unsigned integer to -1
resp. MAX_UINT. In skb_tail_pointer, skb->head + skb->tail is calculated
which results in the page fault.

By using skb_trim, skb->tail is set to the accurate value and thus the
issue is fixed.

I am not an expert in the Linux kernel area, do you think there is an
underlying issue anywhere else? When using my Android phone on my host
computer, I do not have that problem - it might be in some
(e.g. virtio_bt?) driver? On the other hand, with the patch my problem
is solved and the phone is usable in the virtual machine!