NULL pointer in imx-sdma.c with from context arg in reject_firmware_nowait

[Xavi Drudis Ferran]

El Mon, Apr 16, 2018 at 03:46:54AM +0200, Denis 'GNUtoo' Carikli deia:
> On Sat, 14 Apr 2018 19:23:44 +0200
> Xavi Drudis Ferran <xdrudis at tinet.cat> wrote:
> [...]
> > in drivers/dma/imx-sdma.c
> I didn't look into the details of what linux-libre changes inside this
> driver but the unmodified mainline driver is supposed to work without a
> firmware. So just making the request_firmware replacement return that
> it has no firmware should work.
>

It does work when the first parameter of sdma_load_firmware (called
fw) is null and the second (called context and copied to sdma) points
to a sane struct but not when both are null.  In linux-libre 4.16
firmware.c we pass a null in the 2nd parameter too, and that
calls dev_info(NULL, ... ) which is not nice.  I think in
previous versions, sdma_load_firmware didn't even get called.

drivers/dma/imx-sdma.c

static void sdma_load_firmware(const struct firmware *fw, void *context)
{
	struct sdma_engine *sdma = context;
	const struct sdma_firmware_header *header;
	const struct sdma_script_start_addrs *addr;
	unsigned short *ram_code;

	if (!fw) {
		dev_info(sdma->dev, "external firmware not found, using ROM firmware\n");
		/* In this case we just use the ROM firmware. */
		return;
	}
[...]

original include/linux/firmware.h

 reject_firmware_nowait(struct module *module, int uevent,
  		       const char *name, struct device *device,
  		       gfp_t gfp, void *context,
  		       void (*cont)(const struct firmware *fw,
  				    void *context))
  {
  	report_missing_free_firmware(dev_name(device), NULL);
  	/* We assume NONFREE_FIRMWARE will not be found; how could it?  */
  	return request_firmware_nowait(module, uevent, NONFREE_FIRMWARE,
 				       device, gfp, NULL, cont);
  }
 

request_firmware_nowait (see drivers/base/firmware_class.c) will call
request_firmware_work_func, which will eventually call
sdma_load_firmware(fw, context), with fw null because the file named
by NONFREE_FIRMWARE won't exist, but context also a NULL coming from 
the second last parameter in the call above.

I could fix this null pointer dereference with the patch below, I'm just
wondering if the NULL was there on purpose and now I'm making
something worse. I see the point in linux-libre 4.16 was to call the
cont function to help some drivers work. But is it necessary somewhere
to hardcode context to NULL ?

diff -rBbuw orig/linux-4.16/include/linux/firmware.h linux-4.16/include/linux/firmware.h
--- orig/linux-4.16/include/linux/firmware.h	2018-04-02 03:16:13.000000000 +0200
+++ linux-4.16/include/linux/firmware.h	2018-04-14 18:37:32.930013403 +0200
@@ -152,8 +152,9 @@
 	report_missing_free_firmware(dev_name(device), NULL);
 	/* We assume NONFREE_FIRMWARE will not be found; how could it?  */
 	return request_firmware_nowait(module, uevent, NONFREE_FIRMWARE,
-				       device, gfp, NULL, cont);
+				       device, gfp, context, cont);
 }
+
 static inline int
 maybe_reject_firmware_nowait(struct module *module, int uevent,
 			     const char *name, struct device *device,