From patchwork Fri Jul 14 19:58:01 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Josh Zimmerman via tpmdd-devel X-Patchwork-Id: 788758 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.sourceforge.net (lists.sourceforge.net [216.34.181.88]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3x8Nmd4GN1z9sRV for ; Sat, 15 Jul 2017 05:58:57 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=lists.sourceforge.net header.i=@lists.sourceforge.net header.b="XRyKjxi6"; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=sourceforge.net header.i=@sourceforge.net header.b="BAd1etWS"; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=sf.net header.i=@sf.net header.b="Jo9JA3ue"; dkim=neutral (0-bit key) header.d=google.com header.i=@google.com header.b="FdIBr9A+"; dkim-atps=neutral DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.sourceforge.net; s=beta; h=Content-Transfer-Encoding:Content-Type:MIME-Version:Reply-To:From:List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id:Subject:References:In-Reply-To:Message-Id:Date:To; bh=8cbpy4fOqLh9c5kkTXGtaG04eFHdvYGnkaqMPfnHU7Y=; b=XRyKjxi6wWwonFn4eRR+piqGlWG3z2CMfbgCDbqNmRJm3v/IOf4Vgj7BIKfjR3OO1dXCRvO1NkuFsqRg8amUjmLQrmRXweIXPWwFdIWX0MdWt50oxaQN3UVUWjJhjeNjP6zuSZVgsAe/11XNghdhGMKQhJnNcNMYTbwoTV5a8HI=; Received: from localhost ([127.0.0.1] helo=sfs-ml-2.v29.ch3.sourceforge.com) by sfs-ml-2.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1dW6jj-0002jv-Li; Fri, 14 Jul 2017 19:58:51 +0000 Received: from sog-mx-2.v43.ch3.sourceforge.com ([172.29.43.192] helo=mx.sourceforge.net) by sfs-ml-2.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1dW6ji-0002jo-OM for tpmdd-devel@lists.sourceforge.net; Fri, 14 Jul 2017 19:58:50 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=References:In-Reply-To:Message-Id:Date:Subject:To:From; bh=wfv/Z4C+8Hlsgb4FCfpBAc2Lhy9e4uW46hN8UALpf/w=; b=BAd1etWSL3A/DIiR3Qto1h0XvxpxaVJ6kh+vxviuC4WGScVOm62OcXk1evUfWHIHIz/KAy4GGBdcp8iJVZPlzLquViieDHQ4kYnBwOL9hfQ7pH1LWdBW/BAufe+cHy8prW3XRsvEpYOyWEj4biH0B+1E/evtTAZJ2tc9jbQVatA=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x; h=References:In-Reply-To:Message-Id:Date:Subject:To:From; bh=wfv/Z4C+8Hlsgb4FCfpBAc2Lhy9e4uW46hN8UALpf/w=; b=Jo9JA3uePwdYQLf2bZzcXLOxYxlJ0gCukTp5xV5wBVgKuobz39jxf83Lk6w22Boghhc1sxolRkbnEaHwqLSOnq0guhyesqmwZtA449AfqOtincuYRLJSjYaTD9qP3xSMDed/F0Id1wON7SvBOZfgs1Gz1m1Qd4cgoVR7efQJjao=; Received-SPF: pass (sog-mx-2.v43.ch3.sourceforge.com: domain of google.com designates 209.85.192.176 as permitted sender) client-ip=209.85.192.176; envelope-from=joshz@google.com; helo=mail-pf0-f176.google.com; Received: from mail-pf0-f176.google.com ([209.85.192.176]) by sog-mx-2.v43.ch3.sourceforge.com with esmtps (TLSv1:AES128-SHA:128) (Exim 4.76) id 1dW6jg-00070E-IT for tpmdd-devel@lists.sourceforge.net; Fri, 14 Jul 2017 19:58:50 +0000 Received: by mail-pf0-f176.google.com with SMTP id q86so49966216pfl.3 for ; Fri, 14 Jul 2017 12:58:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=from:to:subject:date:message-id:in-reply-to:references; bh=wfv/Z4C+8Hlsgb4FCfpBAc2Lhy9e4uW46hN8UALpf/w=; b=FdIBr9A+WRPmZsRtww6ZqGpjDagfqER94YB/j2Pi0lBq9Owgv3a4REjJnL8O803dw/ fN1PpYUPjWyE9OZ10IXrQtgfPv3anWWp7KiQzQAcA83JWnpf0Tg5mb7LG+OJm0ZboSIO OBPlkkNm7OTPKBtuGnPxhvbiGDxuBXThn+w6XSkdKfzEO72wfXhEO/RnhXRudjudt0g7 LByBQaqMT3hZZS8ImGkmes6J+W9b2yxY2kAp/lq3yCHs/1cWjO+99BryTrbebYPDCQW3 ITZvht4+HBBFXRHqAub0LqmI6hKrXLYMIratkMNIcjsDZBJQoX44OLaw4wEckLrS4LMC dyZw== 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; bh=wfv/Z4C+8Hlsgb4FCfpBAc2Lhy9e4uW46hN8UALpf/w=; b=n8rPNFXePgz/fIUT27Ht4u2K8fiKxFmin3P7qI/hjAgRgtYzez7Srz8kOlqhOKNmyw lKXeLZMhIPIjK5qQW51gtVb+EqOIJkWjzeS5SSB4nCoEHl8GGJcwTftQi9qTM/1EuUyD 20FoBTy61k1IV2GVdjgLhLStXqOTuiJhW4cQ3/G2RE6YGF9Voh69B9zP1tBXX9k0D3dY PwqkBHUGfftNCdkVLq/CtSn1wSLdm4P4fMH1rPGROp7fwggB7GOK8xhHwKwgkyXHAOMQ F+HES8s34j8oEpYKRWfIuVeokMn4fj1prDEGaTwj4jOHW3FebK0c0mT7eBgJ5Ek4yI0s Cj1g== X-Gm-Message-State: AIVw113m44/EUUZ0X0jGRF50NKbbhkdrIUIRq2W//lMIDyts1P2Mjjd7 OS2aKohJvjdIPppf X-Received: by 10.84.178.131 with SMTP id z3mr17815218plb.271.1500062322659; Fri, 14 Jul 2017 12:58:42 -0700 (PDT) Received: from angband.kir.corp.google.com ([100.66.174.26]) by smtp.googlemail.com with ESMTPSA id t26sm22043500pfe.88.2017.07.14.12.58.41 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 14 Jul 2017 12:58:42 -0700 (PDT) To: Jarkko Sakkinen , Jason Gunthorpe , tpmdd-devel@lists.sourceforge.net, gregkh@linuxfoundation.org, stable@vger.kernel.org Date: Fri, 14 Jul 2017 12:58:01 -0700 Message-Id: <20170714195803.7035-3-joshz@google.com> X-Mailer: git-send-email 2.13.2.932.g7449e964c-goog In-Reply-To: <20170714195803.7035-1-joshz@google.com> References: <20170714195803.7035-1-joshz@google.com> X-Spam-Score: -1.3 (-) X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at http://www.dnswl.org/, no trust [209.85.192.176 listed in list.dnswl.org] -1.5 SPF_CHECK_PASS SPF reports sender host as permitted sender for sender-domain -0.0 SPF_PASS SPF: sender matches SPF record -0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature 0.5 RCVD_IN_SORBS_SPAM RBL: SORBS: sender is a spam source [209.85.192.176 listed in dnsbl.sorbs.net] -0.2 AWL AWL: Adjusted score from AWL reputation of From: address X-Headers-End: 1dW6jg-00070E-IT Subject: [tpmdd-devel] [PATCH v1 2/4] tpm: Provide strong locking for device removal X-BeenThere: tpmdd-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: Tpm Device Driver maintainance List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Josh Zimmerman via tpmdd-devel From: Josh Zimmerman via tpmdd-devel Reply-To: Josh Zimmerman MIME-Version: 1.0 Errors-To: tpmdd-devel-bounces@lists.sourceforge.net From: Jason Gunthorpe Add a read/write semaphore around the ops function pointers so ops can be set to null when the driver un-registers. Previously the tpm core expected module locking to be enough to ensure that tpm_unregister could not be called during certain times, however that hasn't been sufficient for a long time. Introduce a read/write semaphore around 'ops' so the core can set it to null when unregistering. This provides a strong fence around the driver callbacks, guaranteeing to the driver that no callbacks are running or will run again. For now the ops_lock is placed very high in the call stack, it could be pushed down and made more granular in future if necessary. Signed-off-by: Jason Gunthorpe Reviewed-by: Stefan Berger Reviewed-by: Jarkko Sakkinen Signed-off-by: Jarkko Sakkinen (cherry picked from commit 4e26195f240d73150e8308ae42874702e3df8d2c) --- drivers/char/tpm/tpm-chip.c | 72 ++++++++++++++++++++++++++++++++++++---- drivers/char/tpm/tpm-dev.c | 11 +++++- drivers/char/tpm/tpm-interface.c | 19 ++++++----- drivers/char/tpm/tpm-sysfs.c | 5 +++ drivers/char/tpm/tpm.h | 14 +++++--- 5 files changed, 100 insertions(+), 21 deletions(-) diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c index f55b4921c723..f3a887e4f692 100644 --- a/drivers/char/tpm/tpm-chip.c +++ b/drivers/char/tpm/tpm-chip.c @@ -36,10 +36,60 @@ static DEFINE_SPINLOCK(driver_lock); struct class *tpm_class; dev_t tpm_devt; -/* - * tpm_chip_find_get - return tpm_chip for a given chip number - * @chip_num the device number for the chip +/** + * tpm_try_get_ops() - Get a ref to the tpm_chip + * @chip: Chip to ref + * + * The caller must already have some kind of locking to ensure that chip is + * valid. This function will lock the chip so that the ops member can be + * accessed safely. The locking prevents tpm_chip_unregister from + * completing, so it should not be held for long periods. + * + * Returns -ERRNO if the chip could not be got. */ +int tpm_try_get_ops(struct tpm_chip *chip) +{ + int rc = -EIO; + + get_device(&chip->dev); + + down_read(&chip->ops_sem); + if (!chip->ops) + goto out_lock; + + if (!try_module_get(chip->dev.parent->driver->owner)) + goto out_lock; + + return 0; +out_lock: + up_read(&chip->ops_sem); + put_device(&chip->dev); + return rc; +} +EXPORT_SYMBOL_GPL(tpm_try_get_ops); + +/** + * tpm_put_ops() - Release a ref to the tpm_chip + * @chip: Chip to put + * + * This is the opposite pair to tpm_try_get_ops(). After this returns chip may + * be kfree'd. + */ +void tpm_put_ops(struct tpm_chip *chip) +{ + module_put(chip->dev.parent->driver->owner); + up_read(&chip->ops_sem); + put_device(&chip->dev); +} +EXPORT_SYMBOL_GPL(tpm_put_ops); + +/** + * tpm_chip_find_get() - return tpm_chip for a given chip number + * @chip_num: id to find + * + * The return'd chip has been tpm_try_get_ops'd and must be released via + * tpm_put_ops + */ struct tpm_chip *tpm_chip_find_get(int chip_num) { struct tpm_chip *pos, *chip = NULL; @@ -49,10 +99,10 @@ struct tpm_chip *tpm_chip_find_get(int chip_num) if (chip_num != TPM_ANY_NUM && chip_num != pos->dev_num) continue; - if (try_module_get(pos->dev.parent->driver->owner)) { + /* rcu prevents chip from being free'd */ + if (!tpm_try_get_ops(pos)) chip = pos; - break; - } + break; } rcu_read_unlock(); return chip; @@ -94,6 +144,7 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev, return ERR_PTR(-ENOMEM); mutex_init(&chip->tpm_mutex); + init_rwsem(&chip->ops_sem); INIT_LIST_HEAD(&chip->list); chip->ops = ops; @@ -171,6 +222,12 @@ static int tpm_add_char_device(struct tpm_chip *chip) static void tpm_del_char_device(struct tpm_chip *chip) { cdev_del(&chip->cdev); + + /* Make the driver uncallable. */ + down_write(&chip->ops_sem); + chip->ops = NULL; + up_write(&chip->ops_sem); + device_del(&chip->dev); } @@ -256,6 +313,9 @@ EXPORT_SYMBOL_GPL(tpm_chip_register); * Takes the chip first away from the list of available TPM chips and then * cleans up all the resources reserved by tpm_chip_register(). * + * Once this function returns the driver call backs in 'op's will not be + * running and will no longer start. + * * NOTE: This function should be only called before deinitializing chip * resources. */ diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c index 6ed0651cbe58..912ad30be585 100644 --- a/drivers/char/tpm/tpm-dev.c +++ b/drivers/char/tpm/tpm-dev.c @@ -136,9 +136,18 @@ static ssize_t tpm_write(struct file *file, const char __user *buf, return -EFAULT; } - /* atomic tpm command send and result receive */ + /* atomic tpm command send and result receive. We only hold the ops + * lock during this period so that the tpm can be unregistered even if + * the char dev is held open. + */ + if (tpm_try_get_ops(priv->chip)) { + mutex_unlock(&priv->buffer_mutex); + return -EPIPE; + } out_size = tpm_transmit(priv->chip, priv->data_buffer, sizeof(priv->data_buffer), 0); + + tpm_put_ops(priv->chip); if (out_size < 0) { mutex_unlock(&priv->buffer_mutex); return out_size; diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c index 4391953a7711..8588f2e4b9af 100644 --- a/drivers/char/tpm/tpm-interface.c +++ b/drivers/char/tpm/tpm-interface.c @@ -687,7 +687,7 @@ int tpm_is_tpm2(u32 chip_num) rc = (chip->flags & TPM_CHIP_FLAG_TPM2) != 0; - tpm_chip_put(chip); + tpm_put_ops(chip); return rc; } @@ -716,7 +716,7 @@ int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf) rc = tpm2_pcr_read(chip, pcr_idx, res_buf); else rc = tpm_pcr_read_dev(chip, pcr_idx, res_buf); - tpm_chip_put(chip); + tpm_put_ops(chip); return rc; } EXPORT_SYMBOL_GPL(tpm_pcr_read); @@ -751,7 +751,7 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash) if (chip->flags & TPM_CHIP_FLAG_TPM2) { rc = tpm2_pcr_extend(chip, pcr_idx, hash); - tpm_chip_put(chip); + tpm_put_ops(chip); return rc; } @@ -761,7 +761,7 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash) rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE, 0, "attempting extend a PCR value"); - tpm_chip_put(chip); + tpm_put_ops(chip); return rc; } EXPORT_SYMBOL_GPL(tpm_pcr_extend); @@ -842,7 +842,7 @@ int tpm_send(u32 chip_num, void *cmd, size_t buflen) rc = tpm_transmit_cmd(chip, cmd, buflen, 0, "attempting tpm_cmd"); - tpm_chip_put(chip); + tpm_put_ops(chip); return rc; } EXPORT_SYMBOL_GPL(tpm_send); @@ -1025,7 +1025,7 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max) if (chip->flags & TPM_CHIP_FLAG_TPM2) { err = tpm2_get_random(chip, out, max); - tpm_chip_put(chip); + tpm_put_ops(chip); return err; } @@ -1047,7 +1047,7 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max) num_bytes -= recd; } while (retries-- && total < max); - tpm_chip_put(chip); + tpm_put_ops(chip); return total ? total : -EIO; } EXPORT_SYMBOL_GPL(tpm_get_random); @@ -1073,7 +1073,7 @@ int tpm_seal_trusted(u32 chip_num, struct trusted_key_payload *payload, rc = tpm2_seal_trusted(chip, payload, options); - tpm_chip_put(chip); + tpm_put_ops(chip); return rc; } EXPORT_SYMBOL_GPL(tpm_seal_trusted); @@ -1099,7 +1099,8 @@ int tpm_unseal_trusted(u32 chip_num, struct trusted_key_payload *payload, rc = tpm2_unseal_trusted(chip, payload, options); - tpm_chip_put(chip); + tpm_put_ops(chip); + return rc; } EXPORT_SYMBOL_GPL(tpm_unseal_trusted); diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c index 10370c22e98b..8af4145d10c7 100644 --- a/drivers/char/tpm/tpm-sysfs.c +++ b/drivers/char/tpm/tpm-sysfs.c @@ -295,5 +295,10 @@ int tpm_sysfs_add_device(struct tpm_chip *chip) void tpm_sysfs_del_device(struct tpm_chip *chip) { + /* The sysfs routines rely on an implicit tpm_try_get_ops, this + * function is called before ops is null'd and the sysfs core + * synchronizes this removal so that no callbacks are running or can + * run again + */ sysfs_remove_group(&chip->dev.parent->kobj, &tpm_dev_group); } diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h index 57c4c26c38ea..e21e2c599e66 100644 --- a/drivers/char/tpm/tpm.h +++ b/drivers/char/tpm/tpm.h @@ -174,7 +174,13 @@ struct tpm_chip { struct device dev; struct cdev cdev; + /* A driver callback under ops cannot be run unless ops_sem is held + * (sometimes implicitly, eg for the sysfs code). ops becomes null + * when the driver is unregistered, see tpm_try_get_ops. + */ + struct rw_semaphore ops_sem; const struct tpm_class_ops *ops; + unsigned int flags; int dev_num; /* /dev/tpm# */ @@ -200,11 +206,6 @@ struct tpm_chip { #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev) -static inline void tpm_chip_put(struct tpm_chip *chip) -{ - module_put(chip->dev.parent->driver->owner); -} - static inline int tpm_read_index(int base, int index) { outb(index, base); @@ -516,6 +517,9 @@ extern int wait_for_tpm_stat(struct tpm_chip *, u8, unsigned long, wait_queue_head_t *, bool); struct tpm_chip *tpm_chip_find_get(int chip_num); +__must_check int tpm_try_get_ops(struct tpm_chip *chip); +void tpm_put_ops(struct tpm_chip *chip); + extern struct tpm_chip *tpmm_chip_alloc(struct device *dev, const struct tpm_class_ops *ops); extern int tpm_chip_register(struct tpm_chip *chip);