* Broken ioremap usage in fsl_pq_mdio.c?
@ 2012-01-06 21:11 Timur Tabi
0 siblings, 0 replies; only message in thread
From: Timur Tabi @ 2012-01-06 21:11 UTC (permalink / raw)
To: Andrew Fleming-AFLEMING, David Miller, netdev@vger.kernel.org
I see some odd code in fsl_pq_mdio.c, so I'm hoping someone can clear this up for me. In fsl_pq_mdio_probe(), I see this code:
/* Set the PHY base address */
addr = of_translate_address(np, addrp);
if (addr == OF_BAD_ADDR) {
err = -EINVAL;
goto err_free_bus;
}
map = ioremap(addr, size);
if (!map) {
err = -ENOMEM;
goto err_free_bus;
}
priv->map = map;
if (of_device_is_compatible(np, "fsl,gianfar-mdio") ||
of_device_is_compatible(np, "fsl,gianfar-tbi") ||
of_device_is_compatible(np, "fsl,ucc-mdio") ||
of_device_is_compatible(np, "ucc_geth_phy"))
map -= offsetof(struct fsl_pq_mdio, miimcfg);
regs = map;
priv->regs = regs;
I don't see how the "map -= offsetof(struct fsl_pq_mdio, miimcfg)" is valid. 'map' initially points to the beginning of an ioremap'd buffer. Subtracting from it will cause it to point to invalid memory area.
I presume this works because ioremap() maps whole pages, and "map - offsetof(struct fsl_pq_mdio, miimcfg)" is still inside that page. However, this is not documented as an assumption, so I don't know if this is really what's intended. Regardless, it seems very fragile. At the very least, there should be another ioremap for priv->regs, but that's still a hack.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2012-01-06 21:11 UTC | newest]
Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-06 21:11 Broken ioremap usage in fsl_pq_mdio.c? Timur Tabi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).