netdev - Re: Reference counting struct inet

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: <alpine.LFD.2.11.1701240853370.1779@ja.home.ssi.bg>
Date:   Tue, 24 Jan 2017 09:38:37 +0200 (EET)
From:   Julian Anastasov <ja@....bg>
To:     David Windsor <dwindsor@...il.com>
cc:     netdev@...r.kernel.org, Kees Cook <keescook@...omium.org>,
        "Reshetova, Elena" <elena.reshetova@...el.com>,
        Hans Liljestrand <ishkamiel@...il.com>
Subject: Re: Reference counting struct inet_peer


	Hello,

On Mon, 23 Jan 2017, David Windsor wrote:

> Hi,
> 
> I'm working on a patchset that adds overflow protection to kernel
> reference counters, as part of the KSPP effort.  We're introducing a
> new type, tentatively called refcount_t, that will ultimately replace
> atomic_t as the type used for kernel reference counters.  refcount_t
> has a constrained interface relative to atomic_t and stores reference
> counts as unsigned integers.
> 
> While performing an audit of kernel reference counters, we've come
> upon a few corner cases that we're unable to cleanly migrate to
> refcount_t.  One of these is the reference counting scheme for struct
> inet_peer.

...

> We're also seeing the same thing (freeing shared objects when their
> refcount becomes -1) in ip_vs.h:
> 
> http://lxr.free-electrons.com/source/include/net/ip_vs.h#L1424
> 
> static inline void ip_vs_dest_put_and_free(struct ip_vs_dest *dest)
> {
>     if (atomic_dec_return(&dest->refcnt) < 0)
>         kfree(dest);
> }

	I think, this is easy to fix. The problem is that
dest_trash currently holds deleted dests (unlinked from RCU lists)
with refcnt=0. If we change the dest_trash to hold dest
with refcnt=1, the above atomic_dec_return can be changed to
atomic_dec_and_test. Change should be small: ip_vs_dest_put
should be removed from __ip_vs_del_dest(), ip_vs_dest_hold()
from ip_vs_trash_get_dest() and refcnt check in
ip_vs_dest_trash_expire() should be updated. Let me know if
this holds your work, I can provide such patch to fix it.

Regards

--
Julian Anastasov <ja@....bg>