[PATCH] Bluetooth: Fix bt_skb_sendmmsg not allocating partial chunks

[PATCH] Bluetooth: Fix bt_skb_sendmmsg not allocating partial chunks

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Since bt_skb_sendmmsg can be used with the likes of SOCK_STREAM it
shall return the partial chunks it could allocate instead of freeing
everything as otherwise it can cause problems like bellow.

Link: https://lore.kernel.org/linux-bluetooth/aa3ee7ac-6c52-3861-1798-3cc1a37f6ebf@molgen.mpg.de/T/#m1f9673e4ab0d55a7dccf87905337ab2e67d689f1
Fixes: 81be03e026dc ("Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 include/net/bluetooth/bluetooth.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 4b3d0b16c185..a647e5fabdbd 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -506,8 +506,7 @@ static inline struct sk_buff *bt_skb_sendmmsg(struct sock *sk,
 
 		tmp = bt_skb_sendmsg(sk, msg, len, mtu, headroom, tailroom);
 		if (IS_ERR(tmp)) {
-			kfree_skb(skb);
-			return tmp;
+			return skb;
 		}
 
 		len -= tmp->len;
-- 
2.34.1

RE: Bluetooth: Fix bt_skb_sendmmsg not allocating partial chunks

[bluez.test.bot]

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

---Test result---

Test Summary:
CheckPatch                    PASS      1.66 seconds
GitLint                       FAIL      1.02 seconds
SubjectPrefix                 PASS      0.84 seconds
BuildKernel                   PASS      39.86 seconds
BuildKernel32                 PASS      35.43 seconds
Incremental Build with patchesPASS      46.00 seconds
TestRunner: Setup             PASS      583.22 seconds
TestRunner: l2cap-tester      PASS      15.80 seconds
TestRunner: bnep-tester       PASS      8.18 seconds
TestRunner: mgmt-tester       PASS      124.55 seconds
TestRunner: rfcomm-tester     PASS      9.28 seconds
TestRunner: sco-tester        PASS      8.98 seconds
TestRunner: smp-tester        PASS      8.79 seconds
TestRunner: userchan-tester   PASS      7.30 seconds

Details
##############################
Test: GitLint - FAIL - 1.02 seconds
Run gitlint with rule in .gitlint
Bluetooth: Fix bt_skb_sendmmsg not allocating partial chunks
9: B1 Line exceeds max length (141>80): "Link: https://lore.kernel.org/linux-bluetooth/aa3ee7ac-6c52-3861-1798-3cc1a37f6ebf@molgen.mpg.de/T/#m1f9673e4ab0d55a7dccf87905337ab2e67d689f1"




---
Regards,
Linux Bluetooth

Re: [PATCH] Bluetooth: Fix bt_skb_sendmmsg not allocating partial chunks

[Paul Menzel]

Dear Luiz,


Am 15.02.22 um 02:59 schrieb Luiz Augusto von Dentz:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> Since bt_skb_sendmmsg can be used with the likes of SOCK_STREAM it
> shall return the partial chunks it could allocate instead of freeing
> everything as otherwise it can cause problems like bellow.

Thank you for fixing the bug I found.

Tested-by: Paul Menzel <pmenzel@molgen.mpg.de> (Nokia N9 (MeeGo/Harmattan)

> Link: https://lore.kernel.org/linux-bluetooth/aa3ee7ac-6c52-3861-1798-3cc1a37f6ebf@molgen.mpg.de/T/#m1f9673e4ab0d55a7dccf87905337ab2e67d689f1

The Linux kernel regression status, when expanding the bug entry: asks 
to add:

Fixes: 81be03e026dc ("Bluetooth: RFCOMM: Replace use of memcpy_from_msg 
with bt_skb_sendmmsg")
Reported-by: Paul Menzel <pmenzel@molgen.mpg.de>
Link: 
https://lore.kernel.org/r/d7206e12-1b99-c3be-84f4-df22af427ef5@molgen.mpg.de

> Fixes: 81be03e026dc ("Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg")
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Maybe also add:

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=215594

> ---
>   include/net/bluetooth/bluetooth.h | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index 4b3d0b16c185..a647e5fabdbd 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -506,8 +506,7 @@ static inline struct sk_buff *bt_skb_sendmmsg(struct sock *sk,
>   
>   		tmp = bt_skb_sendmsg(sk, msg, len, mtu, headroom, tailroom);
>   		if (IS_ERR(tmp)) {
> -			kfree_skb(skb);
> -			return tmp;
> +			return skb;
>   		}
>   
>   		len -= tmp->len;


Kind regards,

Paul


[1]: https://linux-regtracking.leemhuis.info/regzbot/mainline/

Re: [PATCH] Bluetooth: Fix bt_skb_sendmmsg not allocating partial chunks

[Marcel Holtmann]

Hi Luiz,

> Since bt_skb_sendmmsg can be used with the likes of SOCK_STREAM it
> shall return the partial chunks it could allocate instead of freeing
> everything as otherwise it can cause problems like bellow.
> 
> Link: https://lore.kernel.org/linux-bluetooth/aa3ee7ac-6c52-3861-1798-3cc1a37f6ebf@molgen.mpg.de/T/#m1f9673e4ab0d55a7dccf87905337ab2e67d689f1
> Fixes: 81be03e026dc ("Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg")
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
> include/net/bluetooth/bluetooth.h | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel

Re: [PATCH] Bluetooth: Fix bt_skb_sendmmsg not allocating partial chunks

[Thorsten Leemhuis]

[CCing Johan, Jakub and Dave]

Hi Bluetooth maintainers!

On 16.02.22 11:26, Marcel Holtmann wrote:
>
>> Since bt_skb_sendmmsg can be used with the likes of SOCK_STREAM it
>> shall return the partial chunks it could allocate instead of freeing
>> everything as otherwise it can cause problems like bellow.
>>
>> Link: https://lore.kernel.org/linux-bluetooth/aa3ee7ac-6c52-3861-1798-3cc1a37f6ebf@molgen.mpg.de/T/#m1f9673e4ab0d55a7dccf87905337ab2e67d689f1
>> Fixes: 81be03e026dc ("Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg")
>> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>> ---
>> include/net/bluetooth/bluetooth.h | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
> 
> patch has been applied to bluetooth-next tree.

Luiz, Marcel, thx for fixing this 5.16 regression and picking the patch
up for merging. But I have to wonder: why was this simple fix put into a
tree that apparently is meant to only get merged to mainline during the
the next merge window? That will mean this regression will bother people
(maybe Paul is not the only one that is affected by this) for weeks to
come and even make it into 5.17, before it gets fixed for 5.18-rc1.
Despite the lack of a "Cc: <stable@vger.kernel.org>" tag it likely will
get backporting to 5.17.y and 5.16.y afterwards, but the latter soon
after will be EOLed anyway.

Correct me if I'm wrong, but that afaik is not how the Linux development
process is meant to handle such regressions. This approach also
contributes to the huge stable and longterm releases after the end of
each merge window, which some people see as a problem.

I bring this up because there were other regression fixes in the last
few weeks that took such a slow path towards mainline. I also checked
MAINTAINERS and noticed you even have a tree that could feed fixes like
this to Linus via the regular net tree, but apparently you haven't used
it in quite a while:
https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git
I rechecked and noticed not a single bluetooth fix was merged between
v5.16-rc1..v5.16. I doubt Jakub or Dave are the reason, as they merge
fixes from downstream trees every week and send them to Linus shortly
after that.

So why are things like that? Or is there something wrong with my look on
things?

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I'm getting a lot of
reports on my table. I can only look briefly into most of them and lack
knowledge about most of the areas they concern. I thus unfortunately
will sometimes get things wrong or miss something important. I hope
that's not the case here; if you think it is, don't hesitate to tell me
in a public reply, it's in everyone's interest to set the public record
straight.

Re: [PATCH] Bluetooth: Fix bt_skb_sendmmsg not allocating partial chunks

[Thorsten Leemhuis]

Hi, this is your Linux kernel regression tracker again. Top-posting for
once, to make this easily accessible for everyone.

I sent below inquiry on Saturday and didn't get a reply; and I didn't
notice any other activity to get this regression fix mainlined soon.

Bluetooth maintainers, what's up here? I'd like to avoid getting Linus
involved, but I guess I'm out of options if this mail doesn't get things
rolling -- or alternatively an answer why this fix might better wait for
the next merge window to get merged.

Ciao, Thorsten

On 19.02.22 10:18, Thorsten Leemhuis wrote:
> 
> [CCing Johan, Jakub and Dave]
> 
> Hi Bluetooth maintainers!
> 
> On 16.02.22 11:26, Marcel Holtmann wrote:
>>
>>> Since bt_skb_sendmmsg can be used with the likes of SOCK_STREAM it
>>> shall return the partial chunks it could allocate instead of freeing
>>> everything as otherwise it can cause problems like bellow.
>>>
>>> Link: https://lore.kernel.org/linux-bluetooth/aa3ee7ac-6c52-3861-1798-3cc1a37f6ebf@molgen.mpg.de/T/#m1f9673e4ab0d55a7dccf87905337ab2e67d689f1
>>> Fixes: 81be03e026dc ("Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg")
>>> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>> ---
>>> include/net/bluetooth/bluetooth.h | 3 +--
>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> patch has been applied to bluetooth-next tree.
> 
> Luiz, Marcel, thx for fixing this 5.16 regression and picking the patch
> up for merging. But I have to wonder: why was this simple fix put into a
> tree that apparently is meant to only get merged to mainline during the
> the next merge window? That will mean this regression will bother people
> (maybe Paul is not the only one that is affected by this) for weeks to
> come and even make it into 5.17, before it gets fixed for 5.18-rc1.
> Despite the lack of a "Cc: <stable@vger.kernel.org>" tag it likely will
> get backporting to 5.17.y and 5.16.y afterwards, but the latter soon
> after will be EOLed anyway.
> 
> Correct me if I'm wrong, but that afaik is not how the Linux development
> process is meant to handle such regressions. This approach also
> contributes to the huge stable and longterm releases after the end of
> each merge window, which some people see as a problem.
> 
> I bring this up because there were other regression fixes in the last
> few weeks that took such a slow path towards mainline. I also checked
> MAINTAINERS and noticed you even have a tree that could feed fixes like
> this to Linus via the regular net tree, but apparently you haven't used
> it in quite a while:
> https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git
> I rechecked and noticed not a single bluetooth fix was merged between
> v5.16-rc1..v5.16. I doubt Jakub or Dave are the reason, as they merge
> fixes from downstream trees every week and send them to Linus shortly
> after that.
> 
> So why are things like that? Or is there something wrong with my look on
> things?
> 
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> 
> P.S.: As the Linux kernel's regression tracker I'm getting a lot of
> reports on my table. I can only look briefly into most of them and lack
> knowledge about most of the areas they concern. I thus unfortunately
> will sometimes get things wrong or miss something important. I hope
> that's not the case here; if you think it is, don't hesitate to tell me
> in a public reply, it's in everyone's interest to set the public record
> straight.

Re: [PATCH] Bluetooth: Fix bt_skb_sendmmsg not allocating partial chunks

[Luiz Augusto von Dentz]

Hi Thorsten,

On Thu, Feb 24, 2022 at 6:09 AM Thorsten Leemhuis <linux@leemhuis.info> wrote:
>
> Hi, this is your Linux kernel regression tracker again. Top-posting for
> once, to make this easily accessible for everyone.
>
> I sent below inquiry on Saturday and didn't get a reply; and I didn't
> notice any other activity to get this regression fix mainlined soon.
>
> Bluetooth maintainers, what's up here? I'd like to avoid getting Linus
> involved, but I guess I'm out of options if this mail doesn't get things
> rolling -- or alternatively an answer why this fix might better wait for
> the next merge window to get merged.

As you probably noticed we have some other regression going on, we are
planning to have a pull request to net.git once they are resolved.

> Ciao, Thorsten
>
> On 19.02.22 10:18, Thorsten Leemhuis wrote:
> >
> > [CCing Johan, Jakub and Dave]
> >
> > Hi Bluetooth maintainers!
> >
> > On 16.02.22 11:26, Marcel Holtmann wrote:
> >>
> >>> Since bt_skb_sendmmsg can be used with the likes of SOCK_STREAM it
> >>> shall return the partial chunks it could allocate instead of freeing
> >>> everything as otherwise it can cause problems like bellow.
> >>>
> >>> Link: https://lore.kernel.org/linux-bluetooth/aa3ee7ac-6c52-3861-1798-3cc1a37f6ebf@molgen.mpg.de/T/#m1f9673e4ab0d55a7dccf87905337ab2e67d689f1
> >>> Fixes: 81be03e026dc ("Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg")
> >>> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >>> ---
> >>> include/net/bluetooth/bluetooth.h | 3 +--
> >>> 1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >> patch has been applied to bluetooth-next tree.
> >
> > Luiz, Marcel, thx for fixing this 5.16 regression and picking the patch
> > up for merging. But I have to wonder: why was this simple fix put into a
> > tree that apparently is meant to only get merged to mainline during the
> > the next merge window? That will mean this regression will bother people
> > (maybe Paul is not the only one that is affected by this) for weeks to
> > come and even make it into 5.17, before it gets fixed for 5.18-rc1.
> > Despite the lack of a "Cc: <stable@vger.kernel.org>" tag it likely will
> > get backporting to 5.17.y and 5.16.y afterwards, but the latter soon
> > after will be EOLed anyway.
> >
> > Correct me if I'm wrong, but that afaik is not how the Linux development
> > process is meant to handle such regressions. This approach also
> > contributes to the huge stable and longterm releases after the end of
> > each merge window, which some people see as a problem.
> >
> > I bring this up because there were other regression fixes in the last
> > few weeks that took such a slow path towards mainline. I also checked
> > MAINTAINERS and noticed you even have a tree that could feed fixes like
> > this to Linus via the regular net tree, but apparently you haven't used
> > it in quite a while:
> > https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git
> > I rechecked and noticed not a single bluetooth fix was merged between
> > v5.16-rc1..v5.16. I doubt Jakub or Dave are the reason, as they merge
> > fixes from downstream trees every week and send them to Linus shortly
> > after that.
> >
> > So why are things like that? Or is there something wrong with my look on
> > things?
> >
> > Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> >
> > P.S.: As the Linux kernel's regression tracker I'm getting a lot of
> > reports on my table. I can only look briefly into most of them and lack
> > knowledge about most of the areas they concern. I thus unfortunately
> > will sometimes get things wrong or miss something important. I hope
> > that's not the case here; if you think it is, don't hesitate to tell me
> > in a public reply, it's in everyone's interest to set the public record
> > straight.



-- 
Luiz Augusto von Dentz