Skip to contentSkip to conversation

drivers/tpm/tpm_tis_sandbox.c: Fix uninitialized variable use

PATCH New Tom Rini 3 participants 12 messages Apr 12, 2016 – Apr 22, 2016 GPIO

lore
Share & tools
lore

Tom Rini

author
#0 lore raw

PATCH 1/3 drivers/tpm/tpm_tis_sandbox.c: Fix uninitialized variable use

In rollback_space_kernel we were not initializing the reserved fields which should be for safety sake, and doing memset here means we don't need to set the version field specifically either.

Reported-by Coverity (CID: 143917)
Cc Simon Glass <[email protected]>
Signed-off-by Tom Rini <[email protected]>
---
 drivers/tpm/tpm_tis_sandbox.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/tpm/tpm_tis_sandbox.c +1 -1
index 4aade56..e7746dc 100644
@@ -214,9 +214,9 @@ static int sandbox_tpm_xfer(struct udevice *dev, const uint8_t *sendbuf,
 
 			data = recvbuf + TPM_RESPONSE_HEADER_LENGTH +
 					sizeof(uint32_t);
+			memset(&rsk, 0, sizeof(struct rollback_space_kernel));
 			rsk.struct_version = 2;
 			rsk.uid = ROLLBACK_SPACE_KERNEL_UID;
-			rsk.kernel_versions = 0;
 			rsk.crc8 = crc8(0, (unsigned char *)&rsk,
 					offsetof(struct rollback_space_kernel,
 						 crc8));
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH 2/3 drivers/gpio/pm8916_gpio.c: Make pid be uint32_t

If get_dev_addr fails it will return FDT_ADDR_T_NONE and:

Quoted 1 line
Quoted 1 line
Quoted 1 line
    "priv->pid == 4294967295U" is always false regardless of the values of its operands. This occurs as the logical operand of if.
Cc Mateusz Kulikowski <[email protected]>
Reported-by Coverity (CID: 143913)
Signed-off-by Tom Rini <[email protected]>
---
 drivers/gpio/pm8916_gpio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpio/pm8916_gpio.c +1 -1
index 1abab7f..0b61975 100644
@@ -50,7 +50,7 @@ DECLARE_GLOBAL_DATA_PTR;
 #define REG_EN_CTL_ENABLE      (1 << 7)
 
 struct pm8916_gpio_bank {
-	uint16_t pid; /* Peripheral ID on SPMI bus */
+	uint32_t pid; /* Peripheral ID on SPMI bus */
 };
 
 static int pm8916_gpio_set_direction(struct udevice *dev, unsigned offset,
Signature · 1 line
1.9.1
#2 lore raw

PATCH 2/3 drivers/gpio/pm8916_gpio.c: Make pid be uint32_t

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi,

Sorry for slow reply.

Quoted 5 lines · Tom Rini

If get_dev_addr fails it will return FDT_ADDR_T_NONE and:

Quoted 1 line
Quoted 1 line
Quoted 1 line
    "priv->pid == 4294967295U" is always false regardless of the values of its operands. This occurs as the logical operand of if.
Cc Mateusz Kulikowski <[email protected]>
Reported-by Coverity (CID: 143913)

[...]

drivers/gpio/pm8916_gpio.c DECLARE_GLOBAL_DATA_PTR; · line 56

Quoted 6 lines
drivers/gpio/pm8916_gpio.c +1 -1
-	uint16_t pid; /* Peripheral ID on SPMI bus */
+	uint32_t pid; /* Peripheral ID on SPMI bus */
 };
 
 static int pm8916_gpio_set_direction(struct udevice *dev, unsigned offset,

Note applies to two patches in this series (pm8916_gpio.c and pm8916.c)

I think (now, when the coverity pointed out mistake) that we should add in that case check if pid fits in 16-bits, as this is maximum pid value on spmi bus.

This checks should be done in pm8916_gpio_probe() and pm8916_probe().

Would you like to do it in your series or want me to post another patch on top of them?

Regards,
Mateusz
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJXFUIWAAoJELvtohmVtQzBow8H/i4ojMv7MM+4YdHZs/Tf1gdy LWh68je+eGX+Nt1pwVXdgh52NJ6/PFJvUstlHaPKpPhlCAfOEEno6aI05dp0UQ5E VG5bqyL1k1IoqDue+3xoL2tJ0IZvBTKDpeqw8rVDH5PEFp5/aXs9FKwZm9VcVpny +3fi0sxFACAOs+PjMPqP9meXVTZyRRCTh/dAeH4bwLRGp249Tgq50vMajYjq1CW5 SM5TGFKeUgs17xVs163INW5s6UnNmqjj+gmVr7HRDH2uDWoQ6P77U9Z+l4yvSb72 Fw/vi6pqnxWsj6DCBEBkZ+jY2SVGB4LXm3bhQH2FO6ljXAW7KYamt1w4DoD9ABU= =bczK
-----END PGP SIGNATURE-----

#3 lore raw

PATCH 2/3 drivers/gpio/pm8916_gpio.c: Make pid be uint32_t

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Quoted 19 lines · Mateusz Kulikowski

Hi,

Sorry for slow reply.

Quoted 5 lines · Tom Rini

If get_dev_addr fails it will return FDT_ADDR_T_NONE and:

Quoted 1 line
Quoted 1 line
Quoted 1 line
    "priv->pid == 4294967295U" is always false regardless of the values of its operands. This occurs as the logical operand of if.
Cc Mateusz Kulikowski <[email protected]>
Reported-by Coverity (CID: 143913)

[...]

drivers/gpio/pm8916_gpio.c DECLARE_GLOBAL_DATA_PTR; · line 56

Quoted 5 lines
drivers/gpio/pm8916_gpio.c +1 -1
-	uint16_t pid; /* Peripheral ID on SPMI bus */
+	uint32_t pid; /* Peripheral ID on SPMI bus */
 };
 
 static int pm8916_gpio_set_direction(struct udevice *dev, unsigned offset,
Quoted 8 lines

Note applies to two patches in this series (pm8916_gpio.c and pm8916.c)

I think (now, when the coverity pointed out mistake) that we should add in that case check if pid fits in 16-bits, as this is maximum pid value on spmi bus.

This checks should be done in pm8916_gpio_probe() and pm8916_probe().

Would you like to do it in your series or want me to post another patch on top of them?

Or even better - leave it as uint16_t and do check & cast in probe()

Matuesz
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJXFUJtAAoJELvtohmVtQzBblgH/ROdYlG668Kz8ak/sUo/xksn NFeWMDC8pORbCOLhPnHcEJhAPzx+a0c5pUWUvsloTnj8t08rM+pcmnVV+jW4DhAC TxUgXYKjXMzbNlYYGn4cfb1Qlyss0+/xkv3C33LwI4hQYmE46RtThYtZqt6Uk+lk 9/eGCHt+H1tbKoD1BZIbzUwoDMTGlv7FgpzsH0/iW1QsDhbfUc27jkZNUvDgZO3Y ElX9I9cSFD5oqSVO3X+iDtGXaD6UEOGU+nb42wkTc9gyg9qunS9TV2kJsrrJu2DD Le4mN8NevrxyYbRZZM7EThkBD7skHr8wKUm6ccoQZLuMK+ZaYaukGirYTFSBQS0= =MwzG
-----END PGP SIGNATURE-----

author
#4 lore raw

PATCH 2/3 drivers/gpio/pm8916_gpio.c: Make pid be uint32_t

Quoted 25 lines · Mateusz Kulikowski

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Quoted 21 lines · Mateusz Kulikowski

Hi,

Sorry for slow reply.

Quoted 5 lines · Tom Rini

If get_dev_addr fails it will return FDT_ADDR_T_NONE and:

Quoted 1 line
Quoted 1 line
Quoted 1 line
    "priv->pid == 4294967295U" is always false regardless of the values of its operands. This occurs as the logical operand of if.
Cc Mateusz Kulikowski <[email protected]>
Reported-by Coverity (CID: 143913)

[...]

drivers/gpio/pm8916_gpio.c DECLARE_GLOBAL_DATA_PTR; · line 56

Quoted 5 lines
drivers/gpio/pm8916_gpio.c +1 -1
-	uint16_t pid; /* Peripheral ID on SPMI bus */
+	uint32_t pid; /* Peripheral ID on SPMI bus */
 };
 
 static int pm8916_gpio_set_direction(struct udevice *dev, unsigned offset,

Note applies to two patches in this series (pm8916_gpio.c and pm8916.c)

Quoted 8 lines
Quoted 6 lines

I think (now, when the coverity pointed out mistake) that we should add in that case check if pid fits in 16-bits, as this is maximum pid value on spmi bus.

This checks should be done in pm8916_gpio_probe() and pm8916_probe().

Would you like to do it in your series or want me to post another patch on top of them?

Or even better - leave it as uint16_t and do check & cast in probe()

Further sanity checks on the data we get isn't a bad idea, so no casting.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160420/4578e853/attachment.sig>
author
#5 lore raw

PATCH 2/3 drivers/gpio/pm8916_gpio.c: Make pid be uint32_t

In reply to Mateusz Kulikowski
Quoted 24 lines · Mateusz Kulikowski

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi,

Sorry for slow reply.

Quoted 5 lines · Tom Rini

If get_dev_addr fails it will return FDT_ADDR_T_NONE and:

Quoted 1 line
Quoted 1 line
Quoted 1 line
    "priv->pid == 4294967295U" is always false regardless of the values of its operands. This occurs as the logical operand of if.
Cc Mateusz Kulikowski <[email protected]>
Reported-by Coverity (CID: 143913)

[...]

drivers/gpio/pm8916_gpio.c DECLARE_GLOBAL_DATA_PTR; · line 56

Quoted 6 lines
drivers/gpio/pm8916_gpio.c +1 -1
-	uint16_t pid; /* Peripheral ID on SPMI bus */
+	uint32_t pid; /* Peripheral ID on SPMI bus */
 };
 
 static int pm8916_gpio_set_direction(struct udevice *dev, unsigned offset,

Note applies to two patches in this series (pm8916_gpio.c and pm8916.c)

Quoted 6 lines

I think (now, when the coverity pointed out mistake) that we should add in that case check if pid fits in 16-bits, as this is maximum pid value on spmi bus.

This checks should be done in pm8916_gpio_probe() and pm8916_probe().

Would you like to do it in your series or want me to post another patch on top of them?

Please do a follow up, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160420/ba042fa9/attachment.sig>
#6 lore raw

PATCH 2/3 drivers/gpio/pm8916_gpio.c: Make pid be uint32_t

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi,

Quoted 1 line · Tom Rini

On Mon, Apr 18, 2016 at 10:23:24PM +0200, Mateusz Kulikowski wrote:

[..]

Quoted 10 lines
Quoted 7 lines

I think (now, when the coverity pointed out mistake) that we should add in that case check if pid fits in 16-bits, as this is maximum pid value on spmi bus.

This checks should be done in pm8916_gpio_probe() and pm8916_probe().

Would you like to do it in your series or want me to post another patch on top of them?

Please do a follow up, thanks!

Will do that, but it will take a few days as I have a lot of non-coding related assignments now :)

btw. SPMI core checks pid and sid internally during reads/writes so we're pretty safe anyway.

Regards,
Mateusz
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJXGjE2AAoJELvtohmVtQzBMj4H/3+zypiWH9BZdDheztNA6yZt vN2fIz9kWQB8GFoBktU7kRAAWXaTDB0CWsKQmcqnJtZJeqyUV0Dj1CEJoS4fcptD Lq0uljuufLkfkSb/XhRe5sLxWbvygHVrGBz/OemFCQivBlu8MEe4pwNyEUFwTHrc otF9Uk1y92HjsyaQhjMX6WWR5Ei8hrVSryppt1eRFoGBAw9uL7TcKnc96IKrbFZ+ ns/40n/K8O2k8J2JMxMXB1QbJj/lEj/y4yY2Il2B2Z3WHb4ZhErtoko55uh0bGf4 PoGH1wPTZxNrcKn0CCwnHxmIxGJMpJ8XyZYhsEI4J+ZBs7izzb4dSn8ekOsr1nI= =Y2ca
-----END PGP SIGNATURE-----

author
#7 lore raw

PATCH 2/3 drivers/gpio/pm8916_gpio.c: Make pid be uint32_t

In reply to Tom Rini
Quoted 6 lines · Tom Rini

If get_dev_addr fails it will return FDT_ADDR_T_NONE and:

Quoted 1 line
Quoted 1 line
Quoted 1 line
    "priv->pid == 4294967295U" is always false regardless of the values of its operands. This occurs as the logical operand of if.
Cc Mateusz Kulikowski <[email protected]>
Reported-by Coverity (CID: 143913)
Signed-off-by Tom Rini <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160421/20ed5d78/attachment.sig>
author
#8 lore raw

PATCH 3/3 drivers/power/pmic/pm8916.c: Make usid be uint32_t

In reply to Tom Rini

If get_dev_addr fails it will return FDT_ADDR_T_NONE and:

Quoted 1 line
Quoted 1 line
Quoted 1 line
    "priv->usid == 4294967295U" is always false regardless of the values of its operands. This occurs as the logical operand of if.
Cc Mateusz Kulikowski <[email protected]>
Reported-by Coverity (CID: 143914)
Signed-off-by Tom Rini <[email protected]>
---
 drivers/power/pmic/pm8916.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/power/pmic/pm8916.c +1 -1
index 9acf5f5..d4c7d4a 100644
@@ -18,7 +18,7 @@ DECLARE_GLOBAL_DATA_PTR;
 #define REG_MASK 0xFF
 
 struct pm8916_priv {
-	uint16_t usid; /* Slave ID on SPMI bus */
+	uint32_t usid; /* Slave ID on SPMI bus */
 };
 
 static int pm8916_reg_count(struct udevice *dev)
Signature · 1 line
1.9.1
author
#9 lore raw

PATCH 3/3 drivers/power/pmic/pm8916.c: Make usid be uint32_t

Quoted 6 lines · Tom Rini

If get_dev_addr fails it will return FDT_ADDR_T_NONE and:

Quoted 1 line
Quoted 1 line
Quoted 1 line
    "priv->usid == 4294967295U" is always false regardless of the values of its operands. This occurs as the logical operand of if.
Cc Mateusz Kulikowski <[email protected]>
Reported-by Coverity (CID: 143914)
Signed-off-by Tom Rini <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160421/a99c38e2/attachment.sig>
#10 lore raw

PATCH 1/3 drivers/tpm/tpm_tis_sandbox.c: Fix uninitialized variable use

In reply to Tom Rini
Quoted 10 lines · Tom Rini

In rollback_space_kernel we were not initializing the reserved fields which should be for safety sake, and doing memset here means we don't need to set the version field specifically either.

Reported-by Coverity (CID: 143917)
Cc Simon Glass <[email protected]>
Signed-off-by Tom Rini <[email protected]>
---
 drivers/tpm/tpm_tis_sandbox.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by Simon Glass <[email protected]>
#11 lore raw

PATCH 1/3 drivers/tpm/tpm_tis_sandbox.c: Fix uninitialized variable use

Quoted 5 lines · Simon Glass
Quoted 4 lines · Tom Rini

In rollback_space_kernel we were not initializing the reserved fields which should be for safety sake, and doing memset here means we don't need to set the version field specifically either.

Quoted 8 lines
Quoted 6 lines
Reported-by Coverity (CID: 143917)
Cc Simon Glass <[email protected]>
Signed-off-by Tom Rini <[email protected]>
---
 drivers/tpm/tpm_tis_sandbox.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by Simon Glass <[email protected]>

Applied to u-boot-dm, thanks!