Re: [net-next,v9,02/15] net: build socket infrastructure for QUIC protocol

On Thu, Feb 5, 2026 at 7:48 AM Paolo Abeni <[email protected]> wrote:
>
> On 2/5/26 12:54 PM, Simon Horman wrote:
> > This is an AI-generated review of your patch. The human sending this
> > email has considered the AI review valid, or at least plausible.
> >
> > For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> > ---
> > net: build socket infrastructure for QUIC protocol
> >
> > This patch lays the groundwork for QUIC socket support in the kernel.
> > It defines the core structures and protocol hooks needed to create
> > QUIC sockets, without implementing any protocol behavior at this stage.
> > Basic integration is included to allow building the module via
> > CONFIG_IP_QUIC=m. This provides the scaffolding necessary for adding
> > actual QUIC socket behavior in follow-up patches.
> >
> >> diff --git a/net/quic/protocol.c b/net/quic/protocol.c
> >> new file mode 100644
> >> index 000000000000..6ab81f1c50cc
> >> --- /dev/null
> >> +++ b/net/quic/protocol.c
> >
> > [ ... ]
> >
> >> @@ -0,0 +1,369 @@
> >
> > [ ... ]
> >
> >> +static int quic_protosw_init(void)
> >> +{
> >> +    int err;
> >> +
> >> +    err = proto_register(&quic_prot, 1);
> >> +    if (err)
> >> +            return err;
> >> +
> >> +    err = proto_register(&quicv6_prot, 1);
> >> +    if (err) {
> >> +            proto_unregister(&quic_prot);
> >> +            return err;
> >> +    }
> >> +
> >> +    inet_register_protosw(&quic_stream_protosw);
> >> +    inet_register_protosw(&quic_dgram_protosw);
> >> +    inet6_register_protosw(&quicv6_stream_protosw);
> >> +    inet6_register_protosw(&quicv6_dgram_protosw);
> >> +
> >> +    return 0;
> >> +}
> >
> > Can the return values from inet6_register_protosw() be checked?
> >
> > Looking at the implementation, inet6_register_protosw() returns int and
> > can fail with -EINVAL (if p->type >= SOCK_MAX) or -EPERM (if trying to
> > override a permanent protocol). The calls at lines 234-235 don't check
> > these return values, so if registration fails, quic_protosw_init() still
> > returns 0 (success).
> >
> > More critically, quic_protosw_exit() unconditionally calls
> > inet6_unregister_protosw() on both structures:
> >
> > static void quic_protosw_exit(void)
> > {
> >       inet_unregister_protosw(&quic_dgram_protosw);
> >       inet_unregister_protosw(&quic_stream_protosw);
> >       proto_unregister(&quic_prot);
> >
> >       inet6_unregister_protosw(&quicv6_dgram_protosw);
> >       inet6_unregister_protosw(&quicv6_stream_protosw);
> >       proto_unregister(&quicv6_prot);
> > }
> >
> > If a protosw was never successfully registered, its list field remains
> > zero-initialized (NULL pointers). inet6_unregister_protosw() calls
> > list_del_rcu(&p->list), which with NULL pointers will dereference NULL
> > in __list_del_entry() -> __list_del(NULL, NULL), causing a kernel crash.
> >
> > Note that inet_register_protosw() returns void (cannot fail from caller's
> > perspective), but inet6_register_protosw() returns int (can fail). The
> > code correctly checks errors from proto_register() but not from
> > inet6_register_protosw().
> >
> > Should the return values from inet6_register_protosw() be checked, and
> > should the cleanup path track which registrations succeeded so only those
> > are unregistered?
>
> I think it's easy to infer that that such failures are impossible, but
> since a repost is needed please drop some sentence in the commit message
> to help AI see the point.
>
Will do, thanks.