Skip to contentSkip to conversation

tpm: fix tpm_acpi sparse warning on different address spaces

PATCH In review Kent Yoder 3 participants 7 messages Aug 30, 2012 – Sep 12, 2012

Subsystems (2)
Kernel janitors TPM device driver
lore
Share & tools
lore

Kent Yoder

author
#0 lore raw

PATCH 1/1 tpm: fix tpm_acpi sparse warning on different address spaces

acpi_os_map_memory expects its return value to be in the __iomem address space. Cast it back later when used in a memcpy to avoid the same sparse warning there.

Signed-off-by Kent Yoder <[email protected]>
---
 drivers/char/tpm/tpm_acpi.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/char/tpm/tpm_acpi.c +2 -2
index fe3fa94..36bdcfa 100644
@@ -49,7 +49,7 @@ int read_log(struct tpm_bios_log *log)
 {
 	struct acpi_tcpa *buff;
 	acpi_status status;
-	struct acpi_table_header *virt;
+	void __iomem *virt;
 	u64 len, start;
 
 	if (log->bios_event_log != NULL) {

@@ -102,7 +102,7 @@ int read_log(struct tpm_bios_log *log)
 		return -EIO;
 	}
 
-	memcpy(log->bios_event_log, virt, len);
+	memcpy(log->bios_event_log, (void *)virt, len);
 
 	acpi_os_unmap_memory(virt, len);
 	return 0;
Signature · 1 line
1.7.5.4
#1 lore raw
Quoted 5 lines · Kent Yoder

acpi_os_map_memory expects its return value to be in the __iomem address space. Cast it back later when used in a memcpy to avoid the same sparse warning there.

Signed-off-by Kent Yoder <[email protected]>

I can't download linux-next right now but I don't think this is the correct fix. I don't think you can memcpy() directly from __iomem. The static checker is warning about a valid bug which should be addressed instead of just casted away and silenced.

regards,
dan carpenter

author
#2 lore raw
Quoted 11 lines · Dan Carpenter
Quoted 5 lines · Kent Yoder

acpi_os_map_memory expects its return value to be in the __iomem address space. Cast it back later when used in a memcpy to avoid the same sparse warning there.

Signed-off-by Kent Yoder <[email protected]>

I can't download linux-next right now but I don't think this is the correct fix. I don't think you can memcpy() directly from __iomem. The static checker is warning about a valid bug which should be addressed instead of just casted away and silenced.

I took a look at other uses of the return from acpi_os_map_memory(),
such as acpi_tb_parse_fadt(), which passes the returned pointer to acpi_tb_create_local_fadt(), which uses ACPI_MEMCPY on it, which is just memcpy. This code lives in drivers/acpi.

Kent

Quoted 5 lines

regards,
dan carpenter

#3 lore raw
Quoted 13 lines · Kent Yoder
Quoted 11 lines · Dan Carpenter
Quoted 5 lines · Kent Yoder

acpi_os_map_memory expects its return value to be in the __iomem address space. Cast it back later when used in a memcpy to avoid the same sparse warning there.

Signed-off-by Kent Yoder <[email protected]>

I can't download linux-next right now but I don't think this is the correct fix. I don't think you can memcpy() directly from __iomem. The static checker is warning about a valid bug which should be addressed instead of just casted away and silenced.

Quoted 5 lines

I took a look at other uses of the return from acpi_os_map_memory(),
such as acpi_tb_parse_fadt(), which passes the returned pointer to acpi_tb_create_local_fadt(), which uses ACPI_MEMCPY on it, which is just memcpy. This code lives in drivers/acpi.

Sparse finds that bug as well. ;)

drivers/acpi/acpica/tbfadt.c:247:15: warning: incorrect type in assignment (different address spaces)
drivers/acpi/acpica/tbfadt.c:247:15: expected struct acpi_table_header *table
drivers/acpi/acpica/tbfadt.c:247:15: got void [noderef] <asn:2>*
drivers/acpi/acpica/tbfadt.c:266:30: warning: incorrect type in argument 1 (different address spaces)
drivers/acpi/acpica/tbfadt.c:266:30: expected void [noderef] <asn:2>*logical_address
drivers/acpi/acpica/tbfadt.c:266:30: got struct acpi_table_header *table

It should be memcpy_fromio() probably.

regards,
dan carpenter

author
#4 lore raw
Quoted 10 lines · Dan Carpenter

Sparse finds that bug as well. ;)

drivers/acpi/acpica/tbfadt.c:247:15: warning: incorrect type in assignment (different address spaces)
drivers/acpi/acpica/tbfadt.c:247:15: expected struct acpi_table_header *table
drivers/acpi/acpica/tbfadt.c:247:15: got void [noderef] <asn:2>*
drivers/acpi/acpica/tbfadt.c:266:30: warning: incorrect type in argument 1 (different address spaces)
drivers/acpi/acpica/tbfadt.c:266:30: expected void [noderef] <asn:2>*logical_address
drivers/acpi/acpica/tbfadt.c:266:30: got struct acpi_table_header *table

It should be memcpy_fromio() probably.

Thanks Dan, that's clearly better. James, I'll resubmit this one
today.

Thanks,
Kent

author
#5 lore raw

PATCH 1/1 tpm: fix tpm_acpi sparse warning on different address spaces

In reply to Dan Carpenter

acpi_os_map_memory expects its return value to be in the __iomem address space. Tag the variable we're using as such and use memcpy_fromio to avoid further sparse warnings.

Signed-off-by Kent Yoder <[email protected]>
---
 drivers/char/tpm/tpm_acpi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/char/tpm/tpm_acpi.c +2 -2
index fe3fa94..56051d0 100644
@@ -49,7 +49,7 @@ int read_log(struct tpm_bios_log *log)
 {
 	struct acpi_tcpa *buff;
 	acpi_status status;
-	struct acpi_table_header *virt;
+	void __iomem *virt;
 	u64 len, start;
 
 	if (log->bios_event_log != NULL) {

@@ -102,7 +102,7 @@ int read_log(struct tpm_bios_log *log)
 		return -EIO;
 	}
 
-	memcpy(log->bios_event_log, virt, len);
+	memcpy_fromio(log->bios_event_log, virt, len);
 
 	acpi_os_unmap_memory(virt, len);
 	return 0;
Signature · 1 line
1.7.11.4
#6 lore raw
Quoted 5 lines · Kent Yoder

acpi_os_map_memory expects its return value to be in the __iomem address space. Tag the variable we're using as such and use memcpy_fromio to avoid further sparse warnings.

Signed-off-by Kent Yoder <[email protected]>

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next

Signature · 2 lines
James Morris
<[email protected]>