Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 28 Nov 2016 14:12:19 +0000
From: "Reshetova, Elena" <>
To: Peter Zijlstra <>
CC: ""
	<>, Greg KH <>,
	Kees Cook <>, ""
	<>, Boqun Feng <>, Hans Liljestrand
	<>, David Windsor <>, ""
	<>, "" <>
Subject: RE: Conversion from atomic_t to refcount_t: summary of issues

> > Also, refcount_add() seems to be needed in number of places since it
> > looks like refcounts in some cases are increased by two or by some
> > constant.  Luckily we haven't seen a need a sub().
> There is sub_and_test() usage in for example memcontrol.c.

Right (sub_and_test is one pattern I haven't processed yet, but I am *hoping* that there are not that many).
Also now I run into atomic_sub(len -1, &sk->sk_wmem_alloc) in net/core/sock.c

> > The following functions are also needed quite commonly:
> > refcount_inc_return()
> > refcount_dec_return()
> What for? They don't typicaly make sense for refcounting? Other than the
> trivial pattern of dec_return() == 0, which is already well covered.

Well, I guess you have to ask developers what for. They do verify the return to be equal to different numbers or values...
Sometimes it is ">0", sometimes "==1", sometimes "!=-1". These all seem to be border cases and checked in some scenarios. 

> > I also saw one use of this from net/ipv4/udp.c:
> >     if (!sk || !atomic_inc_not_zero_hint(&sk->sk_refcnt, 2))
> Yes, that one is quite unfortunate, we can trivially support that
> ofcourse, but it does make a bit of a mess of things.
> > Lastly as I mentioned previously, almost half of invocations of dec()
> > in the code is plain atomic_dec() without any if statements and any
> > checks on what happens as a result of dec().  Peter previously
> > suggested to turn them into WARN_ON(refcount_dec_and_test()), but
> > looking in the code, it is not really clear what would this help to
> > achieve?
> Well, it clearly marks where refcounting goes bad and we leak crap. A
> regular decrement should _never_ hit 0.
> > It is clear that in that places the caller explicitly
> > doesn't care about how the dec() goes and what is the end result....
> No, the typical usage would be you _know_ it will not hit 0. Any other
> usage is broken and bad.

Ok, makes sense. 

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.