linux-kernel - Re: [PATCH v2 2/3] tpm_tis: Use devm_ioremap

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]

Date:	Tue, 1 Dec 2015 12:44:19 -0700
From:	Jason Gunthorpe <jgunthorpe@...idianresearch.com>
To:	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>
Cc:	Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>,
	tpmdd-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org,
	Martin Wilck <Martin.Wilck@...fujitsu.com>,
	Peter Huewe <peterhuewe@....de>
Subject: Re: [PATCH v2 2/3] tpm_tis: Use devm_ioremap_resource

On Tue, Dec 01, 2015 at 08:22:40PM +0100, Uwe Kleine-König wrote:

> here, which IMHO reads nicer and maybe is even more efficient (I don't
> know much about x86).

Sure

> > +	if (resource_size(&tpm_info.res) == 0)
> > +		return -ENODEV;
> > +
> 
> Does this result in an error message from the upper layers?

I think so, yes. The probe will fail which causes the driver core to
report a message.

The scenario this triggers is if the acpi stuff doesn't have a mem
resource, which is a firmware bug, I think. It could get a dedicated
print if that is what you are thinking?

-	if (resource_size(&tpm_info.res) == 0)
+	if (tpm_info.res.flags == 0) {
+		dev_err(&pdev->dev, FW_BUG "no memory resource defined\n");
 		return -ENODEV;
+	}

 	if (is_itpm(acpi_dev))
 		itpm = true;

[resource_size is wrong as well since it will return 1 for
 0'd struct resource, sigh..]

Previously it would try to call devm_ioremap with start/len=0 as the
range which should also fails in broadly the same way. So this is just
moving the existing failure up.

Something was needed because the change to struct resource means the
new code would call devm_ioremap_resource with a 0'd resource struct,
which is not as safe as start/len=0 as before.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/