On Mon, Mar 15, 2021 at 5:23 AM Vasily Averin <vvs@xxxxxxxxxxxxx> wrote:
>
> An untrusted netadmin inside a memcg-limited container can create a
> huge number of routing entries. Currently, allocated kernel objects
> are not accounted to proper memcg, so this can lead to global memory
> shortage on the host and cause lot of OOM kiils.
>
> One such object is the 'struct fib6_node' mostly allocated in
> net/ipv6/route.c::__ip6_ins_rt() inside the lock_bh()/unlock_bh() section:
>
> write_lock_bh(&table->tb6_lock);
> err = fib6_add(&table->tb6_root, rt, info, mxc);
> write_unlock_bh(&table->tb6_lock);
>
> It this case is
'In this case it is'
> not enough to simply add SLAB_ACCOUNT to corresponding
> kmem cache. The proper memory cgroup still cannot be found due to the
> incorrect 'in_interrupt()' check used in memcg_kmem_bypass().
> To be sure that caller is not executed in process contxt
'context'
> '!in_task()' check should be used instead
You missed the signoff and it seems like the whole series is missing
it as well. Please run scripts/checkpatch.pl on the patches before
sending again.
> ---
> mm/memcontrol.c | 2 +-
> net/ipv6/ip6_fib.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 845eec0..568f2cb 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1076,7 +1076,7 @@ static __always_inline bool memcg_kmem_bypass(void)
> return false;
>
> /* Memcg to charge can't be determined. */
> - if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD))
> + if (!in_task() || !current->mm || (current->flags & PF_KTHREAD))
Can you please also add some explanation in the commit message on the
differences between in_interrupt() and in_task()? Why is
in_interrupt() not correct here but !in_task() is? What about kernels
with or without PREEMPT_COUNT?
> return true;
>
> return false;
> diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
> index ef9d022..fa92ed1 100644
> --- a/net/ipv6/ip6_fib.c
> +++ b/net/ipv6/ip6_fib.c
> @@ -2445,7 +2445,7 @@ int __init fib6_init(void)
>
> fib6_node_kmem = kmem_cache_create("fib6_nodes",
> sizeof(struct fib6_node),
> - 0, SLAB_HWCACHE_ALIGN,
> + 0, SLAB_HWCACHE_ALIGN|SLAB_ACCOUNT,
> NULL);
> if (!fib6_node_kmem)
> goto out;
> --
> 1.8.3.1
>
