Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/1.2/patches/814038/?format=api
{ "id": 814038, "url": "http://patchwork.ozlabs.org/api/1.2/patches/814038/?format=api", "web_url": "http://patchwork.ozlabs.org/project/netdev/patch/9de7bef8f4f8c0f369361118f6d2daee5d188467.1505444541.git.lucien.xin@gmail.com/", "project": { "id": 7, "url": "http://patchwork.ozlabs.org/api/1.2/projects/7/?format=api", "name": "Linux network development", "link_name": "netdev", "list_id": "netdev.vger.kernel.org", "list_email": "netdev@vger.kernel.org", "web_url": null, "scm_url": null, "webscm_url": null, "list_archive_url": "", "list_archive_url_format": "", "commit_url_format": "" }, "msgid": "<9de7bef8f4f8c0f369361118f6d2daee5d188467.1505444541.git.lucien.xin@gmail.com>", "list_archive_url": null, "date": "2017-09-15T03:02:21", "name": "[net] sctp: fix an use-after-free issue in sctp_sock_dump", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": true, "hash": "263f295002fb1a6d08c45847a1f93e25f89d3d73", "submitter": { "id": 61073, "url": "http://patchwork.ozlabs.org/api/1.2/people/61073/?format=api", "name": "Xin Long", "email": "lucien.xin@gmail.com" }, "delegate": { "id": 34, "url": "http://patchwork.ozlabs.org/api/1.2/users/34/?format=api", "username": "davem", "first_name": "David", "last_name": "Miller", "email": "davem@davemloft.net" }, "mbox": "http://patchwork.ozlabs.org/project/netdev/patch/9de7bef8f4f8c0f369361118f6d2daee5d188467.1505444541.git.lucien.xin@gmail.com/mbox/", "series": [ { "id": 3200, "url": "http://patchwork.ozlabs.org/api/1.2/series/3200/?format=api", "web_url": "http://patchwork.ozlabs.org/project/netdev/list/?series=3200", "date": "2017-09-15T03:02:21", "name": "[net] sctp: fix an use-after-free issue in sctp_sock_dump", "version": 1, "mbox": "http://patchwork.ozlabs.org/series/3200/mbox/" } ], "comments": "http://patchwork.ozlabs.org/api/patches/814038/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/814038/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<netdev-owner@vger.kernel.org>", "X-Original-To": "patchwork-incoming@ozlabs.org", "Delivered-To": "patchwork-incoming@ozlabs.org", "Authentication-Results": [ "ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)", "ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"va8mFwdX\"; dkim-atps=neutral" ], "Received": [ "from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xtgF03JBmz9sRm\n\tfor <patchwork-incoming@ozlabs.org>;\n\tFri, 15 Sep 2017 13:02:44 +1000 (AEST)", "(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751926AbdIODCk (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tThu, 14 Sep 2017 23:02:40 -0400", "from mail-pg0-f65.google.com ([74.125.83.65]:36180 \"EHLO\n\tmail-pg0-f65.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751810AbdIODCa (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Thu, 14 Sep 2017 23:02:30 -0400", "by mail-pg0-f65.google.com with SMTP id d8so617700pgt.3;\n\tThu, 14 Sep 2017 20:02:30 -0700 (PDT)", "from localhost ([209.132.188.80]) by smtp.gmail.com with ESMTPSA id\n\tr12sm32017082pfd.187.2017.09.14.20.02.28\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tThu, 14 Sep 2017 20:02:29 -0700 (PDT)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=from:to:cc:subject:date:message-id;\n\tbh=m6wazx+oxurSkWtFVeppsaDAOz3U2o6fP0Lxh7aW3Ls=;\n\tb=va8mFwdX3X4XKvbwRajGkjAB11lrfV/3+snHHoVA7sVr3Cy4eBK99wbdLB+fWGXQJT\n\tyWNRAqBdnGkf0vU/mPl1LdzR+mXw9DCpIGcOS4GQWV6ZZ77cCdDnoJ5eeBnGVzLRDrLQ\n\tC7cHjKVxxYjqCfQAArHDWdzAeBfyKN0D3szPuEpyT26v9ZwKOnMwt06+hSMJE4m+el7V\n\tvuB3QWC9CGpi9BzXKFV6J1iBxAkhE3JOW3ZQ5/nv51rx2R1Or2xFTr44rORb5OSs3cci\n\tgp7hVMIPOoxNa79XSD/P8AHRDR0KmTH+9TV/pSn1M6KCnKyYks/OU/v++QUNYKtmGKKo\n\tRPOA==", "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id;\n\tbh=m6wazx+oxurSkWtFVeppsaDAOz3U2o6fP0Lxh7aW3Ls=;\n\tb=P6kPigMPWAR18MvAb32QvngNIRFXfpUOYX4eo25U/6uA8bmc9Kjvp9qrnnDuQDCXp8\n\tK32rg8nLJaQruYxsYW9g3LHgFPYY27YTuD2i+KEaqwmDFORYi7FqJxsTxBuxBdv1PJXN\n\tiJfldPnvq5lg0Ng4GSiQoj7+7HwBTdofkQHj+j7Yhe08ODBeOvVO0HXY81s1Hx/n8dxs\n\tPsGBLwQl+oBk1KsMKGUbUShZ0J1Df5QM2VniD22ZfvB09eGkG2anoQvJb36ytlwMRBvw\n\tJYjLpEyYsAuZ4Csvi/YMcUpWGdMxiYcSPjB9bfOAZA7Ks3vORYAdp7VZZQjkj6ZmBbin\n\twXNQ==", "X-Gm-Message-State": "AHPjjUgZoRcMBgCbvsKVijgov+YOZsocNLHLjKoF4LkLy1q+kEp0GvaY\n\terdlLsxOwuf2iZWM/DU=", "X-Google-Smtp-Source": "ADKCNb5eeUnfXSkb513NRQ0s5bhn4SAXa6HMWCZkFOf6lplplLyoUtbzdMrsJ2Qs82WTgxlpG7uVMA==", "X-Received": "by 10.101.89.65 with SMTP id g1mr22823930pgu.368.1505444549585; \n\tThu, 14 Sep 2017 20:02:29 -0700 (PDT)", "From": "Xin Long <lucien.xin@gmail.com>", "To": "network dev <netdev@vger.kernel.org>, linux-sctp@vger.kernel.org", "Cc": "davem@davemloft.net, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>,\n\tNeil Horman <nhorman@tuxdriver.com>, pabeni@redhat.com", "Subject": "[PATCH net] sctp: fix an use-after-free issue in sctp_sock_dump", "Date": "Fri, 15 Sep 2017 11:02:21 +0800", "Message-Id": "<9de7bef8f4f8c0f369361118f6d2daee5d188467.1505444541.git.lucien.xin@gmail.com>", "X-Mailer": "git-send-email 2.1.0", "Sender": "netdev-owner@vger.kernel.org", "Precedence": "bulk", "List-ID": "<netdev.vger.kernel.org>", "X-Mailing-List": "netdev@vger.kernel.org" }, "content": "Commit 86fdb3448cc1 (\"sctp: ensure ep is not destroyed before doing the\ndump\") tried to fix an use-after-free issue by checking !sctp_sk(sk)->ep\nwith holding sock and sock lock.\n\nBut Paolo noticed that endpoint could be destroyed in sctp_rcv without\nsock lock protection. It means the use-after-free issue still could be\ntriggered when sctp_rcv put and destroy ep after sctp_sock_dump checks\n!ep, although it's pretty hard to reproduce.\n\nI could reproduce it by mdelay in sctp_rcv while msleep in sctp_close\nand sctp_sock_dump long time.\n\nThis patch is to add another param cb_done to sctp_for_each_transport\nand dump ep->assocs with holding tsp after jumping out of transport's\ntraversal in it to avoid this issue.\n\nIt can also improve sctp diag dump to make it run faster, as no need\nto save sk into cb->args[5] and keep calling sctp_for_each_transport\nany more.\n\nThis patch is also to use int * instead of int for the pos argument\nin sctp_for_each_transport, which could make postion increment only\nin sctp_for_each_transport and no need to keep changing cb->args[2]\nin sctp_sock_filter and sctp_sock_dump any more.\n\nFixes: 86fdb3448cc1 (\"sctp: ensure ep is not destroyed before doing the dump\")\nReported-by: Paolo Abeni <pabeni@redhat.com>\nSigned-off-by: Xin Long <lucien.xin@gmail.com>\n---\n include/net/sctp/sctp.h | 3 ++-\n net/sctp/sctp_diag.c | 32 +++++++++-----------------------\n net/sctp/socket.c | 40 +++++++++++++++++++++++++---------------\n 3 files changed, 36 insertions(+), 39 deletions(-)", "diff": "diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h\nindex 06b4f51..d7d8cba 100644\n--- a/include/net/sctp/sctp.h\n+++ b/include/net/sctp/sctp.h\n@@ -127,7 +127,8 @@ int sctp_transport_lookup_process(int (*cb)(struct sctp_transport *, void *),\n \t\t\t\t const union sctp_addr *laddr,\n \t\t\t\t const union sctp_addr *paddr, void *p);\n int sctp_for_each_transport(int (*cb)(struct sctp_transport *, void *),\n-\t\t\t struct net *net, int pos, void *p);\n+\t\t\t int (*cb_done)(struct sctp_transport *, void *),\n+\t\t\t struct net *net, int *pos, void *p);\n int sctp_for_each_endpoint(int (*cb)(struct sctp_endpoint *, void *), void *p);\n int sctp_get_sctp_info(struct sock *sk, struct sctp_association *asoc,\n \t\t struct sctp_info *info);\ndiff --git a/net/sctp/sctp_diag.c b/net/sctp/sctp_diag.c\nindex e99518e..7008a99 100644\n--- a/net/sctp/sctp_diag.c\n+++ b/net/sctp/sctp_diag.c\n@@ -279,9 +279,11 @@ static int sctp_tsp_dump_one(struct sctp_transport *tsp, void *p)\n \treturn err;\n }\n \n-static int sctp_sock_dump(struct sock *sk, void *p)\n+static int sctp_sock_dump(struct sctp_transport *tsp, void *p)\n {\n+\tstruct sctp_endpoint *ep = tsp->asoc->ep;\n \tstruct sctp_comm_param *commp = p;\n+\tstruct sock *sk = ep->base.sk;\n \tstruct sk_buff *skb = commp->skb;\n \tstruct netlink_callback *cb = commp->cb;\n \tconst struct inet_diag_req_v2 *r = commp->r;\n@@ -289,9 +291,7 @@ static int sctp_sock_dump(struct sock *sk, void *p)\n \tint err = 0;\n \n \tlock_sock(sk);\n-\tif (!sctp_sk(sk)->ep)\n-\t\tgoto release;\n-\tlist_for_each_entry(assoc, &sctp_sk(sk)->ep->asocs, asocs) {\n+\tlist_for_each_entry(assoc, &ep->asocs, asocs) {\n \t\tif (cb->args[4] < cb->args[1])\n \t\t\tgoto next;\n \n@@ -327,40 +327,30 @@ static int sctp_sock_dump(struct sock *sk, void *p)\n \t\tcb->args[4]++;\n \t}\n \tcb->args[1] = 0;\n-\tcb->args[2]++;\n \tcb->args[3] = 0;\n \tcb->args[4] = 0;\n release:\n \trelease_sock(sk);\n-\tsock_put(sk);\n \treturn err;\n }\n \n-static int sctp_get_sock(struct sctp_transport *tsp, void *p)\n+static int sctp_sock_filter(struct sctp_transport *tsp, void *p)\n {\n \tstruct sctp_endpoint *ep = tsp->asoc->ep;\n \tstruct sctp_comm_param *commp = p;\n \tstruct sock *sk = ep->base.sk;\n-\tstruct netlink_callback *cb = commp->cb;\n \tconst struct inet_diag_req_v2 *r = commp->r;\n \tstruct sctp_association *assoc =\n \t\tlist_entry(ep->asocs.next, struct sctp_association, asocs);\n \n \t/* find the ep only once through the transports by this condition */\n \tif (tsp->asoc != assoc)\n-\t\tgoto out;\n+\t\treturn 0;\n \n \tif (r->sdiag_family != AF_UNSPEC && sk->sk_family != r->sdiag_family)\n-\t\tgoto out;\n-\n-\tsock_hold(sk);\n-\tcb->args[5] = (long)sk;\n+\t\treturn 0;\n \n \treturn 1;\n-\n-out:\n-\tcb->args[2]++;\n-\treturn 0;\n }\n \n static int sctp_ep_dump(struct sctp_endpoint *ep, void *p)\n@@ -503,12 +493,8 @@ static void sctp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,\n \tif (!(idiag_states & ~(TCPF_LISTEN | TCPF_CLOSE)))\n \t\tgoto done;\n \n-next:\n-\tcb->args[5] = 0;\n-\tsctp_for_each_transport(sctp_get_sock, net, cb->args[2], &commp);\n-\n-\tif (cb->args[5] && !sctp_sock_dump((struct sock *)cb->args[5], &commp))\n-\t\tgoto next;\n+\tsctp_for_each_transport(sctp_sock_filter, sctp_sock_dump,\n+\t\t\t\tnet, (int *)&cb->args[2], &commp);\n \n done:\n \tcb->args[1] = cb->args[4];\ndiff --git a/net/sctp/socket.c b/net/sctp/socket.c\nindex 1b00a1e..d4730ad 100644\n--- a/net/sctp/socket.c\n+++ b/net/sctp/socket.c\n@@ -4658,29 +4658,39 @@ int sctp_transport_lookup_process(int (*cb)(struct sctp_transport *, void *),\n EXPORT_SYMBOL_GPL(sctp_transport_lookup_process);\n \n int sctp_for_each_transport(int (*cb)(struct sctp_transport *, void *),\n-\t\t\t struct net *net, int pos, void *p) {\n+\t\t\t int (*cb_done)(struct sctp_transport *, void *),\n+\t\t\t struct net *net, int *pos, void *p) {\n \tstruct rhashtable_iter hti;\n-\tvoid *obj;\n-\tint err;\n-\n-\terr = sctp_transport_walk_start(&hti);\n-\tif (err)\n-\t\treturn err;\n+\tstruct sctp_transport *tsp;\n+\tint ret;\n \n-\tobj = sctp_transport_get_idx(net, &hti, pos + 1);\n-\tfor (; !IS_ERR_OR_NULL(obj); obj = sctp_transport_get_next(net, &hti)) {\n-\t\tstruct sctp_transport *transport = obj;\n+again:\n+\tret = sctp_transport_walk_start(&hti);\n+\tif (ret)\n+\t\treturn ret;\n \n-\t\tif (!sctp_transport_hold(transport))\n+\ttsp = sctp_transport_get_idx(net, &hti, *pos + 1);\n+\tfor (; !IS_ERR_OR_NULL(tsp); tsp = sctp_transport_get_next(net, &hti)) {\n+\t\tif (!sctp_transport_hold(tsp))\n \t\t\tcontinue;\n-\t\terr = cb(transport, p);\n-\t\tsctp_transport_put(transport);\n-\t\tif (err)\n+\t\tret = cb(tsp, p);\n+\t\tif (ret)\n \t\t\tbreak;\n+\t\t(*pos)++;\n+\t\tsctp_transport_put(tsp);\n \t}\n \tsctp_transport_walk_stop(&hti);\n \n-\treturn err;\n+\tif (ret) {\n+\t\tif (cb_done && !cb_done(tsp, p)) {\n+\t\t\t(*pos)++;\n+\t\t\tsctp_transport_put(tsp);\n+\t\t\tgoto again;\n+\t\t}\n+\t\tsctp_transport_put(tsp);\n+\t}\n+\n+\treturn ret;\n }\n EXPORT_SYMBOL_GPL(sctp_for_each_transport);\n \n", "prefixes": [ "net" ] }