[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <75AFEC8C-C050-4AC4-AF63-0B798B040502@gmail.com>
Date: Wed, 14 Aug 2024 16:30:02 +0900
From: Jeongjun Park <aha310510@...il.com>
To: "D. Wythe" <alibuda@...ux.alibaba.com>
Cc: davem@...emloft.net, dust.li@...ux.alibaba.com, edumazet@...gle.com,
gbayer@...ux.ibm.com, guwen@...ux.alibaba.com, jaka@...ux.ibm.com,
kuba@...nel.org, linux-kernel@...r.kernel.org, linux-s390@...r.kernel.org,
netdev@...r.kernel.org, pabeni@...hat.com,
syzbot+f69bfae0a4eb29976e44@...kaller.appspotmail.com,
tonylu@...ux.alibaba.com, wenjia@...ux.ibm.com
Subject: Re: [PATCH net,v3] net/smc: prevent NULL pointer dereference in txopt_get
> D. Wythe wrote:
>
>
>
>> 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
>
Wow, I didn't know this could be done this way. I'll test it with that code
and get back to you
Regards,
Jeongjun Park
>
>>
>> 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 = {
>>>>>>> --
>
Powered by blists - more mailing lists