Received: by 2002:ac0:8c9a:0:0:0:0:0 with SMTP id r26csp183894ima; Thu, 31 Jan 2019 14:38:52 -0800 (PST) X-Google-Smtp-Source: ALg8bN45zAFZFmw2+gRvdlVDNtlkLHiAeVKI0pkz8BvgtpoV+2XEroXDqE8YgBOCI1ST3Zy4vds7 X-Received: by 2002:a62:6408:: with SMTP id y8mr36307222pfb.202.1548974332327; Thu, 31 Jan 2019 14:38:52 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1548974332; cv=none; d=google.com; s=arc-20160816; b=0snXKPod70Ufo/KIK/GFdAkfS+RccKl/N/3R8C6Ko+AxhG9uvIpTsv1a1J4d9zqKuS /LdQoeojJQ7Uz1oYmQQt9GQSiYG+M2EkQc+fZaP1k/SA5UwApPPOzwrjHjo4izrcsrjd EvxtIJX6WYCWwXzXmBIX641/+m+x8dC2ROHgOgW+No2UXWTRVaB1+oHPZt1bOHxcxYwA dvOoimCZnhhElYQOTmz1jdFe/gpCS4HFfre1uT1Qr9Z1JKk26nwzZfleUw11WdnL2FL7 PuMSaqU1O9j8zVGBRBnkKt6VT06q1UFu898caylMY+6jRRiBWkdSX9l8Fa2jCpbhCpT0 n1dw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:organization:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date; bh=fBSEkUPq0XuPpWcEGwysLmC3+BLL5ZPjj1t5i5LSYwY=; b=NkoKM/YQIQY3IdtvxEYy/hfp2ta+tVSob2R3/R/Su7lyLjwNKODAHkNjWMXodn9u/A 5OQIcQf95ZgJggB1eZyQWaYdwMVb5KRuPxSF1ESARKMlSnHcbMGMiM1s0gcLekgxPdgU +uhZBDodSOgCq5eNf8oIctwdA/ffW7wZPfomUMf9hyJikRjkJyaIcrf41qkbIebLOAVX YkALdQwubaX+xTmDIJr4Mj56cKqkJGrWxxJbi13BKobLcT5pAxDOBcGvNh1uNj2WwXrz TdGulPFpc52QCScAFpVe+KQ4NZqIkM12eBOvArLBKnMryXwAug378fMc7iSpONKrnJHZ 9/wg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=intel.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id w61si5528645plb.309.2019.01.31.14.38.36; Thu, 31 Jan 2019 14:38:52 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727274AbfAaUpR (ORCPT + 99 others); Thu, 31 Jan 2019 15:45:17 -0500 Received: from mga17.intel.com ([192.55.52.151]:47279 "EHLO mga17.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725883AbfAaUpQ (ORCPT ); Thu, 31 Jan 2019 15:45:16 -0500 X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 31 Jan 2019 12:45:15 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,545,1539673200"; d="scan'208";a="114310298" Received: from rkazants-mobl.ccr.corp.intel.com (HELO localhost) ([10.249.254.212]) by orsmga008.jf.intel.com with ESMTP; 31 Jan 2019 12:45:11 -0800 Date: Thu, 31 Jan 2019 22:45:10 +0200 From: Jarkko Sakkinen To: Linus Torvalds Cc: tomas.winkler@intel.com, Jason Gunthorpe , James Bottomley , linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org, Linux List Kernel Mailing Subject: Re: Getting weird TPM error after rebasing my tree to security/next-general Message-ID: <20190131204510.GA354@linux.intel.com> References: <20190122132910.GA2720@linux.intel.com> <20190123153638.GA8727@linux.intel.com> <20190129132016.GA1602@linux.intel.com> <20190131122606.GA12470@linux.intel.com> <20190131160437.GA5629@linux.intel.com> <20190131170603.GA18349@linux.intel.com> <20190131183530.GA27112@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jan 31, 2019 at 10:51:03AM -0800, Linus Torvalds wrote: > On Thu, Jan 31, 2019 at 10:35 AM Jarkko Sakkinen > wrote: > > > > OK, so the length of the response is not trashed, but only the error > > code. The attached patch fully fixes the issue. > > > > Here's the header again: > > > > struct tpm_output_header { > > __be16 tag; > > __be32 length; > > __be32 return_code; > > } __packed; > > > > The first to fields *are* read correctly and the last field get 1's > > (thus TPM error -1). > > Ok, so this makes sense, even though that patch is (I think) completely wrong. I don't disagree on that :-) Just pinpointed the location where it fails. > What happens is that the 32-bit fields are mis-aligned: the "tag" is > obviously properly 16-bit aligned, but then both "length" and > "return_code" are 32-bit fields that are only aligned on a 16-bit > alignment. > > What happens is that first you copy the two first fields: > > memcpy_fromio(buf, priv->rsp, 6); > > which copies "tag" and "length", but it copies them by reading then as > a 4-byte and then 2-byte value (in that order). So it actually reads > 'tag' and 'first two bytes of 'length', and then the second access > reads the last two bytes of 'length' > > And it all works, because the accesses are aligned by address of > access, even though they are *not* aligned in the 'struct > tpm_output_header' fields. Right, they are still naturally aligned accesses. > But then later on, when you read 'return_code', and do > > memcpy_fromio(&buf[6], &priv->rsp[6], expected - 6); > > you now do a 4-byte memcpy at offset 6. So it does a 4-byte access, > bit it's not 4-byte aligned. > > Honestly, memcpy() itself shouldn't have worked *either*, but you > lucked out. Gcc doesn't know that it's a 4-byte access, so it actually > calls out to the memcpy() routine. And that one happened to be "rep > movsb" on your machine. And that happened to work. I understand what you mean. Just surprised that this hasn't failed before to anyone (the same driver has been even successfully used on ARM64 with TrustZone based fTPM implementation). It has been in for three years now. > But it's really not supposed to work, and it really *wouldn't* have > worked if somebody disabled the rep-string functions. > > In fact, we have another patch (that isn't applied) that makes even > the memcpy_erms() just call the sw version of memcpy() for short > copies (because "rep movsb" is slow for those cases). That would also > have broken your driver. > > Linus /Jarkko