linux-kernel - Re: [patch] PCI: check the return value of device_create_bin_file() in pci_create

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]

Message-ID: <20080806233018.GG10636@verge.net.au>
Date:	Thu, 7 Aug 2008 09:30:20 +1000
From:	Simon Horman <horms@...ge.net.au>
To:	Matthew Wilcox <matthew@....cx>
Cc:	Sven Wegener <sven.wegener@...aler.net>,
	Michael Ellerman <michael@...erman.id.au>,
	linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
	Jesse Barnes <jbarnes@...tuousgeek.org>
Subject: Re: [patch] PCI: check the return value of
	device_create_bin_file() in pci_create_bus()

Check the return value of device_create_bin_file in pci_create_bus and
unwind if necessary. Don't propagate error to caller, as failure to create
these files shouldn't prevent PCI from being initialised.

Cc: Sven Wegener <sven.wegener@...aler.net>
Cc: Michael Ellerman <michael@...erman.id.au>
Cc: Matthew Wilcox <matthew@....cx>
Signed-off-by: Simon Horman <horms@...ge.net.au>

--- 

Tue, 5 Aug 2008 21:14:07 +1000
* Revised the patch to free and NULLify b->legacy_io in
  pci_create_legacy_files() on error. Thanks to
  Sven Wegener and Michael Ellerman for pointing out this omission.

Wed, 06 Aug 2008 10:49:30 +1000
* Don't propagate error to caller, as failure to create these files
  shouldn't prevent PCI from being initialised.
  Thanks to Matthew Wilcox

Thu, 07 Aug 2008 09:28:56 +1000
* Remove spurious return value.
  Amusingly this patch was introducing warnings :-)

This patch resolves the following warnings:

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-07 09:08:32.000000000 +1000
+++ linux-2.6/drivers/pci/probe.c	2008-08-07 09:14:53.000000000 +1000
@@ -52,27 +52,46 @@ EXPORT_SYMBOL(no_pci_devices);
  * Some platforms allow access to legacy I/O port and ISA memory space on
  * a per-bus basis.  This routine creates the files and ties them into
  * their associated read, write and mmap files from pci-sysfs.c
+ *
+ * On error unwind, but don't propogate the error to the caller
+ * as it is ok to set up the PCI bus without these files.
  */
 static void 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;
+
+	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)
+		goto legacy_io_err;
+
+	/* 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)
+		goto legacy_mem_err;
+
+	return;
+
+legacy_mem_err:
+	device_remove_bin_file(&b->dev, b->legacy_io);
+legacy_io_err:
+	kfree(b->legacy_io);
+	b->legacy_io = NULL;
+	return;
 }
 
 void pci_remove_legacy_files(struct pci_bus *b)
--
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/