linux-kernel - Re: [PATCH 004 of 19] knfsd: lockd: introduce nsm

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:	Sat, 2 Sep 2006 09:50:00 +1000
From:	Neil Brown <neilb@...e.de>
To:	Andrew Morton <akpm@...l.org>
Cc:	nfs@...ts.sourceforge.net, linux-kernel@...r.kernel.org,
	Olaf Kirch <okir@...e.de>
Subject: Re: [PATCH 004 of 19] knfsd: lockd: introduce nsm_handle

On Thursday August 31, akpm@...l.org wrote:
> On Fri, 1 Sep 2006 14:38:25 +1000
> NeilBrown <neilb@...e.de> wrote:
> 
> > +nsm_release(struct nsm_handle *nsm)
> > +{
> > +	if (!nsm)
> > +		return;
> > +	if (atomic_read(&nsm->sm_count) == 1) {
> > +		down(&nsm_sema);
> > +		if (atomic_dec_and_test(&nsm->sm_count)) {
> > +			list_del(&nsm->sm_link);
> > +			kfree(nsm);
> > +		}
> > +		up(&nsm_sema);
> > +	}
> > +}
> 
> That's weird-looking.  Are you sure?  afaict, if ->sm_count ever gets a
> value of 2 or more, it's unfreeable.  

How do you *do* that?  I thought I had already found the bugs...

Thanks :-)
NeilBrown


---
Subject: Fix locking in nsm_release

The locking is all backwards and broken.

We first atomic_dec_and_test.
If this fails, someone else has an active reference and we need do
no more.
If it succeeds, then the only ref is in the hash table, but someone
might be about to find and use that reference.  nsm_mutex provides
exclusion against this.  If sm_count is still 0 once the
mutex has been gained, then it is safe to discard the nsm.

Signed-off-by: Neil Brown <neilb@...e.de>

### Diffstat output
 ./fs/lockd/host.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff .prev/fs/lockd/host.c ./fs/lockd/host.c
--- .prev/fs/lockd/host.c	2006-09-01 18:56:39.000000000 +1000
+++ ./fs/lockd/host.c	2006-09-02 09:44:04.000000000 +1000
@@ -507,9 +507,9 @@ nsm_release(struct nsm_handle *nsm)
 {
 	if (!nsm)
 		return;
-	if (atomic_read(&nsm->sm_count) == 1) {
+	if (atomic_dec_and_test(&nsm->sm_count)) {
 		mutex_lock(&nsm_mutex);
-		if (atomic_dec_and_test(&nsm->sm_count)) {
+		if (atomic_read(&nsm->sm_count) == 0) {
 			list_del(&nsm->sm_link);
 			kfree(nsm);
 		}

-- 
VGER BF report: H 0
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/