From: Luiz Augusto von Dentz <[email protected]>
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/[email protected]/T/#m1f9673e4ab0d55a7dccf87905337ab2e67d689f1
Fixes: 81be03e026dc ("Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg")
Signed-off-by: Luiz Augusto von Dentz <[email protected]>
---
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
Dear Luiz,
Am 15.02.22 um 02:59 schrieb Luiz Augusto von Dentz:
> From: Luiz Augusto von Dentz <[email protected]>
>
> 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 <[email protected]> (Nokia N9 (MeeGo/Harmattan)
> Link: https://lore.kernel.org/linux-bluetooth/[email protected]/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 <[email protected]>
Link:
https://lore.kernel.org/r/[email protected]
> Fixes: 81be03e026dc ("Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg")
> Signed-off-by: Luiz Augusto von Dentz <[email protected]>
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/
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/[email protected]/T/#m1f9673e4ab0d55a7dccf87905337ab2e67d689f1
> Fixes: 81be03e026dc ("Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg")
> Signed-off-by: Luiz Augusto von Dentz <[email protected]>
> ---
> include/net/bluetooth/bluetooth.h | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
[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/[email protected]/T/#m1f9673e4ab0d55a7dccf87905337ab2e67d689f1
>> Fixes: 81be03e026dc ("Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg")
>> Signed-off-by: Luiz Augusto von Dentz <[email protected]>
>> ---
>> 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: <[email protected]>" 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.
Hi Thorsten,
On Thu, Feb 24, 2022 at 6:09 AM Thorsten Leemhuis <[email protected]> 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/[email protected]/T/#m1f9673e4ab0d55a7dccf87905337ab2e67d689f1
> >>> Fixes: 81be03e026dc ("Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg")
> >>> Signed-off-by: Luiz Augusto von Dentz <[email protected]>
> >>> ---
> >>> 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: <[email protected]>" 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
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/[email protected]/T/#m1f9673e4ab0d55a7dccf87905337ab2e67d689f1
>>> Fixes: 81be03e026dc ("Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg")
>>> Signed-off-by: Luiz Augusto von Dentz <[email protected]>
>>> ---
>>> 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: <[email protected]>" 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.