[PATCH] tpm: avoid NULL pointer dereference in tpm_tis_send()

[Ilias Apalodimas]

On Sat, Nov 27, 2021 at 11:57:42AM +0100, Heinrich Schuchardt wrote:
> 
> 
> On 11/27/21 00:38, Ilias Apalodimas wrote:
> > Hi Heinrich,
> > 
> > On Fri, Nov 26, 2021 at 11:54:52PM +0100, Heinrich Schuchardt wrote:
> > > You should not dereference a pointer before checking if it is NULL.
> > > 
> > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> > > ---
> > >   drivers/tpm/tpm2_tis_core.c | 4 +++-
> > >   1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/tpm/tpm2_tis_core.c b/drivers/tpm/tpm2_tis_core.c
> > > index ec8c730fe9..0ff21a1ef7 100644
> > > --- a/drivers/tpm/tpm2_tis_core.c
> > > +++ b/drivers/tpm/tpm2_tis_core.c
> > > @@ -218,7 +218,7 @@ static int tpm_tis_ready(struct udevice *dev)
> > >   int tpm_tis_send(struct udevice *dev, const u8 *buf, size_t len)
> > >   {
> > >   	struct tpm_chip *chip = dev_get_priv(dev);
> > > -	struct tpm_tis_phy_ops *phy_ops = chip->phy_ops;
> > > +	struct tpm_tis_phy_ops *phy_ops;
> > >   	size_t burstcnt, wr_size, sent = 0;
> > >   	u8 data = TPM_STS_GO;
> > >   	u8 status;
> > > @@ -227,6 +227,8 @@ int tpm_tis_send(struct udevice *dev, const u8 *buf, size_t len)
> > >   	if (!chip)
> > >   		return -ENODEV;
> > > +	phy_ops = chip->phy_ops;
> > > +
> > 
> > At this point I believe dev_get_priv() wont fail, since the device is
> > opened and running.  We can remove the if check though.
> 
> What happens if the user issues the 'efidebug' command followed by the
> 'unbind' command to remove the TPM before launching an image?
> 

These functions are called as part of the tpm-uclass.  The current TPM uclass
code is looking for priv and tries to derive the device ops.  So if anything 
goes wrong, that will happen way before this even gets invoked.

I'd much rather prefer doing the checks for that there instead of the API code.
It makes more sense to me to have the DM code provide an assurance the ptr is 
always present instead of checking the pointers on every function.

Thanks
/Ilias

> Best regards
> 
> Heinrich
> 
> > 
> > >   	ret = tpm_tis_request_locality(dev, 0);
> > >   	if (ret < 0)
> > >   		return -EBUSY;
> > > -- 
> > > 2.32.0
> > > 
> > 
> > 
> > Thanks!
> > /Ilias
> >