| Message ID | 20220215015938.1950978-1-luiz.dentz@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Bluetooth: Fix bt_skb_sendmmsg not allocating partial chunks | expand |
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
[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.
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.
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.
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;