From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ed Swierk Subject: Re: [PATCH v6 3/5] tpm: Factor out reading of timeout and duration capabilities Date: Mon, 20 Jun 2016 18:46:50 -0700 Message-ID: References: <1465426818-89356-1-git-send-email-eswierk@skyportsystems.com> <1465610107-87762-1-git-send-email-eswierk@skyportsystems.com> <1465610107-87762-4-git-send-email-eswierk@skyportsystems.com> <20160619120157.GA29626@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============4134684558862644046==" Return-path: In-Reply-To: <20160619120157.GA29626-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: tpmdd-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org To: Jarkko Sakkinen Cc: linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: tpmdd-devel@lists.sourceforge.net --===============4134684558862644046== Content-Type: multipart/alternative; boundary=001a113d0efcfd3c9e0535c000cb --001a113d0efcfd3c9e0535c000cb Content-Type: text/plain; charset=UTF-8 On Sun, Jun 19, 2016 at 5:12 AM, Jarkko Sakkinen < jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote: > I think inside tpm_get_timeouts() I'd rather something along the lines > (with error handling and such details taken away): > > rc = tpm_getcap(...); > > if (rc == TPM_ERR_INVALID_POSTINIT) { > tpm_startup(...); > tpm_getca(...); > } > Agreed. > + if (rc) { > > + dev_err(chip->pdev, > > + "Error %zd reading %s\n", rc, desc); > > + return rc; > > + } > > + > > + if (be32_to_cpu(tpm_cmd.header.out.return_code) != 0 || > > + be32_to_cpu(tpm_cmd.header.out.length) > > + != sizeof(tpm_cmd.header.out) + sizeof(u32) + size * > sizeof(u32)) { > > + dev_err(chip->pdev, > > + "Bad return code or length reading %s\n", desc); > > + return -EINVAL; > > + } > > This is bogus code. All this kind of checks should be contained in > tpm_transmit_cmd(). This is easily "fixed" by moving tpm_getcap() :) > tpm_transmit_cmd() already checks the return_code, so that's easy. It doesn't know the expected length, but tpm_getcap() does (or can, based on the subcap_id). --Ed --001a113d0efcfd3c9e0535c000cb Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
On S= un, Jun 19, 2016 at 5:12 AM, Jarkko Sakkinen <jarkko.sakkin= en-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
= I think inside tpm_get_timeouts() I'd rather something along the lines<= br> (with error handling and such details taken away):

rc =3D tpm_getcap(...);

if (rc =3D=3D TPM_ERR_INVALID_POSTINIT) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 tpm_startup(...);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 tpm_getca(...);
}

Agreed.
=

> +=C2=A0 =C2=A0 =C2=A0if (rc) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dev_err(chip->pdev= ,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0"Error %zd reading %s\n", rc, desc);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return rc;
> +=C2=A0 =C2=A0 =C2=A0}
> +
> +=C2=A0 =C2=A0 =C2=A0if (be32_to_cpu(tpm_cmd.header.out.return_code) != =3D 0 ||
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0be32_to_cpu(tpm_cmd.header.out.leng= th)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0!=3D sizeof(tpm_cmd.header.out) + s= izeof(u32) + size * sizeof(u32)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dev_err(chip->pdev= ,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0"Bad return code or length reading %s\n", desc);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return -EINVAL;
> +=C2=A0 =C2=A0 =C2=A0}

This is bogus code. All this kind of checks should be contained in tpm_transmit_cmd(). This is easily "fixed" by moving tpm_getcap()= :)

tpm_transmit_cmd() already checks t= he return_code, so that's easy. It doesn't know the expected length= , but tpm_getcap() does (or can, based on the subcap_id).

--Ed

--001a113d0efcfd3c9e0535c000cb-- --===============4134684558862644046== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline ------------------------------------------------------------------------------ Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San Francisco, CA to explore cutting-edge tech and listen to tech luminaries present their vision of the future. This family event has something for everyone, including kids. Get more information and register today. http://sdm.link/attshape --===============4134684558862644046== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ tpmdd-devel mailing list tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org https://lists.sourceforge.net/lists/listinfo/tpmdd-devel --===============4134684558862644046==--