From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Dumazet Subject: Re: [PATCH net] af_key: free SKBs under RCU protection Date: Sun, 23 Sep 2018 10:15:41 -0700 Message-ID: <9a3d8036-9f8c-6e8b-f16c-7c278c448db3@gmail.com> References: <1537402712-12875-1-git-send-email-stranche@codeaurora.org> <6d194ac2-15e8-76d7-31d0-b4c4eed68d5a@gmail.com> <357e28c3fa0c7bacaffde4e960f58a87@codeaurora.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------6843093CD031C54901D418C2" Cc: netdev@vger.kernel.org, steffen.klassert@secunet.com To: stranche@codeaurora.org, Eric Dumazet Return-path: Received: from mail-pg1-f193.google.com ([209.85.215.193]:42305 "EHLO mail-pg1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726904AbeIWXNy (ORCPT ); Sun, 23 Sep 2018 19:13:54 -0400 Received: by mail-pg1-f193.google.com with SMTP id y4-v6so8143376pgp.9 for ; Sun, 23 Sep 2018 10:15:43 -0700 (PDT) In-Reply-To: <357e28c3fa0c7bacaffde4e960f58a87@codeaurora.org> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: This is a multi-part message in MIME format. --------------6843093CD031C54901D418C2 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 09/20/2018 12:25 PM, stranche@codeaurora.org wrote: > Perhaps a cleaner solution here is to always clone the SKB in > pfkey_broadcast_one(). That will ensure that the two kfree_skb() calls > in pfkey_broadcast() will never be passed an SKB with sock_rfree() as > its destructor, and we can avoid this race condition. Yes, this whole idea of avoiding the cloning is brain dead. Better play safe and having a straightforward implementation. I suggest something like this (I could not reproduce the bug with the syzkaller repro) Note that I removed the sock_hold(sk)/sock_put(sk) pair as this is useless. The only time GFP_KERNEL might be used is when the sk is already owned by the caller. net/key/af_key.c | 40 +++++++++++++++------------------------- 1 file changed, 15 insertions(+), 25 deletions(-) --------------6843093CD031C54901D418C2 Content-Type: text/plain; charset=UTF-8; name="patch2712.txt" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="patch2712.txt" ZGlmZiAtLWdpdCBhL25ldC9rZXkvYWZfa2V5LmMgYi9uZXQva2V5L2FmX2tleS5jCmluZGV4 IDlkNjEyNjY1MjZlNzY3NzcwZDlhMWNlMTg0YWM4Y2RkNTlkZTMwOWEuLjdkYTYyOWQ1OTcx NzEyZDUyMTk1MjhjNTViYWQ4NjliYjA4NGEzNDMgMTAwNjQ0Ci0tLSBhL25ldC9rZXkvYWZf a2V5LmMKKysrIGIvbmV0L2tleS9hZl9rZXkuYwpAQCAtMTk2LDMwICsxOTYsMjIgQEAgc3Rh dGljIGludCBwZmtleV9yZWxlYXNlKHN0cnVjdCBzb2NrZXQgKnNvY2spCiAJcmV0dXJuIDA7 CiB9CiAKLXN0YXRpYyBpbnQgcGZrZXlfYnJvYWRjYXN0X29uZShzdHJ1Y3Qgc2tfYnVmZiAq c2tiLCBzdHJ1Y3Qgc2tfYnVmZiAqKnNrYjIsCi0JCQkgICAgICAgZ2ZwX3QgYWxsb2NhdGlv biwgc3RydWN0IHNvY2sgKnNrKQorc3RhdGljIGludCBwZmtleV9icm9hZGNhc3Rfb25lKHN0 cnVjdCBza19idWZmICpza2IsIGdmcF90IGFsbG9jYXRpb24sCisJCQkgICAgICAgc3RydWN0 IHNvY2sgKnNrKQogewogCWludCBlcnIgPSAtRU5PQlVGUzsKIAotCXNvY2tfaG9sZChzayk7 Ci0JaWYgKCpza2IyID09IE5VTEwpIHsKLQkJaWYgKHJlZmNvdW50X3JlYWQoJnNrYi0+dXNl cnMpICE9IDEpIHsKLQkJCSpza2IyID0gc2tiX2Nsb25lKHNrYiwgYWxsb2NhdGlvbik7Ci0J CX0gZWxzZSB7Ci0JCQkqc2tiMiA9IHNrYjsKLQkJCXJlZmNvdW50X2luYygmc2tiLT51c2Vy cyk7Ci0JCX0KLQl9Ci0JaWYgKCpza2IyICE9IE5VTEwpIHsKLQkJaWYgKGF0b21pY19yZWFk KCZzay0+c2tfcm1lbV9hbGxvYykgPD0gc2stPnNrX3JjdmJ1ZikgewotCQkJc2tiX3NldF9v d25lcl9yKCpza2IyLCBzayk7Ci0JCQlza2JfcXVldWVfdGFpbCgmc2stPnNrX3JlY2VpdmVf cXVldWUsICpza2IyKTsKLQkJCXNrLT5za19kYXRhX3JlYWR5KHNrKTsKLQkJCSpza2IyID0g TlVMTDsKLQkJCWVyciA9IDA7Ci0JCX0KKwlpZiAoYXRvbWljX3JlYWQoJnNrLT5za19ybWVt X2FsbG9jKSA+IHNrLT5za19yY3ZidWYpCisJCXJldHVybiBlcnI7CisKKwlza2IgPSBza2Jf Y2xvbmUoc2tiLCBhbGxvY2F0aW9uKTsKKworCWlmIChza2IpIHsKKwkJc2tiX3NldF9vd25l cl9yKHNrYiwgc2spOworCQlza2JfcXVldWVfdGFpbCgmc2stPnNrX3JlY2VpdmVfcXVldWUs IHNrYik7CisJCXNrLT5za19kYXRhX3JlYWR5KHNrKTsKKwkJZXJyID0gMDsKIAl9Ci0Jc29j a19wdXQoc2spOwogCXJldHVybiBlcnI7CiB9CiAKQEAgLTIzNCw3ICsyMjYsNiBAQCBzdGF0 aWMgaW50IHBma2V5X2Jyb2FkY2FzdChzdHJ1Y3Qgc2tfYnVmZiAqc2tiLCBnZnBfdCBhbGxv Y2F0aW9uLAogewogCXN0cnVjdCBuZXRuc19wZmtleSAqbmV0X3Bma2V5ID0gbmV0X2dlbmVy aWMobmV0LCBwZmtleV9uZXRfaWQpOwogCXN0cnVjdCBzb2NrICpzazsKLQlzdHJ1Y3Qgc2tf YnVmZiAqc2tiMiA9IE5VTEw7CiAJaW50IGVyciA9IC1FU1JDSDsKIAogCS8qIFhYWCBEbyB3 ZSBuZWVkIHNvbWV0aGluZyBsaWtlIG5ldGxpbmtfb3ZlcnJ1bj8gIEkgdGhpbmsKQEAgLTI1 Myw3ICsyNDQsNyBAQCBzdGF0aWMgaW50IHBma2V5X2Jyb2FkY2FzdChzdHJ1Y3Qgc2tfYnVm ZiAqc2tiLCBnZnBfdCBhbGxvY2F0aW9uLAogCQkgKiBzb2NrZXQuCiAJCSAqLwogCQlpZiAo cGZrLT5wcm9taXNjKQotCQkJcGZrZXlfYnJvYWRjYXN0X29uZShza2IsICZza2IyLCBHRlBf QVRPTUlDLCBzayk7CisJCQlwZmtleV9icm9hZGNhc3Rfb25lKHNrYiwgR0ZQX0FUT01JQywg c2spOwogCiAJCS8qIHRoZSBleGFjdCB0YXJnZXQgd2lsbCBiZSBwcm9jZXNzZWQgbGF0ZXIg Ki8KIAkJaWYgKHNrID09IG9uZV9zaykKQEAgLTI2OCw3ICsyNTksNyBAQCBzdGF0aWMgaW50 IHBma2V5X2Jyb2FkY2FzdChzdHJ1Y3Qgc2tfYnVmZiAqc2tiLCBnZnBfdCBhbGxvY2F0aW9u LAogCQkJCWNvbnRpbnVlOwogCQl9CiAKLQkJZXJyMiA9IHBma2V5X2Jyb2FkY2FzdF9vbmUo c2tiLCAmc2tiMiwgR0ZQX0FUT01JQywgc2spOworCQllcnIyID0gcGZrZXlfYnJvYWRjYXN0 X29uZShza2IsIEdGUF9BVE9NSUMsIHNrKTsKIAogCQkvKiBFcnJvciBpcyBjbGVhcmVkIGFm dGVyIHN1Y2Nlc3NmdWwgc2VuZGluZyB0byBhdCBsZWFzdCBvbmUKIAkJICogcmVnaXN0ZXJl ZCBLTSAqLwpAQCAtMjc4LDkgKzI2OSw4IEBAIHN0YXRpYyBpbnQgcGZrZXlfYnJvYWRjYXN0 KHN0cnVjdCBza19idWZmICpza2IsIGdmcF90IGFsbG9jYXRpb24sCiAJcmN1X3JlYWRfdW5s b2NrKCk7CiAKIAlpZiAob25lX3NrICE9IE5VTEwpCi0JCWVyciA9IHBma2V5X2Jyb2FkY2Fz dF9vbmUoc2tiLCAmc2tiMiwgYWxsb2NhdGlvbiwgb25lX3NrKTsKKwkJZXJyID0gcGZrZXlf YnJvYWRjYXN0X29uZShza2IsIGFsbG9jYXRpb24sIG9uZV9zayk7CiAKLQlrZnJlZV9za2Io c2tiMik7CiAJa2ZyZWVfc2tiKHNrYik7CiAJcmV0dXJuIGVycjsKIH0K --------------6843093CD031C54901D418C2--