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;
Date:   Fri, 8 Feb 2019 15:17:54 +0200
From:   Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
To:     Stefan Berger <stefanb@linux.ibm.com>
Cc:     Alexander Steffen <Alexander.Steffen@infineon.com>,
        linux-integrity@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-security-module@vger.kernel.org,
        Peter Huewe <PeterHuewe@gmx.de>,
        Jason Gunthorpe <jgg@ziepe.ca>,
        Tomas Winkler <tomas.winkler@intel.com>,
        Tadeusz Struk <tadeusz.struk@intel.com>,
        Stefan Berger <stefanb@linux.vnet.ibm.com>,
        Nayna Jain <nayna@linux.ibm.com>
Subject: Re: [PATCH v11 00/16] Remove nested TPM operations
Message-ID: <20190208131754.GA18133@linux.intel.com>
References: <20190206120634.GA4680@linux.intel.com>
 <3d6fdcc1-8221-2137-fc82-596377cc2fdc@infineon.com>
 <20190207212912.GD23906@linux.intel.com>
 <0af40351-91d5-d0a7-2965-0a9c6505af79@linux.ibm.com>
 <20190208003314.GB7202@linux.intel.com>
 <1fbd29ad-1005-8450-4c39-00974468183b@linux.ibm.com>
 <20190208111442.GA11755@linux.intel.com>
 <06473b24-7c59-9405-bbe0-8d30b3450391@linux.ibm.com>
 <20190208130227.GA16679@linux.intel.com>
 <da69e24d-ad5c-1e7b-9f02-a435872ad635@linux.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <da69e24d-ad5c-1e7b-9f02-a435872ad635@linux.ibm.com>
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

On Fri, Feb 08, 2019 at 08:10:32AM -0500, Stefan Berger wrote:
> On 2/8/19 8:02 AM, Jarkko Sakkinen wrote:
> > On Fri, Feb 08, 2019 at 07:05:26AM -0500, Stefan Berger wrote:
> > > See my comment on [PATCH v11 08/16]. It needs to be added in that patch
> > > since otherwise rc holds a non-zero value on function exit, which is wrong
> > > at that point.
> > The snippet in question:
> > 
> > rc = chip->ops->send(chip, buf, count);
> > if (rc < 0) {
> > 	if (rc != -EPIPE)
> > 		dev_err(&chip->dev,
> > 			"%s: tpm_send: error %d\n", __func__, rc);
> > 	return rc;
> > }
> > 
> > if (chip->flags & TPM_CHIP_FLAG_IRQ)
> > 	goto out_recv;
> > 
> > 'send()' ought to return zero on success case.
> > 
> > This is how the snippet was before applying any patches scheduled for
> > v5.1:
> > 
> > rc = chip->ops->send(chip, buf, count);
> > if (rc < 0) {
> > 	if (rc != -EPIPE)
> > 		dev_err(&chip->dev,
> > 			"%s: tpm_send: error %d\n", __func__, rc);
> > 	return rc;
> > }
> > 
> > if (chip->flags & TPM_CHIP_FLAG_IRQ)
> > 	goto out_recv;
> > 
> > Does not compute.
> 
> tpm_tis_send_main returns 'len' and that's what we have here.

Before doing any kind of code change, we should at least know what
has caused this that it has worked before.

And also which commit caused the regression to happen, because it
looks like a bug in tpm_tis_core, not in the main TPM driver. It
would need the fixes tag and cc to stable.

/Jarkko