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  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]
Date:   Fri, 11 Jan 2019 09:12:57 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Kris Van Hees <kris.van.hees@...cle.com>
Cc:     netdev@...r.kernel.org, ast@...nel.org
Subject: Re: Potential memory leak in htab_map_update_elem?

Hi Kris,

On 01/11/2019 06:08 AM, Kris Van Hees wrote:
> Maybe I am missing something trivial here, but it looks to me that there is
> a leak of htab elements in htab_map_update_elem when you are updating an
> existing element.  After the new element is linked into the bucket list, the
> following code snippet is found:
> 
>         if (l_old) {
>                 hlist_nulls_del_rcu(&l_old->hash_node);
>                 if (!htab_is_prealloc(htab))
>                         free_htab_elem(htab, l_old);
>         }
> 
> Nothing is done with l_old in the remainder of the function, and to me this
> looks like that element is be leaked if the htab is preallocated because we
> never add it to the free list.  In fact, free_htab_elem() contains the very
> conditional that handles the two cases (preallocated vs non-preallocated.

In this case in alloc_htab_elem() we are swapping out the per-cpu extra element
with the existing one to avoid freelist_pop/push combination in order to have
a fast replace, meaning we cannot call free_htab_elem() on it like in plain
htab_map_delete_elem() case.

Thanks,
Daniel

Powered by blists - more mailing lists