linux-kernel - Re: [PATCH] tpm: use get_unaligned

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20161123165734.GA2763@obsidianresearch.com>
Date:   Wed, 23 Nov 2016 09:57:35 -0700
From:   Jason Gunthorpe <jgunthorpe@...idianresearch.com>
To:     Tomas Winkler <tomas.winkler@...el.com>
Cc:     tpmdd-devel@...ts.sourceforge.net,
        Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] tpm: use get_unaligned_be32 unaligned buffer access.

On Wed, Nov 23, 2016 at 01:04:54PM +0200, Tomas Winkler wrote:
> Use get_unaligned_be32 as b32_to_cpu doesn't work correctly on
> all platforms for unaligned access.
> 
> The fix doesn't cover all the cases as also some cast
> structures have members on unaligned addresses.

I think this is a good idea..

> @@ -353,8 +353,8 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
>  	if (bufsiz > TPM_BUFSIZE)
>  		bufsiz = TPM_BUFSIZE;
>  
> -	count = be32_to_cpu(*((__be32 *) (buf + 2)));
> -	ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
> +	count = get_unaligned_be32(buf + 2);
> +	ordinal = get_unaligned_be32(buf + 6);

But lets fix this better and get rid of the constants too...

 const tpm_input_header *hdr = buf;
 count = be32_to_cpu(hdr->length);
 ordinal = be32_to_cpu(hdr->ordinal);

Compiler will take care of unaligned for __packed.

> @@ -178,7 +178,7 @@ static int crb_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>  		return -EIO;
>  
>  	memcpy_fromio(buf, priv->rsp, 6);
> -	expected = be32_to_cpup((__be32 *) &buf[2]);
> +	expected = get_unaligned_be32(buf + 2);

Here too, except tpm_output_header (and is tpm1 and 2 the same here?)

> @@ -451,7 +452,7 @@ static int tpm_tis_i2c_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>  		goto out;
>  	}
>  
> -	expected = be32_to_cpu(*(__be32 *)(buf + 2));
> +	expected = get_unaligned_be32(buf + 2);

Ditto

> @@ -314,7 +315,7 @@ static int i2c_nuvoton_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>  		 * convert number of expected bytes field from big endian 32 bit
>  		 * to machine native
>  		 */
> -		expected = be32_to_cpu(*(__be32 *) (buf + 2));
> +		expected = get_unaligned_be32(buf + 2);

Ditto

>  		if (expected > count) {
>  			dev_err(dev, "%s() expected > count\n", __func__);
>  			size = -EIO;
> @@ -442,7 +443,7 @@ static int i2c_nuvoton_send(struct tpm_chip *chip, u8 *buf, size_t len)
>  		i2c_nuvoton_ready(chip);
>  		return rc;
>  	}
> -	ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
> +	ordinal = get_unaligned_be32(buf + 6);

Ditto

> @@ -174,8 +173,7 @@ static int tpm_nsc_recv(struct tpm_chip *chip, u8 * buf, size_t count)
>  		return -EIO;
>  	}
>  
> -	native_size = (__force __be32 *) (buf + 2);
> -	size = be32_to_cpu(*native_size);
> +	size = get_unaligned_be32(buf + 2);

Ditto

> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 7993678954a2..5323c54dc917 100644
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -222,7 +222,7 @@ static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>  		goto out;
>  	}
>  
> -	expected = be32_to_cpu(*(__be32 *) (buf + 2));
> +	expected = get_unaligned_be32(buf + 2);

Ditto

> @@ -371,7 +371,7 @@ static int tpm_tis_send_main(struct tpm_chip *chip, u8 *buf, size_t len)
>  		goto out_err;
>  
>  	if (chip->flags & TPM_CHIP_FLAG_IRQ) {
> -		ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
> +		ordinal = get_unaligned_be32(buf + 6);

Ditto

Jason