From patchwork Fri Nov 17 10:07:24 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Javier Martinez Canillas X-Patchwork-Id: 10062525 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 585FE6023A for ; Fri, 17 Nov 2017 10:07:45 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4BF7E2A785 for ; Fri, 17 Nov 2017 10:07:45 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 406E62A7F9; Fri, 17 Nov 2017 10:07:45 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.4 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A95862A785 for ; Fri, 17 Nov 2017 10:07:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752428AbdKQKHo (ORCPT ); Fri, 17 Nov 2017 05:07:44 -0500 Received: from mail-wm0-f66.google.com ([74.125.82.66]:43040 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752696AbdKQKHd (ORCPT ); Fri, 17 Nov 2017 05:07:33 -0500 Received: by mail-wm0-f66.google.com with SMTP id x63so5322017wmf.2 for ; Fri, 17 Nov 2017 02:07:32 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=o5svfkdR3Ul8PI3vVmvmCJarkl5z7uGM4FGiiP+EQ0s=; b=aNZjJ5yl332Mw/W+pG+zDyLjZJoD7qu9wrtqSLdeA3ri3gfaYe00LMvuou/z2EwqF8 08aCTTfaA/6amGIjW6QNffuaqXKidTkwsMvRANpNUGvMqP4mMOGGlzsQASYL6g2zYUsM BTtrJQfNb4lRdA15DVu2+YrO/NJ0zWYJOJeP9KQFd1jOxPGBydJ0CybDMy+Fw4VlStKi B35dSue90hEU07JRw00loM2pGobIG82e70kqKLAsBk7Zi8IGhU42ayKOchk2m975KWwX YH9MSM4S/pbP3irZaNRuotpxzE/cKWC5DEQg/JSayCTktdnVEJyE8CZn/0g1GAYlqkHB 3q8Q== X-Gm-Message-State: AJaThX6ww/is/n9XpvRO2QVfcJLfHKmN1JNeJnTN2qFHW/O/P2Vhy5f2 rZOMPN2Obfnxk4MVWzp40Wp/9w== X-Google-Smtp-Source: AGs4zMaRMq6QKNpD1lfcunymux4BFFCC/XNONKKXFHr/czMMsr5iweYpCWn0tDv3duAU6F8hwcinxA== X-Received: by 10.28.168.88 with SMTP id r85mr4162179wme.63.1510913251308; Fri, 17 Nov 2017 02:07:31 -0800 (PST) Received: from minerva.redhat.com ([90.77.100.34]) by smtp.gmail.com with ESMTPSA id m68sm3996780wma.0.2017.11.17.02.07.29 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 17 Nov 2017 02:07:30 -0800 (PST) From: Javier Martinez Canillas To: linux-kernel@vger.kernel.org Cc: Javier Martinez Canillas , Jarkko Sakkinen , Peter Huewe , Philip Tricca , Jason Gunthorpe , linux-integrity@vger.kernel.org, William Roberts Subject: [RFC PATCH] tpm: don't return -EINVAL if TPM command validation fails Date: Fri, 17 Nov 2017 11:07:24 +0100 Message-Id: <20171117100724.19257-1-javierm@redhat.com> X-Mailer: git-send-email 2.14.3 Sender: linux-integrity-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP According to the TPM Library Specification, a TPM device must do a command header validation before processing and return a TPM_RC_COMMAND_CODE code if the command is not implemented and the TPM_RC_COMMAND_SIZE code if the command buffer size is not correct. So user-space will expect to handle these response codes as errors, but if the in-kernel resource manager is used (/dev/tpmrm?) then an -EINVAL errno code is returned instead if the command isn't implemented or the buffer size isn't correct. This confuses user-space since doesn't expect that. This is also not consistent with the behavior when not using TPM spaces and accessing the TPM directly (/dev/tpm?), in this case the command is is sent to the TPM anyways and user-space can get an error from the TPM. Instead of returning an -EINVAL errno code when the tpm_validate_command() function fails, allow the command to be sent to the TPM but just don't do any TPM space management. That way the TPM can report back a proper error and the behavior be consistent when using either /dev/tpm? or /dev/tpmrm?. Signed-off-by: Javier Martinez Canillas --- Hello, This patch is an RFC because I'm not sure if this is the correct way to fix this issue. I'm not that familiar with the TPM driver so may had missed some details. And example of user-space getting confused by the TPM chardev returning -EINVAL when sending a not supported TPM command can be seen in this tpm2-tools issue: https://github.com/intel/tpm2-tools/issues/621 Best regards, Javier drivers/char/tpm/tpm-interface.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c index 1d6729be4cd6..86527e9a27cc 100644 --- a/drivers/char/tpm/tpm-interface.c +++ b/drivers/char/tpm/tpm-interface.c @@ -390,9 +390,14 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space, u32 count, ordinal; unsigned long stop; bool need_locality; + bool cmd_validated; - if (!tpm_validate_command(chip, space, buf, bufsiz)) - return -EINVAL; + /* + * If command validation fails, sent it to the TPM anyways so it can + * report a proper error to user-space. Just don't do any TPM space + * management in this case. + */ + cmd_validated = tpm_validate_command(chip, space, buf, bufsiz); if (bufsiz > TPM_BUFSIZE) bufsiz = TPM_BUFSIZE; @@ -424,9 +429,11 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space, chip->locality = rc; } - rc = tpm2_prepare_space(chip, space, ordinal, buf); - if (rc) - goto out; + if (cmd_validated) { + rc = tpm2_prepare_space(chip, space, ordinal, buf); + if (rc) + goto out; + } rc = chip->ops->send(chip, (u8 *) buf, count); if (rc < 0) { @@ -481,7 +488,8 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space, goto out; } - rc = tpm2_commit_space(chip, space, ordinal, buf, &len); + if (cmd_validated) + rc = tpm2_commit_space(chip, space, ordinal, buf, &len); out: if (need_locality && chip->ops->relinquish_locality) {