ACK: [SRU][Bionic][PATCH 0/1] Fix WARN_ON in sk_stream_kill_queues

[Andrei Gherzan]

On 23/06/15 05:14PM, Thadeu Lima de Souza Cascardo wrote:
> On Thu, Jun 15, 2023 at 09:00:18PM +0100, Andrei Gherzan wrote:
> > BugLink: https://bugs.launchpad.net/bugs/2020279
> > 
> > [Impact]
> > 
> > The upstream commit 323fbd0edf3f ("net: dccp: Add handling of
> > IPV6_PKTOPTIONS to dccp_v6_do_rcv()") started to trigger WARN_ON in
> > sk_stream_kill_queues. Bionic doesn't have this commit but the same is
> > triggered with ca43ccf41224b023fc290073d5603a755fd12eed ("dccp/tcp:
> > Avoid negative sk_forward_alloc by ipv6_pinfo.pktoptions.") which is
> > included in Bionic.
> > 
> > [Fix]
> > 
> > Upstream already has the fix merged and also backported in various versions
> > including 4.14 from which the fix was cherry-picked for the Ubuntu
> > Bionic tree.
> > 
> > [Test Plan]
> > 
> > The bug was reproduced, and the fix was validated with the program
> > referenced in the patch's description:
> > https://lore.kernel.org/netdev/20230208004245.83497-1-kuniyu@amazon.com/
> > 
> > [Where problems could occur]
> > 
> > The regression can be considered low, since this commit was backported
> > on various versions from 4.14 to 6.2 (where the initial fix landed) and
> > has been both reproduced and validated at runtime.
> > 
> 
> I like how instead of having this section called "Potential regression", it
> is titled "*Where* problems could occur" (emphasis mine). But in the body
> of the section, you are stating "how likely" you think you may hit a
> regression or why you think problems cannot occur, not *where* they would
> likely occur. And by where I would like it to mean where in the code, which
> subsystem, thinks like that.
> 
> I don't think we should dismiss the reasons why we think this unlikely will
> cause a regression, but we can't keep ignoring the "where".

Thanks for the feedback. I can confirm that my intension was not to
ignore the "where" but follow the sample in StablePatchFormat wiki
page[1]:

[Where problems could occur]
* The regression can be considered as low, since:
[...]

We should update this to make it more straightforward and avoid it in
the future. I will keep this in mind for further submissions.

[1] https://wiki.ubuntu.com/Kernel/Dev/StablePatchFormat

> 
> Acked-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>
> 
> > [Other Info]
> > 
> > All the Ubuntu kernel trees have been checked for this bug and only
> > Bionic is affected. The earlier tree don't have the commits that trigger
> > this bug while the later trees already include the fix.
> > 
> > Kuniyuki Iwashima (1):
> >   net: Remove WARN_ON_ONCE(sk->sk_forward_alloc) from
> >     sk_stream_kill_queues().
> > 
> >  net/caif/caif_socket.c | 1 +
> >  net/core/stream.c      | 1 -
> >  2 files changed, 1 insertion(+), 1 deletion(-)
> > 

Thanks for applying the patch.

Regards,
-- 
Andrei Gherzan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20230626/67d71e14/attachment.sig>