On Tue, 5 Aug 2008, Simon Horman wrote:
> Check the return value of device_create_bin_file in pci_create_bus,
> unwind if neccessary, and propogate any errors to the caller.
>
> Signed-off-by: Simon Horman <horms@xxxxxxxxxxxx>
>
> ---
>
> drivers/pci/probe.c: In function `pci_create_bus':
> drivers/pci/probe.c:66: warning: ignoring return value of `device_create_bin_file', declared with attribute warn_unused_result
> drivers/pci/probe.c:74: warning: ignoring return value of `device_create_bin_file', declared with attribute warn_unused_result
>
> # ia64-unknown-linux-gnu-gcc --version
> ia64-unknown-linux-gnu-gcc (GCC) 3.4.5
> Copyright (C) 2004 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions. There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>
> Index: linux-2.6/drivers/pci/probe.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/probe.c 2008-08-05 19:58:43.000000000 +1000
> +++ linux-2.6/drivers/pci/probe.c 2008-08-05 19:59:15.000000000 +1000
> @@ -53,26 +53,37 @@ EXPORT_SYMBOL(no_pci_devices);
> * a per-bus basis. This routine creates the files and ties them into
> * their associated read, write and mmap files from pci-sysfs.c
> */
> -static void pci_create_legacy_files(struct pci_bus *b)
> +static int pci_create_legacy_files(struct pci_bus *b)
> {
> + int error;
> +
> b->legacy_io = kzalloc(sizeof(struct bin_attribute) * 2,
> GFP_ATOMIC);
> - if (b->legacy_io) {
> - b->legacy_io->attr.name = "legacy_io";
> - b->legacy_io->size = 0xffff;
> - b->legacy_io->attr.mode = S_IRUSR | S_IWUSR;
> - b->legacy_io->read = pci_read_legacy_io;
> - b->legacy_io->write = pci_write_legacy_io;
> - device_create_bin_file(&b->dev, b->legacy_io);
> -
> - /* Allocated above after the legacy_io struct */
> - b->legacy_mem = b->legacy_io + 1;
> - b->legacy_mem->attr.name = "legacy_mem";
> - b->legacy_mem->size = 1024*1024;
> - b->legacy_mem->attr.mode = S_IRUSR | S_IWUSR;
> - b->legacy_mem->mmap = pci_mmap_legacy_mem;
> - device_create_bin_file(&b->dev, b->legacy_mem);
> + if (!b->legacy_io)
> + return -ENOMEM;
> +
> + b->legacy_io->attr.name = "legacy_io";
> + b->legacy_io->size = 0xffff;
> + b->legacy_io->attr.mode = S_IRUSR | S_IWUSR;
> + b->legacy_io->read = pci_read_legacy_io;
> + b->legacy_io->write = pci_write_legacy_io;
> + error = device_create_bin_file(&b->dev, b->legacy_io);
> + if (error)
> + return error;
I'd release the memory here and NULLify legacy_io.
> +
> + /* Allocated above after the legacy_io struct */
> + b->legacy_mem = b->legacy_io + 1;
> + b->legacy_mem->attr.name = "legacy_mem";
> + b->legacy_mem->size = 1024*1024;
> + b->legacy_mem->attr.mode = S_IRUSR | S_IWUSR;
> + b->legacy_mem->mmap = pci_mmap_legacy_mem;
> + error = device_create_bin_file(&b->dev, b->legacy_mem);
> + if (error) {
> + device_remove_bin_file(&b->dev, b->legacy_io);
> + return error;
Here too.
Reason: If we fail to create the legacy_io file, legacy_mem will still be
NULL, because it has been not initialized at that point. But we will try
to remove it in pci_remove_legacy_files and in sysfs_remove_bin_file
we're going to derefence it and blow up.
> }
> +
> + return 0;
> }
>
> void pci_remove_legacy_files(struct pci_bus *b)
> @@ -84,7 +95,7 @@ void pci_remove_legacy_files(struct pci_
> }
> }
> #else /* !HAVE_PCI_LEGACY */
> -static inline void pci_create_legacy_files(struct pci_bus *bus) { return; }
> +static inline int pci_create_legacy_files(struct pci_bus *bus) { return 0; }
> void pci_remove_legacy_files(struct pci_bus *bus) { return; }
> #endif /* HAVE_PCI_LEGACY */
>
> @@ -1157,7 +1168,9 @@ struct pci_bus * pci_create_bus(struct d
> goto dev_create_file_err;
>
> /* Create legacy_io and legacy_mem files for this bus */
> - pci_create_legacy_files(b);
> + error = pci_create_legacy_files(b);
> + if (error)
> + goto pci_create_legacy_files_err;
>
> b->number = b->secondary = bus;
> b->resource[0] = &ioport_resource;
> @@ -1167,6 +1180,8 @@ struct pci_bus * pci_create_bus(struct d
>
> return b;
>
> +pci_create_legacy_files_err:
> + device_remove_file(&b->dev, &dev_attr_cpuaffinity);
> dev_create_file_err:
> device_unregister(&b->dev);
> class_dev_reg_err:
--
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
