Re: [PATCH net,v3] net/smc: prevent NULL pointer dereference in txopt_get

[D. Wythe]

On 8/14/24 11:58 AM, Jeongjun Park wrote:
> Because clcsk_*, like clcsock, is initialized during the smc init process,
> the code was moved to prevent clcsk_* from having an address like
> inet_sk(sk)->pinet6, thereby preventing the previously initialized values
> ​​from being tampered with.

I don't agree with your approach, but I finally got the problem you 
described. In fact, the issue here is that smc_sock should also be an 
inet_sock, whereas currently it's just a sock. Therefore, the best 
solution would be to embed an inet_sock within smc_sock rather than 
performing this movement as you suggested.

struct smc_sock {               /* smc sock container */
    union {
        struct sock         sk;
        struct inet_sock    inet;
    };

    ...

Thanks.
D. Wythe


> Additionally, if you don't need alignment in smc_inet6_prot , I'll modify
> the patch to only add the necessary code without alignment.
>
> Regards,
> Jeongjun Park


> > > Also, regarding alignment, it's okay for me whether it's aligned or
> > > not，But I checked the styles of other types of
> > > structures and did not strictly require alignment, so I now feel that
> > > there is no need to
> > > modify so much to do alignment.
> > >
> > > D. Wythe
> >
> >
> > > > > > +
> > > > > >     static struct proto smc_inet6_prot = {
> > > > > > -     .name           = "INET6_SMC",
> > > > > > -     .owner          = THIS_MODULE,
> > > > > > -     .init           = smc_inet_init_sock,
> > > > > > -     .hash           = smc_hash_sk,
> > > > > > -     .unhash         = smc_unhash_sk,
> > > > > > -     .release_cb     = smc_release_cb,
> > > > > > -     .obj_size       = sizeof(struct smc_sock),
> > > > > > -     .h.smc_hash     = &smc_v6_hashinfo,
> > > > > > -     .slab_flags     = SLAB_TYPESAFE_BY_RCU,
> > > > > > +     .name                           = "INET6_SMC",
> > > > > > +     .owner                          = THIS_MODULE,
> > > > > > +     .init                           = smc_inet_init_sock,
> > > > > > +     .hash                           = smc_hash_sk,
> > > > > > +     .unhash                         = smc_unhash_sk,
> > > > > > +     .release_cb                     = smc_release_cb,
> > > > > > +     .obj_size                       = sizeof(struct smc6_sock),
> > > > > > +     .h.smc_hash                     = &smc_v6_hashinfo,
> > > > > > +     .slab_flags                     = SLAB_TYPESAFE_BY_RCU,
> > > > > > +     .ipv6_pinfo_offset              = offsetof(struct smc6_sock,
> > > > > > np),
> > > > > >     };
> > > > > >
> > > > > >     static const struct proto_ops smc_inet6_stream_ops = {
> > > > > > --