From mboxrd@z Thu Jan 1 00:00:00 1970 From: Patrick McHardy Subject: Re: BUG? a possible race between htable_find_get() and htable_put() Date: Wed, 13 Jan 2010 07:41:15 +0100 Message-ID: <4B4D6B0B.1070106@trash.net> References: <2014bcab1001121851g28e8e7d3x5ed3604b6854a0ed@mail.gmail.com> <4B4D6A8E.8000303@trash.net> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------030801060700020807060802" Cc: netfilter-devel@vger.kernel.org To: =?UTF-8?B?7ZmN7IugIHNoaW4gaG9uZw==?= Return-path: Received: from stinky.trash.net ([213.144.137.162]:35670 "EHLO stinky.trash.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751118Ab0AMGlS (ORCPT ); Wed, 13 Jan 2010 01:41:18 -0500 In-Reply-To: <4B4D6A8E.8000303@trash.net> Sender: netfilter-devel-owner@vger.kernel.org List-ID: This is a multi-part message in MIME format. --------------030801060700020807060802 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Patrick McHardy wrote: > 홍신 shin hong wrote: >> Hi. I am reporting a suspected race between htable_find_get() >> and htable_put() in net/netfilter/xt_hashlimit.c. >> >> I found this issue while I read the code so that it might not realistic. >> But, please examine the code to check possibility of race condition. >> >> htable_put() first updates hinfo->use and then unlink the object from the list. >> But, htable_find_get() first searches an object from the list, >> and then updates hinfo->use. > > Nice catch, this does indeed look like a bug. The entire locking > concept seems a bit strange, we neither need an atomic_t for the > reference count nor two locks to protect the list. This patch > changes the code to use the hashlimit_mutex for list and reference > count protection. > > I'll commit this later unless someone can spot further bugs :) Locking around list removal and destruction was missing from the previous patch, fixed version attached. --------------030801060700020807060802 Content-Type: text/plain; name="x" Content-Transfer-Encoding: base64 Content-Disposition: inline; filename="x" ZGlmZiAtLWdpdCBhL25ldC9uZXRmaWx0ZXIveHRfaGFzaGxpbWl0LmMgYi9uZXQvbmV0Zmls dGVyL3h0X2hhc2hsaW1pdC5jCmluZGV4IGRkMTZlNDAuLjRhNzIwNDQgMTAwNjQ0Ci0tLSBh L25ldC9uZXRmaWx0ZXIveHRfaGFzaGxpbWl0LmMKKysrIGIvbmV0L25ldGZpbHRlci94dF9o YXNobGltaXQuYwpAQCAtNzksNyArNzksNyBAQCBzdHJ1Y3QgZHN0aGFzaF9lbnQgewogCiBz dHJ1Y3QgeHRfaGFzaGxpbWl0X2h0YWJsZSB7CiAJc3RydWN0IGhsaXN0X25vZGUgbm9kZTsJ CS8qIGdsb2JhbCBsaXN0IG9mIGFsbCBodGFibGVzICovCi0JYXRvbWljX3QgdXNlOworCWlu dCB1c2U7CiAJdV9pbnQ4X3QgZmFtaWx5OwogCiAJc3RydWN0IGhhc2hsaW1pdF9jZmcxIGNm ZzsJLyogY29uZmlnICovCkBAIC05Nyw4ICs5Nyw3IEBAIHN0cnVjdCB4dF9oYXNobGltaXRf aHRhYmxlIHsKIAlzdHJ1Y3QgaGxpc3RfaGVhZCBoYXNoWzBdOwkvKiBoYXNodGFibGUgaXRz ZWxmICovCiB9OwogCi1zdGF0aWMgREVGSU5FX1NQSU5MT0NLKGhhc2hsaW1pdF9sb2NrKTsJ LyogcHJvdGVjdHMgaHRhYmxlcyBsaXN0ICovCi1zdGF0aWMgREVGSU5FX01VVEVYKGhsaW1p dF9tdXRleCk7CS8qIGFkZGl0aW9uYWwgY2hlY2tlbnRyeSBwcm90ZWN0aW9uICovCitzdGF0 aWMgREVGSU5FX01VVEVYKGhhc2hsaW1pdF9tdXRleCk7CS8qIHByb3RlY3RzIGh0YWJsZXMg bGlzdCAqLwogc3RhdGljIEhMSVNUX0hFQUQoaGFzaGxpbWl0X2h0YWJsZXMpOwogc3RhdGlj IHN0cnVjdCBrbWVtX2NhY2hlICpoYXNobGltaXRfY2FjaGVwIF9fcmVhZF9tb3N0bHk7CiAK QEAgLTIzMiw3ICsyMzEsNyBAQCBzdGF0aWMgaW50IGh0YWJsZV9jcmVhdGVfdjAoc3RydWN0 IHh0X2hhc2hsaW1pdF9pbmZvICptaW5mbywgdV9pbnQ4X3QgZmFtaWx5KQogCWZvciAoaSA9 IDA7IGkgPCBoaW5mby0+Y2ZnLnNpemU7IGkrKykKIAkJSU5JVF9ITElTVF9IRUFEKCZoaW5m by0+aGFzaFtpXSk7CiAKLQlhdG9taWNfc2V0KCZoaW5mby0+dXNlLCAxKTsKKwloaW5mby0+ dXNlID0gMTsKIAloaW5mby0+Y291bnQgPSAwOwogCWhpbmZvLT5mYW1pbHkgPSBmYW1pbHk7 CiAJaGluZm8tPnJuZF9pbml0aWFsaXplZCA9IDA7CkBAIC0yNTAsOSArMjQ5LDkgQEAgc3Rh dGljIGludCBodGFibGVfY3JlYXRlX3YwKHN0cnVjdCB4dF9oYXNobGltaXRfaW5mbyAqbWlu Zm8sIHVfaW50OF90IGZhbWlseSkKIAloaW5mby0+dGltZXIuZXhwaXJlcyA9IGppZmZpZXMg KyBtc2Vjc190b19qaWZmaWVzKGhpbmZvLT5jZmcuZ2NfaW50ZXJ2YWwpOwogCWFkZF90aW1l cigmaGluZm8tPnRpbWVyKTsKIAotCXNwaW5fbG9ja19iaCgmaGFzaGxpbWl0X2xvY2spOwor CW11dGV4X2xvY2soJmhhc2hsaW1pdF9tdXRleCk7CiAJaGxpc3RfYWRkX2hlYWQoJmhpbmZv LT5ub2RlLCAmaGFzaGxpbWl0X2h0YWJsZXMpOwotCXNwaW5fdW5sb2NrX2JoKCZoYXNobGlt aXRfbG9jayk7CisJbXV0ZXhfdW5sb2NrKCZoYXNobGltaXRfbXV0ZXgpOwogCiAJcmV0dXJu IDA7CiB9CkBAIC0yOTMsNyArMjkyLDcgQEAgc3RhdGljIGludCBodGFibGVfY3JlYXRlKHN0 cnVjdCB4dF9oYXNobGltaXRfbXRpbmZvMSAqbWluZm8sIHVfaW50OF90IGZhbWlseSkKIAlm b3IgKGkgPSAwOyBpIDwgaGluZm8tPmNmZy5zaXplOyBpKyspCiAJCUlOSVRfSExJU1RfSEVB RCgmaGluZm8tPmhhc2hbaV0pOwogCi0JYXRvbWljX3NldCgmaGluZm8tPnVzZSwgMSk7CisJ aGluZm8tPnVzZSA9IDE7CiAJaGluZm8tPmNvdW50ID0gMDsKIAloaW5mby0+ZmFtaWx5ID0g ZmFtaWx5OwogCWhpbmZvLT5ybmRfaW5pdGlhbGl6ZWQgPSAwOwpAQCAtMzEyLDkgKzMxMSw5 IEBAIHN0YXRpYyBpbnQgaHRhYmxlX2NyZWF0ZShzdHJ1Y3QgeHRfaGFzaGxpbWl0X210aW5m bzEgKm1pbmZvLCB1X2ludDhfdCBmYW1pbHkpCiAJaGluZm8tPnRpbWVyLmV4cGlyZXMgPSBq aWZmaWVzICsgbXNlY3NfdG9famlmZmllcyhoaW5mby0+Y2ZnLmdjX2ludGVydmFsKTsKIAlh ZGRfdGltZXIoJmhpbmZvLT50aW1lcik7CiAKLQlzcGluX2xvY2tfYmgoJmhhc2hsaW1pdF9s b2NrKTsKKwltdXRleF9sb2NrKCZoYXNobGltaXRfbXV0ZXgpOwogCWhsaXN0X2FkZF9oZWFk KCZoaW5mby0+bm9kZSwgJmhhc2hsaW1pdF9odGFibGVzKTsKLQlzcGluX3VubG9ja19iaCgm aGFzaGxpbWl0X2xvY2spOworCW11dGV4X3VubG9jaygmaGFzaGxpbWl0X211dGV4KTsKIAog CXJldHVybiAwOwogfQpAQCAtMzgwLDI3ICszNzksMjQgQEAgc3RhdGljIHN0cnVjdCB4dF9o YXNobGltaXRfaHRhYmxlICpodGFibGVfZmluZF9nZXQoY29uc3QgY2hhciAqbmFtZSwKIAlz dHJ1Y3QgeHRfaGFzaGxpbWl0X2h0YWJsZSAqaGluZm87CiAJc3RydWN0IGhsaXN0X25vZGUg KnBvczsKIAotCXNwaW5fbG9ja19iaCgmaGFzaGxpbWl0X2xvY2spOwogCWhsaXN0X2Zvcl9l YWNoX2VudHJ5KGhpbmZvLCBwb3MsICZoYXNobGltaXRfaHRhYmxlcywgbm9kZSkgewogCQlp ZiAoIXN0cmNtcChuYW1lLCBoaW5mby0+cGRlLT5uYW1lKSAmJgogCQkgICAgaGluZm8tPmZh bWlseSA9PSBmYW1pbHkpIHsKLQkJCWF0b21pY19pbmMoJmhpbmZvLT51c2UpOwotCQkJc3Bp bl91bmxvY2tfYmgoJmhhc2hsaW1pdF9sb2NrKTsKKwkJCWhpbmZvLT51c2UrKzsKIAkJCXJl dHVybiBoaW5mbzsKIAkJfQogCX0KLQlzcGluX3VubG9ja19iaCgmaGFzaGxpbWl0X2xvY2sp OwogCXJldHVybiBOVUxMOwogfQogCiBzdGF0aWMgdm9pZCBodGFibGVfcHV0KHN0cnVjdCB4 dF9oYXNobGltaXRfaHRhYmxlICpoaW5mbykKIHsKLQlpZiAoYXRvbWljX2RlY19hbmRfdGVz dCgmaGluZm8tPnVzZSkpIHsKLQkJc3Bpbl9sb2NrX2JoKCZoYXNobGltaXRfbG9jayk7CisJ bXV0ZXhfbG9jaygmaGFzaGxpbWl0X211dGV4KTsKKwlpZiAoLS1oaW5mby0+dXNlID09IDAp IHsKIAkJaGxpc3RfZGVsKCZoaW5mby0+bm9kZSk7Ci0JCXNwaW5fdW5sb2NrX2JoKCZoYXNo bGltaXRfbG9jayk7CiAJCWh0YWJsZV9kZXN0cm95KGhpbmZvKTsKIAl9CisJbXV0ZXhfdW5s b2NrKCZoYXNobGltaXRfbXV0ZXgpOwogfQogCiAvKiBUaGUgYWxnb3JpdGhtIHVzZWQgaXMg dGhlIFNpbXBsZSBUb2tlbiBCdWNrZXQgRmlsdGVyIChUQkYpCkBAIC02ODcsMTkgKzY4Mywx MyBAQCBzdGF0aWMgYm9vbCBoYXNobGltaXRfbXRfY2hlY2tfdjAoY29uc3Qgc3RydWN0IHh0 X210Y2hrX3BhcmFtICpwYXIpCiAJaWYgKHItPm5hbWVbc2l6ZW9mKHItPm5hbWUpIC0gMV0g IT0gJ1wwJykKIAkJcmV0dXJuIGZhbHNlOwogCi0JLyogVGhpcyBpcyB0aGUgYmVzdCB3ZSd2 ZSBnb3Q6IFdlIGNhbm5vdCByZWxlYXNlIGFuZCByZS1ncmFiIGxvY2ssCi0JICogc2luY2Ug Y2hlY2tlbnRyeSgpIGlzIGNhbGxlZCBiZWZvcmUgeF90YWJsZXMuYyBncmFicyB4dF9tdXRl eC4KLQkgKiBXZSBhbHNvIGNhbm5vdCBncmFiIHRoZSBoYXNodGFibGUgc3BpbmxvY2ssIHNp bmNlIGh0YWJsZV9jcmVhdGUgd2lsbAotCSAqIGNhbGwgdm1hbGxvYywgYW5kIHRoYXQgY2Fu IHNsZWVwLiAgQW5kIHdlIGNhbm5vdCBqdXN0IHJlLXNlYXJjaAotCSAqIHRoZSBsaXN0IG9m IGh0YWJsZSdzIGluIGh0YWJsZV9jcmVhdGUoKSwgc2luY2UgdGhlbiB3ZSB3b3VsZAotCSAq IGNyZWF0ZSBkdXBsaWNhdGUgcHJvYyBmaWxlcy4gLUhXICovCi0JbXV0ZXhfbG9jaygmaGxp bWl0X211dGV4KTsKKwltdXRleF9sb2NrKCZoYXNobGltaXRfbXV0ZXgpOwogCXItPmhpbmZv ID0gaHRhYmxlX2ZpbmRfZ2V0KHItPm5hbWUsIHBhci0+bWF0Y2gtPmZhbWlseSk7CiAJaWYg KCFyLT5oaW5mbyAmJiBodGFibGVfY3JlYXRlX3YwKHIsIHBhci0+bWF0Y2gtPmZhbWlseSkg IT0gMCkgewotCQltdXRleF91bmxvY2soJmhsaW1pdF9tdXRleCk7CisJCW11dGV4X3VubG9j aygmaGFzaGxpbWl0X211dGV4KTsKIAkJcmV0dXJuIGZhbHNlOwogCX0KLQltdXRleF91bmxv Y2soJmhsaW1pdF9tdXRleCk7CisJbXV0ZXhfdW5sb2NrKCZoYXNobGltaXRfbXV0ZXgpOwog CiAJcmV0dXJuIHRydWU7CiB9CkBAIC03MjgsMTkgKzcxOCwxMyBAQCBzdGF0aWMgYm9vbCBo YXNobGltaXRfbXRfY2hlY2soY29uc3Qgc3RydWN0IHh0X210Y2hrX3BhcmFtICpwYXIpCiAJ CQlyZXR1cm4gZmFsc2U7CiAJfQogCi0JLyogVGhpcyBpcyB0aGUgYmVzdCB3ZSd2ZSBnb3Q6 IFdlIGNhbm5vdCByZWxlYXNlIGFuZCByZS1ncmFiIGxvY2ssCi0JICogc2luY2UgY2hlY2tl bnRyeSgpIGlzIGNhbGxlZCBiZWZvcmUgeF90YWJsZXMuYyBncmFicyB4dF9tdXRleC4KLQkg KiBXZSBhbHNvIGNhbm5vdCBncmFiIHRoZSBoYXNodGFibGUgc3BpbmxvY2ssIHNpbmNlIGh0 YWJsZV9jcmVhdGUgd2lsbAotCSAqIGNhbGwgdm1hbGxvYywgYW5kIHRoYXQgY2FuIHNsZWVw LiAgQW5kIHdlIGNhbm5vdCBqdXN0IHJlLXNlYXJjaAotCSAqIHRoZSBsaXN0IG9mIGh0YWJs ZSdzIGluIGh0YWJsZV9jcmVhdGUoKSwgc2luY2UgdGhlbiB3ZSB3b3VsZAotCSAqIGNyZWF0 ZSBkdXBsaWNhdGUgcHJvYyBmaWxlcy4gLUhXICovCi0JbXV0ZXhfbG9jaygmaGxpbWl0X211 dGV4KTsKKwltdXRleF9sb2NrKCZoYXNobGltaXRfbXV0ZXgpOwogCWluZm8tPmhpbmZvID0g aHRhYmxlX2ZpbmRfZ2V0KGluZm8tPm5hbWUsIHBhci0+bWF0Y2gtPmZhbWlseSk7CiAJaWYg KCFpbmZvLT5oaW5mbyAmJiBodGFibGVfY3JlYXRlKGluZm8sIHBhci0+bWF0Y2gtPmZhbWls eSkgIT0gMCkgewotCQltdXRleF91bmxvY2soJmhsaW1pdF9tdXRleCk7CisJCW11dGV4X3Vu bG9jaygmaGFzaGxpbWl0X211dGV4KTsKIAkJcmV0dXJuIGZhbHNlOwogCX0KLQltdXRleF91 bmxvY2soJmhsaW1pdF9tdXRleCk7CisJbXV0ZXhfdW5sb2NrKCZoYXNobGltaXRfbXV0ZXgp OwogCXJldHVybiB0cnVlOwogfQogCg== --------------030801060700020807060802--