From patchwork Tue Aug 25 18:12:26 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marcelo Henrique Cerri X-Patchwork-Id: 1351213 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=canonical.com Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4BbcY506G2z9sTN; Wed, 26 Aug 2020 04:12:57 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1kAdRG-0002A2-4U; Tue, 25 Aug 2020 18:12:54 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1kAdRE-00029O-Ac for kernel-team@lists.ubuntu.com; Tue, 25 Aug 2020 18:12:52 +0000 Received: from mail-qk1-f197.google.com ([209.85.222.197]) by youngberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1kAdRD-00070u-VY for kernel-team@lists.ubuntu.com; Tue, 25 Aug 2020 18:12:52 +0000 Received: by mail-qk1-f197.google.com with SMTP id b207so5253628qkc.12 for ; Tue, 25 Aug 2020 11:12:51 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=OOqQbjdY+HF9d1qwZSXGzHy8WZr9A2IogZCTVB/MzPc=; b=NlAW5t7zY+yXZ4rf/SZOlRwa9PlrVP5w5lIER37RennpipC76oTrHpA2+1ztFrco1D K78bUITQN2JEWwztJQf8TXz3SRpdNBh00je36GUnoZugMxUtm1VOf+ngDdxfcvCkucEK DOjndwXy481Ae10Qb2zZL9F/FlCGZMZGLTxNQUrWLfPMA1HsccSMwOQJAEqYZn8bCXj0 1epgqAhhVUPNdlr2E1JuNFwnvsTs7flkQvsiezXQJ4pfbnay3qV+V02rBEygtQFF1I67 KZjuM32/SAZX4lhw276oMkwzKbUMU4ojuQMlxjhtpIVsvA9NjfNsP7hv/enkxgreL9x3 Fp2g== X-Gm-Message-State: AOAM532awFRbrZ4q4st9Q4LZvSqjJ9aZHJbCd3+nVSVsuEq9ILpmVzs8 ZyazmSTfvp4Cf1C4g9CKbnbnGf/n2dhdr0uA0vYv6grcAm4Cl5vQlS5cMhJLmRD682FjEfblEgz Ki2Kbe8/GqnJbklWdF8vMGpgIBkcrUe6Gu4UOdkjM X-Received: by 2002:a37:74d:: with SMTP id 74mr10549795qkh.147.1598379170577; Tue, 25 Aug 2020 11:12:50 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxfMRqL7vrzJb2BsP31nE5yOIh6uq/LW5RHv1f+OcZKqw9UMURLrUCMJ5iDE4dE4n6M9+CYnA== X-Received: by 2002:a37:74d:: with SMTP id 74mr10549779qkh.147.1598379170256; Tue, 25 Aug 2020 11:12:50 -0700 (PDT) Received: from localhost.localdomain ([201.82.186.200]) by smtp.gmail.com with ESMTPSA id r48sm881332qtb.26.2020.08.25.11.12.48 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 25 Aug 2020 11:12:49 -0700 (PDT) From: Marcelo Henrique Cerri To: kernel-team@lists.ubuntu.com Subject: [bionic:linux-azure-4.15][PATCH 1/3] RDMA/rxe: Close a race after ib_register_device Date: Tue, 25 Aug 2020 15:12:26 -0300 Message-Id: <20200825181228.3274075-2-marcelo.cerri@canonical.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200825181228.3274075-1-marcelo.cerri@canonical.com> References: <20200825180923.3254380-1-marcelo.cerri@canonical.com> <20200825181228.3274075-1-marcelo.cerri@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Jason Gunthorpe BugLink: https://bugs.launchpad.net/bugs/1874503 Since rxe allows unregistration from other threads the rxe pointer can become invalid any moment after ib_register_driver returns. This could cause a user triggered use after free. Add another driver callback to be called right after the device becomes registered to complete any device setup required post-registration. This callback has enough core locking to prevent the device from becoming unregistered. Signed-off-by: Jason Gunthorpe (backported from commit ca22354b140853b8155692d5b2bc0110aa54e937) Signed-off-by: Sultan Alsawaf Signed-off-by: Marcelo Henrique Cerri --- drivers/infiniband/core/device.c | 8 ++++++++ drivers/infiniband/sw/rxe/rxe_net.c | 8 ++++---- drivers/infiniband/sw/rxe/rxe_net.h | 2 +- drivers/infiniband/sw/rxe/rxe_sysfs.c | 8 ++------ drivers/infiniband/sw/rxe/rxe_verbs.c | 14 ++++++++++++++ include/rdma/ib_verbs.h | 5 +++++ 6 files changed, 34 insertions(+), 11 deletions(-) diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c index 6b0d1d8609ca..664e497d2daf 100644 --- a/drivers/infiniband/core/device.c +++ b/drivers/infiniband/core/device.c @@ -547,6 +547,12 @@ int ib_register_device(struct ib_device *device, device->reg_state = IB_DEV_REGISTERED; + if (device->enable_driver) { + ret = device->enable_driver(device); + if (ret) + goto sysfs_cleanup; + } + list_for_each_entry(client, &client_list, list) if (!add_client_context(device, client) && client->add) client->add(device); @@ -558,6 +564,8 @@ int ib_register_device(struct ib_device *device, mutex_unlock(&device_mutex); return 0; +sysfs_cleanup: + ib_device_unregister_sysfs(device); cg_cleanup: ib_device_unregister_rdmacg(device); cache_cleanup: diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c index 24a68a9da8be..d568dc12a00b 100644 --- a/drivers/infiniband/sw/rxe/rxe_net.c +++ b/drivers/infiniband/sw/rxe/rxe_net.c @@ -551,27 +551,27 @@ enum rdma_link_layer rxe_link_layer(struct rxe_dev *rxe, unsigned int port_num) return IB_LINK_LAYER_ETHERNET; } -struct rxe_dev *rxe_net_add(struct net_device *ndev) +int rxe_net_add(struct net_device *ndev) { int err; struct rxe_dev *rxe = NULL; rxe = (struct rxe_dev *)ib_alloc_device(sizeof(*rxe)); if (!rxe) - return NULL; + return -ENOMEM; rxe->ndev = ndev; err = rxe_add(rxe, ndev->mtu); if (err) { ib_dealloc_device(&rxe->ib_dev); - return NULL; + return err; } spin_lock_bh(&dev_list_lock); list_add_tail(&rxe->list, &rxe_dev_list); spin_unlock_bh(&dev_list_lock); - return rxe; + return 0; } void rxe_remove_all(void) diff --git a/drivers/infiniband/sw/rxe/rxe_net.h b/drivers/infiniband/sw/rxe/rxe_net.h index 1c06b3bfe1b6..0a889f21f64d 100644 --- a/drivers/infiniband/sw/rxe/rxe_net.h +++ b/drivers/infiniband/sw/rxe/rxe_net.h @@ -47,7 +47,7 @@ extern struct rxe_recv_sockets recv_sockets; extern struct notifier_block rxe_net_notifier; void rxe_release_udp_tunnel(struct socket *sk); -struct rxe_dev *rxe_net_add(struct net_device *ndev); +int rxe_net_add(struct net_device *ndev); int rxe_net_init(void); void rxe_net_exit(void); diff --git a/drivers/infiniband/sw/rxe/rxe_sysfs.c b/drivers/infiniband/sw/rxe/rxe_sysfs.c index d5ed7571128f..08c9e9539977 100644 --- a/drivers/infiniband/sw/rxe/rxe_sysfs.c +++ b/drivers/infiniband/sw/rxe/rxe_sysfs.c @@ -75,7 +75,6 @@ static int rxe_param_set_add(const char *val, const struct kernel_param *kp) int err = 0; char intf[32]; struct net_device *ndev = NULL; - struct rxe_dev *rxe; len = sanitize_arg(val, intf, sizeof(intf)); if (!len) { @@ -97,15 +96,12 @@ static int rxe_param_set_add(const char *val, const struct kernel_param *kp) goto err; } - rxe = rxe_net_add(ndev); - if (!rxe) { + err = rxe_net_add(ndev); + if (err) { pr_err("failed to add %s\n", intf); - err = -EINVAL; goto err; } - rxe_set_port_state(ndev); - pr_info("added %s to %s\n", rxe->ib_dev.name, intf); err: if (ndev) dev_put(ndev); diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c index f9cec72273cd..72f65a5a4ee2 100644 --- a/drivers/infiniband/sw/rxe/rxe_verbs.c +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c @@ -1188,6 +1188,15 @@ static struct device_attribute *rxe_dev_attributes[] = { &dev_attr_parent, }; +static int rxe_enable_driver(struct ib_device *ib_dev) +{ + struct rxe_dev *rxe = container_of(ib_dev, struct rxe_dev, ib_dev); + + rxe_set_port_state(rxe); + dev_info(&rxe->ib_dev.dev, "added %s\n", netdev_name(rxe->ndev)); + return 0; +} + int rxe_register_device(struct rxe_dev *rxe) { int err; @@ -1290,6 +1299,7 @@ int rxe_register_device(struct rxe_dev *rxe) dev->detach_mcast = rxe_detach_mcast; dev->get_hw_stats = rxe_ib_get_hw_stats; dev->alloc_hw_stats = rxe_ib_alloc_hw_stats; + dev->enable_driver = rxe_enable_driver; tfm = crypto_alloc_shash("crc32", 0, 0); if (IS_ERR(tfm)) { @@ -1321,6 +1331,10 @@ int rxe_register_device(struct rxe_dev *rxe) err1: crypto_free_shash(rxe->tfm); + /* + * Note that rxe may be invalid at this point if another thread + * unregistered it. + */ return err; } diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h index ad7547c7ffc2..a9a4e15fd0ad 100644 --- a/include/rdma/ib_verbs.h +++ b/include/rdma/ib_verbs.h @@ -2317,6 +2317,11 @@ struct ib_device { struct ib_rwq_ind_table_init_attr *init_attr, struct ib_udata *udata); int (*destroy_rwq_ind_table)(struct ib_rwq_ind_table *wq_ind_table); + /* + * Called after the device becomes registered, before clients are + * attached + */ + int (*enable_driver)(struct ib_device *device); /** * rdma netdev operation *