On Sunday, August 17, 2008 12:06 pm Jean Delvare wrote:
> Hi all,
> From: Jean Delvare <khali@xxxxxxxxxxxx>
> Subject: PCI: Check dynids driver_data value for validity
>
> Only accept dynids those driver_data value matches one of the driver's
> pci_driver_id entry. This prevents the user from accidentally passing
> values the drivers do not expect.
>
> Signed-off-by: Jean Delvare <khali@xxxxxxxxxxxx>
> Cc: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
> Cc: Milton Miller <miltonm@xxxxxxx>
> Cc: Greg KH <greg@xxxxxxxxx>
> ---
> Documentation/PCI/pci.txt | 4 ++++
> drivers/i2c/busses/i2c-amd756.c | 4 ----
> drivers/i2c/busses/i2c-viapro.c | 4 ----
> drivers/pci/pci-driver.c | 18 ++++++++++++++++--
> 4 files changed, 20 insertions(+), 10 deletions(-)
>
> --- linux-2.6.27-rc3.orig/Documentation/PCI/pci.txt 2008-08-17
> 18:24:33.000000000 +0200 +++
> linux-2.6.27-rc3/Documentation/PCI/pci.txt 2008-08-17 18:24:38.000000000
> +0200 @@ -163,6 +163,10 @@ need pass only as many optional fields a
> o class and classmask fields default to 0
> o driver_data defaults to 0UL.
>
> +Note that driver_data must match the value used by any of the
> pci_device_id +entries defined in the driver. This makes the driver_data
> field mandatory +if all the pci_device_id entries have a non-zero
> driver_data value. +
> Once added, the driver probe routine will be invoked for any unclaimed
> PCI devices listed in its (newly updated) pci_ids list.
>
> --- linux-2.6.27-rc3.orig/drivers/i2c/busses/i2c-amd756.c 2008-08-17
> 17:15:57.000000000 +0200 +++
> linux-2.6.27-rc3/drivers/i2c/busses/i2c-amd756.c 2008-08-17
> 19:42:14.000000000 +0200 @@ -332,10 +332,6 @@ static int __devinit
> amd756_probe(struct
> int error;
> u8 temp;
>
> - /* driver_data might come from user-space, so check it */
> - if (id->driver_data >= ARRAY_SIZE(chipname))
> - return -EINVAL;
> -
> if (amd756_ioport) {
> dev_err(&pdev->dev, "Only one device supported "
> "(you have a strange motherboard, btw)\n");
> --- linux-2.6.27-rc3.orig/drivers/i2c/busses/i2c-viapro.c 2008-08-17
> 17:15:57.000000000 +0200 +++
> linux-2.6.27-rc3/drivers/i2c/busses/i2c-viapro.c 2008-08-17
> 19:42:24.000000000 +0200 @@ -320,10 +320,6 @@ static int __devinit
> vt596_probe(struct
> unsigned char temp;
> int error = -ENODEV;
>
> - /* driver_data might come from user-space, so check it */
> - if (id->driver_data & 1 || id->driver_data > 0xff)
> - return -EINVAL;
> -
> /* Determine the address of the SMBus areas */
> if (force_addr) {
> vt596_smba = force_addr & 0xfff0;
> --- linux-2.6.27-rc3.orig/drivers/pci/pci-driver.c 2008-08-17
> 17:15:57.000000000 +0200 +++
> linux-2.6.27-rc3/drivers/pci/pci-driver.c 2008-08-17 19:17:55.000000000
> +0200 @@ -43,18 +43,32 @@ store_new_id(struct device_driver *drive
> {
> struct pci_dynid *dynid;
> struct pci_driver *pdrv = to_pci_driver(driver);
> + const struct pci_device_id *ids = pdrv->id_table;
> __u32 vendor, device, subvendor=PCI_ANY_ID,
> subdevice=PCI_ANY_ID, class=0, class_mask=0;
> unsigned long driver_data=0;
> int fields=0;
> - int retval = 0;
> + int retval;
>
> - fields = sscanf(buf, "%x %x %x %x %x %x %lux",
> + fields = sscanf(buf, "%x %x %x %x %x %x %lx",
> &vendor, &device, &subvendor, &subdevice,
> &class, &class_mask, &driver_data);
> if (fields < 2)
> return -EINVAL;
>
> + /* Only accept driver_data values that match an existing id_table
> + entry */
> + retval = -EINVAL;
> + while (ids->vendor || ids->subvendor || ids->class_mask) {
> + if (driver_data == ids->driver_data) {
> + retval = 0;
> + break;
> + }
> + ids++;
> + }
> + if (retval) /* No match */
> + return retval;
> +
> dynid = kzalloc(sizeof(*dynid), GFP_KERNEL);
> if (!dynid)
> return -ENOMEM;
>
>
> * * * * *
>
> The patch above applies on top of Milton's patch removing
> dynids.use_driver_data.
Looks good; I think we'll want to put this into linux-next along with Milton's
change. I'll push them out after a quick smoke test.
Thanks,
Jesse
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
