dm: pci: Avoid a memory leak when allocating the ROM

Adjust pci_rom_load() to return an indication of whether it allocated memory or not. Adjust the caller to free it. This fixes a memory leak when PCI_VGA_RAM_IMAGE_START is not used. Reported-by: Coverity (CID: 134194) Signed-off-by: Simon Glass <sjg@chromium.org> Reviewed-by: Tom Rini <trini@konsulko.com>

dm: pci: Avoid a memory leak when allocating the ROM
Adjust pci_rom_load() to return an indication of whether it allocated memory or not. Adjust the caller to free it. This fixes a memory leak when PCI_VGA_RAM_IMAGE_START is not used. Reported-by: Coverity (CID: 134194) Signed-off-by: Simon Glass <sjg@chromium.org> Reviewed-by: Tom Rini <trini@konsulko.com>
Simon Glass · Tom Rini
1 parent f19345b51c
Showing 1 changed file with 33 additions and 12 deletions Side-by-side Diff
drivers/pci/pci_rom.c
@@ -129,14 +129,26 @@
 	return 0;
 }
  
-int pci_rom_load(struct pci_rom_header *rom_header,
-		 struct pci_rom_header **ram_headerp)
+/**
+ * pci_rom_load() - Load a ROM image and return a pointer to it
+ *
+ * @rom_header:		Pointer to ROM image
+ * @ram_headerp:	Returns a pointer to the image in RAM
+ * @allocedp:		Returns true if @ram_headerp was allocated and needs
+ *			to be freed
+ * @return 0 if OK, -ve on error. Note that @allocedp is set up regardless of
+ * the error state. Even if this function returns an error, it may have
+ * allocated memory.
+ */
+static int pci_rom_load(struct pci_rom_header *rom_header,
+			struct pci_rom_header **ram_headerp, bool *allocedp)
 {
 	struct pci_rom_data *rom_data;
 	unsigned int rom_size;
 	unsigned int image_size = 0;
 	void *target;
  
+	*allocedp = false;
 	do {
 		/* Get next image, until we see an x86 version */
 		rom_header = (struct pci_rom_header *)((void *)rom_header +
@@ -159,6 +171,7 @@
 	target = (void *)malloc(rom_size);
 	if (!target)
 		return -ENOMEM;
+	*allocedp = true;
 #endif
 	if (target != rom_header) {
 		ulong start = get_timer(0);
@@ -255,7 +268,7 @@
 	struct pci_child_platdata *pplat = dev_get_parent_platdata(dev);
 	struct pci_rom_header *rom, *ram;
 	int vesa_mode = -1;
-	bool emulate;
+	bool emulate, alloced;
 	int ret;
  
 	/* Only execute VGA ROMs */
  
  
@@ -272,12 +285,14 @@
 	if (ret)
 		return ret;
  
-	ret = pci_rom_load(rom, &ram);
+	ret = pci_rom_load(rom, &ram, &alloced);
 	if (ret)
-		return ret;
+		goto err;
  
-	if (!board_should_run_oprom(dev))
-		return -ENXIO;
+	if (!board_should_run_oprom(dev)) {
+		ret = -ENXIO;
+		goto err;
+	}
  
 #if defined(CONFIG_FRAMEBUFFER_SET_VESA_MODE) && \
 		defined(CONFIG_FRAMEBUFFER_VESA_MODE)
@@ -291,7 +306,8 @@
 #else
 		if (!(exec_method & PCI_ROM_ALLOW_FALLBACK)) {
 			printf("BIOS native execution is only available on x86\n");
-			return -ENOSYS;
+			ret = -ENOSYS;
+			goto err;
 		}
 		emulate = true;
 #endif
@@ -301,7 +317,8 @@
 #else
 		if (!(exec_method & PCI_ROM_ALLOW_FALLBACK)) {
 			printf("BIOS emulation not available - see CONFIG_BIOSEMU\n");
-			return -ENOSYS;
+			ret = -ENOSYS;
+			goto err;
 		}
 		emulate = false;
 #endif
  
@@ -313,12 +330,12 @@
  
 		ret = biosemu_setup(dm_pci_get_bdf(dev), &info);
 		if (ret)
-			return ret;
+			goto err;
 		biosemu_set_interrupt_handler(0x15, int15_handler);
 		ret = biosemu_run(dm_pci_get_bdf(dev), (uchar *)ram, 1 << 16,
 				  info, true, vesa_mode, &mode_info);
 		if (ret)
-			return ret;
+			goto err;
 #endif
 	} else {
 #ifdef CONFIG_X86
  
@@ -329,7 +346,11 @@
 #endif
 	}
 	debug("Final vesa mode %#x\n", mode_info.video_mode);
+	ret = 0;
  
-	return 0;
+err:
+	if (alloced)
+		free(ram);
+	return ret;
 }
...	...	@@ -129,14 +129,26 @@
129	129	return 0;
130	130	}
131	131
132		-int pci_rom_load(struct pci_rom_header *rom_header,
133		- struct pci_rom_header **ram_headerp)
	132	+/**
	133	+ * pci_rom_load() - Load a ROM image and return a pointer to it
	134	+ *
	135	+ * @rom_header: Pointer to ROM image
	136	+ * @ram_headerp: Returns a pointer to the image in RAM
	137	+ * @allocedp: Returns true if @ram_headerp was allocated and needs
	138	+ * to be freed
	139	+ * @return 0 if OK, -ve on error. Note that @allocedp is set up regardless of
	140	+ * the error state. Even if this function returns an error, it may have
	141	+ * allocated memory.
	142	+ */
	143	+static int pci_rom_load(struct pci_rom_header *rom_header,
	144	+ struct pci_rom_header *ram_headerp, bool allocedp)
134	145	{
135	146	struct pci_rom_data *rom_data;
136	147	unsigned int rom_size;
137	148	unsigned int image_size = 0;
138	149	void *target;
139	150
	151	+ *allocedp = false;
140	152	do {
141	153	/* Get next image, until we see an x86 version */
142	154	rom_header = (struct pci_rom_header )((void )rom_header +
...	...	@@ -159,6 +171,7 @@
159	171	target = (void *)malloc(rom_size);
160	172	if (!target)
161	173	return -ENOMEM;
	174	+ *allocedp = true;
162	175	#endif
163	176	if (target != rom_header) {
164	177	ulong start = get_timer(0);
...	...	@@ -255,7 +268,7 @@
255	268	struct pci_child_platdata *pplat = dev_get_parent_platdata(dev);
256	269	struct pci_rom_header rom, ram;
257	270	int vesa_mode = -1;
258		- bool emulate;
	271	+ bool emulate, alloced;
259	272	int ret;
260	273
261	274	/* Only execute VGA ROMs */
262	275
263	276
...	...	@@ -272,12 +285,14 @@
272	285	if (ret)
273	286	return ret;
274	287
275		- ret = pci_rom_load(rom, &ram);
	288	+ ret = pci_rom_load(rom, &ram, &alloced);
276	289	if (ret)
277		- return ret;
	290	+ goto err;
278	291
279		- if (!board_should_run_oprom(dev))
280		- return -ENXIO;
	292	+ if (!board_should_run_oprom(dev)) {
	293	+ ret = -ENXIO;
	294	+ goto err;
	295	+ }
281	296
282	297	#if defined(CONFIG_FRAMEBUFFER_SET_VESA_MODE) && \
283	298	defined(CONFIG_FRAMEBUFFER_VESA_MODE)
...	...	@@ -291,7 +306,8 @@
291	306	#else
292	307	if (!(exec_method & PCI_ROM_ALLOW_FALLBACK)) {
293	308	printf("BIOS native execution is only available on x86\n");
294		- return -ENOSYS;
	309	+ ret = -ENOSYS;
	310	+ goto err;
295	311	}
296	312	emulate = true;
297	313	#endif
...	...	@@ -301,7 +317,8 @@
301	317	#else
302	318	if (!(exec_method & PCI_ROM_ALLOW_FALLBACK)) {
303	319	printf("BIOS emulation not available - see CONFIG_BIOSEMU\n");
304		- return -ENOSYS;
	320	+ ret = -ENOSYS;
	321	+ goto err;
305	322	}
306	323	emulate = false;
307	324	#endif
308	325
...	...	@@ -313,12 +330,12 @@
313	330
314	331	ret = biosemu_setup(dm_pci_get_bdf(dev), &info);
315	332	if (ret)
316		- return ret;
	333	+ goto err;
317	334	biosemu_set_interrupt_handler(0x15, int15_handler);
318	335	ret = biosemu_run(dm_pci_get_bdf(dev), (uchar *)ram, 1 << 16,
319	336	info, true, vesa_mode, &mode_info);
320	337	if (ret)
321		- return ret;
	338	+ goto err;
322	339	#endif
323	340	} else {
324	341	#ifdef CONFIG_X86
325	342
...	...	@@ -329,7 +346,11 @@
329	346	#endif
330	347	}
331	348	debug("Final vesa mode %#x\n", mode_info.video_mode);
	349	+ ret = 0;
332	350
333		- return 0;
	351	+err:
	352	+ if (alloced)
	353	+ free(ram);
	354	+ return ret;
334	355	}