Subject: Broken ioremap usage in fsl_pq_mdio.c?

From: Timur Tabi <[email-masked]>
Date: Fri, 6 Jan 2012 15:11:22 -0600

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