netdev - Re: [PATCH v2 net] net: defer final 'struct net' free in netns dismantle

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <56f68ea6-9605-4120-afda-6ed5beb0c12d@redhat.com>
Date: Thu, 5 Dec 2024 23:09:10 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: "David S . Miller" <davem@...emloft.net>, Jakub Kicinski
 <kuba@...nel.org>, netdev@...r.kernel.org, eric.dumazet@...il.com,
 Ilya Maximets <i.maximets@....org>,
 Dan Streetman <dan.streetman@...onical.com>,
 Steffen Klassert <steffen.klassert@...unet.com>
Subject: Re: [PATCH v2 net] net: defer final 'struct net' free in netns
 dismantle

On 12/5/24 10:14, Eric Dumazet wrote:
> On Thu, Dec 5, 2024 at 9:35 AM Paolo Abeni <pabeni@...hat.com> wrote:
>>
>> On 12/4/24 13:54, Eric Dumazet wrote:
>>> Ilya reported a slab-use-after-free in dst_destroy [1]
>>>
>>> Issue is in xfrm6_net_init() and xfrm4_net_init() :
>>>
>>> They copy xfrm[46]_dst_ops_template into net->xfrm.xfrm[46]_dst_ops.
>>>
>>> But net structure might be freed before all the dst callbacks are
>>> called. So when dst_destroy() calls later :
>>>
>>> if (dst->ops->destroy)
>>>     dst->ops->destroy(dst);
>>>
>>> dst->ops points to the old net->xfrm.xfrm[46]_dst_ops, which has been freed.
>>>
>>> See a relevant issue fixed in :
>>>
>>> ac888d58869b ("net: do not delay dst_entries_add() in dst_release()")
>>>
>>> A fix is to queue the 'struct net' to be freed after one
>>> another cleanup_net() round (and existing rcu_barrier())
>>
>> I'm sorry for the late feedback.
>>
>> If I read correctly the above means that the actual free could be
>> delayed for an unlimited amount of time, did I misread something?
>>
>> I guess the reasoning is that the total amount of memory used by the
>> netns struct should be neglicible?
>>
>> I'm wondering about potential ill side effects WRT containers
>> deployments under memory pressure.
> 
> One net_namespace structure is about 3328 bytes today, a fraction of
> the overall cost of a live netns.
> 
> It would be very unlikely a deployment would have one cleanup_net(),
> adding these in a long list,
> then no other cleanup_net().

I agree it should be fine, I wanted to double check I did not misread
the patch nor missed any side effect.

Acked-by: Paolo Abeni <pabeni@...hat.com>

Thanks,

Paolo