linux-kernel - Re: [RFC PATCH 02/11] refcount: Implement inc/decrement-and-return functions

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]

Date:   Fri, 1 Sep 2017 23:50:03 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     David Howells <dhowells@...hat.com>
Cc:     linux-afs@...ts.infradead.org, linux-fsdevel@...r.kernel.org,
        Kees Cook <keescook@...omium.org>, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 02/11] refcount: Implement inc/decrement-and-return
 functions

On Fri, Sep 01, 2017 at 10:15:39PM +0100, David Howells wrote:
> Peter Zijlstra <peterz@...radead.org> wrote:
> 
> > > 	unsigned int refcount_dec_return(refcount_t *r);
> > > 	unsigned int refcount_inc_return(refcount_t *r);
> > > 
> > 
> > I'm not immediately seeing how wanting 1 to mean unused leads to
> > requiring these two functions.
> 
> Did you read the other other part of the description?
> 
> 	Further, both functions can be used to accurately trace the refcount
> 	(refcount_inc() followed by refcount_read() can't be considered
> 	accurate).

I must admit to having overlooked that. But can we treat the two issues
separately? They are quite distinct.

> > If you'll remember, I did that for inode_count and only needed
> > dec_unless().
> 
> I don't remember.  inode_count?  I can't find such a thing - did you mean
> i_count?  I don't find anything matching "dec_unless.*i_count" either.

Ah, yes, i_count. See these:

https://lkml.kernel.org/r/20170224162044.479190330@infradead.org
https://lkml.kernel.org/r/20170224162044.548813302@infradead.org

But looking at them, i_count was rather special, a normal GC based
scheme doesn't need anything new AFAICT:

add:
	spin_lock(&map->lock)
	refcount_set(&obj->refs, 1);
	map_link(map, obj);
	spin_unlock(&map->lock);

lookup:
	rcu_read_lock();
	obj = map_find(map, key);
	if (obj && !refcount_inc_not_zero(&obj->refs))
	  obj = NULL;
	rcu_read_unlock();

	if (obj) {
	  /* use obj */
	  refcount_dec(&obj->refs); /* should never hit 0 */
	}

GC:
	spin_lock(&map->lock);
	map_for_each_obj_safe(obj, map) {
	  if (refcount_dec_if_one(&obj->refs)) {
	    map_unlink(map, obj);
	    rcu_free(obj);
	  }
	}
	spin_unlock(&map->lock);