linux-kernel - Re: [PATCH 0/3] Convert nsproxy, groups, and creds to refcount

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:   Tue, 21 Jul 2020 12:02:19 +0100
From:   David Howells <dhowells@...hat.com>
To:     "Reshetova, Elena" <elena.reshetova@...el.com>
Cc:     dhowells@...hat.com, Kees Cook <keescook@...omium.org>,
        Xiaoming Ni <nixiaoming@...wei.com>,
        Peter Zijlstra <peterz@...radead.org>,
        David Windsor <dwindsor@...il.com>,
        Hans Liljestrand <ishkamiel@...il.com>,
        Paul Moore <paul@...l-moore.com>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "paulmck@...nel.org" <paulmck@...nel.org>,
        "shakeelb@...gle.com" <shakeelb@...gle.com>,
        James Morris <jamorris@...ux.microsoft.com>,
        "alex.huangjianhui@...wei.com" <alex.huangjianhui@...wei.com>,
        "dylix.dailei@...wei.com" <dylix.dailei@...wei.com>,
        "chenzefeng2@...wei.com" <chenzefeng2@...wei.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 0/3] Convert nsproxy, groups, and creds to refcount_t

> https://github.com/ereshetova/linux-stable/commits/refcount_t_fs

Looking at "fs, cachefiles: convert cachefiles_object.usage from atomic_t to
refcount_t", I see:

-	u = atomic_inc_return(&object->usage);
+	refcount_inc(&object->usage);
	trace_cachefiles_ref(object, _object->cookie,
-			     (enum cachefiles_obj_ref_trace)why, u);
+			     (enum cachefiles_obj_ref_trace)why, refcount_read(&object->usage));
	return &object->fscache;

This change is *not* equivalent.  There's a reason I'm using
atomic_inc_return() and not atomic_inc(),atomic_read().  Yes, the small window
*does* occasionally produce incorrect tracing, and, yes, when that happens it
does make things confusing.

-	u = atomic_dec_return(&object->usage);
	trace_cachefiles_ref(object, _object->cookie,
-			     (enum cachefiles_obj_ref_trace)why, u);
-	ASSERTCMP(u, !=, -1);
-	if (u == 0) {
+			     (enum cachefiles_obj_ref_trace)why, refcount_read(&object->usage) - 1);
+	if (refcount_dec_and_test(&object->usage)) {

This is also not equivalent.  Again, there's a reason I'm using
atomic_dec_return() and not atomic_read(),atomic_dec_and_test().

So, please drop the cachefiles/fscache patches or use refcount_inc_return()
and refcount_dec_return().

Another reason to please drop these patches is that they will cause my
fscache-iter branch to bounce.  A lot of this code is deleted/heavily
rewritten:

https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=fscache-iter

Thanks,
David