linux/drivers/base/firmware_class.c, branch v4.5

firmware: actually return NULL on failed request_firmware_nowait()

2016-01-07T20:45:34Z

The kerneldoc for request_firmware_nowait() says that it may call the provided cont() callback with @fw == NULL, if the firmware request fails. However, this is not the case when called with an empty string (""). This case is short-circuited by the 'name[0] == '\0'' check introduced in commit 471b095dfe0d ("firmware_class: make sure fw requests contain a name"), so _request_firmware() never gets to set the fw to NULL. Noticed while using the new 'trigger_async_request' testing hook: # printf '\x00' > /sys/devices/virtual/misc/test_firmware/trigger_async_request [10553.726178] test_firmware: loading '' [10553.729859] test_firmware: loaded: 995209091 # printf '\x00' > /sys/devices/virtual/misc/test_firmware/trigger_async_request [10733.676184] test_firmware: loading '' [10733.679855] Unable to handle kernel NULL pointer dereference at virtual address 00000004 [10733.687951] pgd = ec188000 [10733.690655] [00000004] *pgd=00000000 [10733.694240] Internal error: Oops: 5 [#1] SMP ARM [10733.698847] Modules linked in: btmrvl_sdio btmrvl bluetooth sbs_battery nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_filter ip6_tables asix usbnet mwifiex_sdio mwifiex cfg80211 jitterentropy_rng drbg joydev snd_seq_midi snd_seq_midi_event snd_rawmidi snd_seq snd_seq_device ppp_async ppp_generic slhc tun [10733.725670] CPU: 0 PID: 6600 Comm: bash Not tainted 4.4.0-rc4-00351-g63d0877 #178 [10733.733137] Hardware name: Rockchip (Device Tree) [10733.737831] task: ed24f6c0 ti: ee322000 task.ti: ee322000 [10733.743222] PC is at do_raw_spin_lock+0x18/0x1a0 [10733.747831] LR is at _raw_spin_lock+0x18/0x1c [10733.752180] pc : [] lr : [] psr: a00d0013 [10733.752180] sp : ee323df8 ip : ee323e20 fp : ee323e1c [10733.763634] r10: 00000051 r9 : b6f18000 r8 : ee323f80 [10733.768847] r7 : c089cebc r6 : 00000001 r5 : 00000000 r4 : ec0e6000 [10733.775360] r3 : dead4ead r2 : c06bd140 r1 : eef913b4 r0 : 00000000 [10733.781874] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [10733.788995] Control: 10c5387d Table: 2c18806a DAC: 00000051 [10733.794728] Process bash (pid: 6600, stack limit = 0xee322218) [10733.800549] Stack: (0xee323df8 to 0xee324000) [10733.804896] 3de0: ec0e6000 00000000 [10733.813059] 3e00: 00000001 c089cebc ee323f80 b6f18000 ee323e2c ee323e20 c054c204 c0065394 [10733.821221] 3e20: ee323e44 ee323e30 c02fec60 c054c1f8 ec0e7ec0 ec3fcfc0 ee323e5c ee323e48 [10733.829384] 3e40: c02fed08 c02fec48 c07dbf74 eeb05a00 ee323e8c ee323e60 c0253828 c02fecac [10733.837547] 3e60: 00000001 c0116950 ee323eac ee323e78 00000001 ec3fce00 ed2d9700 ed2d970c [10733.845710] 3e80: ee323e9c ee323e90 c02e873c c02537d4 ee323eac ee323ea0 c017bd40 c02e8720 [10733.853873] 3ea0: ee323ee4 ee323eb0 c017b250 c017bd00 00000000 00000000 f3e47a54 ec128b00 [10733.862035] 3ec0: c017b10c ee323f80 00000001 c000f504 ee322000 00000000 ee323f4c ee323ee8 [10733.870197] 3ee0: c011b71c c017b118 ee323fb0 c011bc90 becfa8d9 00000001 ec128b00 00000001 [10733.878359] 3f00: b6f18000 ee323f80 ee323f4c ee323f18 c011bc90 c0063950 ee323f3c ee323f28 [10733.886522] 3f20: c0063950 c0549138 00000001 ec128b00 00000001 ec128b00 b6f18000 ee323f80 [10733.894684] 3f40: ee323f7c ee323f50 c011bed8 c011b6ec c0135fb8 c0135f24 ec128b00 ec128b00 [10733.902847] 3f60: 00000001 b6f18000 c000f504 ee322000 ee323fa4 ee323f80 c011c664 c011be24 [10733.911009] 3f80: 00000000 00000000 00000001 b6f18000 b6e79be0 00000004 00000000 ee323fa8 [10733.919172] 3fa0: c000f340 c011c618 00000001 b6f18000 00000001 b6f18000 00000001 00000000 [10733.927334] 3fc0: 00000001 b6f18000 b6e79be0 00000004 00000001 00000001 8068a3f1 b6e79c84 [10733.935496] 3fe0: 00000000 becfa7dc b6de194d b6e20246 400d0030 00000001 7a4536e8 49bda390 [10733.943664] [] (do_raw_spin_lock) from [] (_raw_spin_lock+0x18/0x1c) [10733.951743] [] (_raw_spin_lock) from [] (fw_free_buf+0x24/0x64) [10733.959388] [] (fw_free_buf) from [] (release_firmware+0x68/0x74) [10733.967207] [] (release_firmware) from [] (trigger_async_request_store+0x60/0x124) [10733.976501] [] (trigger_async_request_store) from [] (dev_attr_store+0x28/0x34) [10733.985533] [] (dev_attr_store) from [] (sysfs_kf_write+0x4c/0x58) [10733.993437] [] (sysfs_kf_write) from [] (kernfs_fop_write+0x144/0x1a8) [10734.001689] [] (kernfs_fop_write) from [] (__vfs_write+0x3c/0xe4) After this patch: # printf '\x00' > /sys/devices/virtual/misc/test_firmware/trigger_async_request [ 32.126322] test_firmware: loading '' [ 32.129995] test_firmware: failed to async load firmware -bash: printf: write error: No such device Fixes: 471b095dfe0d ("firmware_class: make sure fw requests contain a name") Signed-off-by: Brian Norris Acked-by: Ming Lei Acked-by: Kees Cook Signed-off-by: Shuah Khan

firmware: fix wrong memory deallocation in fw_add_devm_name()

2015-08-05T22:18:26Z

Device resource data allocated with devres_alloc() must be deallocated by devres_free(). Signed-off-by: Vladimir Zapolskiy Acked-by: Ming Lei Signed-off-by: Greg Kroah-Hartman

Fix firmware loader uevent buffer NULL pointer dereference

2015-07-09T18:20:01Z

The firmware class uevent function accessed the "fw_priv->buf" buffer without the proper locking and testing for NULL. This is an old bug (looks like it goes back to 2012 and commit 1244691c73b2: "firmware loader: introduce firmware_buf"), but for some reason it's triggering only now in 4.2-rc1. Shuah Khan is trying to bisect what it is that causes this to trigger more easily, but in the meantime let's just fix the bug since others are hitting it too (at least Ingo reports having seen it as well). Reported-and-tested-by: Shuah Khan Acked-by: Ming Lei Cc: Greg Kroah-Hartman Signed-off-by: Linus Torvalds

firmware: add missing kfree for work on async call

2015-06-01T01:22:34Z

The recent fix to use kstrdup_const() failed to add a kfree upon failure of name allocation... Cc: Ming Lei Cc: Seth Forshee Cc: Kyle McMartin Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman

firmware: use const for remaining firmware names

2015-05-24T19:36:34Z

We currently use flexible arrays with a char at the end for the remaining internal firmware name uses. There are two limitations with the way we use this. Since we're using a flexible array for a string on the struct if we wanted to use two strings it means we'd have a disjoint means of handling the strings, one using the flexible array, and another a char * pointer. We're also currently not using 'const' for the string. We wish to later extend some firmware data structures with other string/char pointers, but we also want to be very pedantic about const usage. Since we're going to change things to use 'const' we might as well also address unified way to use multiple strings on the structs. Replace the flexible array practice for strings with kstrdup_const() and kfree_const(), this will avoid allocations when the vmlinux .rodata is used, and just allocate a new proper string for us when needed. This also means we can simplify the struct allocations by removing the string length from the allocation size computation, which would otherwise get even more complicated when supporting multiple strings. Cc: Linus Torvalds Cc: Rusty Russell Cc: David Howells Cc: Ming Lei Cc: Seth Forshee Cc: Kyle McMartin Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman

firmware: fix possible use after free on name on asynchronous request

2015-05-24T19:36:34Z

Asynchronous firmware loading copies the pointer to the name passed as an argument only to be scheduled later and used. This behaviour works well for synchronous calling but in asynchronous mode there's a chance the caller could immediately free the passed string after making the asynchronous call. This could trigger a use after free having the kernel look on disk for arbitrary file names. In order to force-test the issue you can use a test-driver designed to illustrate this issue on github [0], use the next-20150505-fix-use-after-free branch. With this patch applied you get: [ 283.512445] firmware name: test_module_stuff.bin [ 287.514020] firmware name: test_module_stuff.bin [ 287.532489] firmware found Without this patch applied you can end up with something such as: [ 135.624216] firmware name: \xffffff80BJ [ 135.624249] platform fake-dev.0: Direct firmware load for \xffffff80Bi failed with error -2 [ 135.624252] No firmware found [ 135.624252] firmware found Unfortunatley in the worst and most common case however you can typically crash your system with a page fault by trying to free something which you cannot, and/or a NULL pointer dereference [1]. The fix and issue using schedule_work() for asynchronous runs is generalized in the following SmPL grammar patch, when applied to next-20150505 only the firmware_class code is affected. This grammar patch can and should further be generalized to vet for for other kernel asynchronous mechanisms. @ calls_schedule_work @ type T; T *priv_work; identifier func, work_func; identifier work; identifier priv_name, name; expression gfp; @@ func(..., const char *name, ...) { ... priv_work = kzalloc(sizeof(T), gfp); ... - priv_work->priv_name = name; + priv_work->priv_name = kstrdup_const(name, gfp); ... (... when any if (...) { ... + kfree_const(priv_work->priv_name); kfree(priv_work); ... } ) ... when any INIT_WORK(&priv_work->work, work_func); ... schedule_work(&priv_work->work); ... } @ the_work_func depends on calls_schedule_work @ type calls_schedule_work.T; T *priv_work; identifier calls_schedule_work.work_func; identifier calls_schedule_work.priv_name; identifier calls_schedule_work.work; identifier some_work; @@ work_func(...) { ... priv_work = container_of(some_work, T, work); ... + kfree_const(priv_work->priv_name); kfree(priv_work); ... } [0] https://github.com/mcgrof/fake-firmware-test.git [1] The following kernel ring buffer splat: firmware name: test_module_stuff.bin firmware name: firmware found general protection fault: 0000 [#1] SMP Modules linked in: test(O) <...etc-it-does-not-matter> drm sr_mod cdrom xhci_pci xhci_hcd rtsx_pci mfd_core video button sg CPU: 3 PID: 87 Comm: kworker/3:2 Tainted: G O 4.0.0-00010-g22b5bb0-dirty #176 Hardware name: LENOVO 20AW000LUS/20AW000LUS, BIOS GLET43WW (1.18 ) 12/04/2013 Workqueue: events request_firmware_work_func task: ffff8800c7f8e290 ti: ffff8800c7f94000 task.ti: ffff8800c7f94000 RIP: 0010:[] [] fw_free_buf+0xc/0x40 RSP: 0000:ffff8800c7f97d78 EFLAGS: 00010286 RAX: ffffffff81ae3700 RBX: ffffffff816d1181 RCX: 0000000000000006 RDX: 0001ee850ff68500 RSI: 0000000000000246 RDI: c35d5f415e415d41 RBP: ffff8800c7f97d88 R08: 000000000000000a R09: 0000000000000000 R10: 0000000000000358 R11: ffff8800c7f97a7e R12: ffff8800c7ec1e80 R13: ffff88021e2d4cc0 R14: ffff88021e2dff00 R15: 00000000000000c0 FS: 0000000000000000(0000) GS:ffff88021e2c0000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00000000034b8cd8 CR3: 000000021073c000 CR4: 00000000001407e0 Stack: ffffffff816d1181 ffff8800c7ec1e80 ffff8800c7f97da8 ffffffff814a58f8 000000000000000a ffffffff816d1181 ffff8800c7f97dc8 ffffffffa047002c ffff88021e2dff00 ffff8802116ac1c0 ffff8800c7f97df8 ffffffff814a65fe Call Trace: [] ? __schedule+0x361/0x940 [] release_firmware+0x58/0x80 [] ? __schedule+0x361/0x940 [] test_mod_cb+0x2c/0x43 [test] [] request_firmware_work_func+0x5e/0x80 [] ? __schedule+0x361/0x940 [] process_one_work+0x14a/0x3f0 [] worker_thread+0x121/0x460 [] ? rescuer_thread+0x310/0x310 [] kthread+0xc9/0xe0 [] ? kthread_create_on_node+0x180/0x180 [] ret_from_fork+0x58/0x90 [] ? kthread_create_on_node+0x180/0x180 Code: c7 c6 dd ad a3 81 48 c7 c7 20 97 ce 81 31 c0 e8 0b b2 ed ff e9 78 ff ff ff 66 0f 1f 44 00 00 0f 1f 44 00 00 55 48 89 e5 41 54 53 <4c> 8b 67 38 48 89 fb 4c 89 e7 e8 85 f7 22 00 f0 83 2b 01 74 0f RIP [] fw_free_buf+0xc/0x40 RSP ---[ end trace 4e62c56a58d0eac1 ]--- BUG: unable to handle kernel paging request at ffffffffffffffd8 IP: [] kthread_data+0x10/0x20 PGD 1c13067 PUD 1c15067 PMD 0 Oops: 0000 [#2] SMP Modules linked in: test(O) <...etc-it-does-not-matter> drm sr_mod cdrom xhci_pci xhci_hcd rtsx_pci mfd_core video button sg CPU: 3 PID: 87 Comm: kworker/3:2 Tainted: G D O 4.0.0-00010-g22b5bb0-dirty #176 Hardware name: LENOVO 20AW000LUS/20AW000LUS, BIOS GLET43WW (1.18 ) 12/04/2013 task: ffff8800c7f8e290 ti: ffff8800c7f94000 task.ti: ffff8800c7f94000 RIP: 0010:[] [] kthread_data+0x10/0x20 RSP: 0018:ffff8800c7f97b18 EFLAGS: 00010096 RAX: 0000000000000000 RBX: 0000000000000003 RCX: 000000000000000d RDX: 0000000000000003 RSI: 0000000000000003 RDI: ffff8800c7f8e290 RBP: ffff8800c7f97b18 R08: 000000000000bc00 R09: 0000000000007e76 R10: 0000000000000001 R11: 000000000000002f R12: ffff8800c7f8e290 R13: 00000000000154c0 R14: 0000000000000003 R15: 0000000000000000 FS: 0000000000000000(0000) GS:ffff88021e2c0000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000028 CR3: 0000000210675000 CR4: 00000000001407e0 Stack: ffff8800c7f97b38 ffffffff8108dcd5 ffff8800c7f97b38 ffff88021e2d54c0 ffff8800c7f97b88 ffffffff816d1500 ffff880213d42368 ffff8800c7f8e290 ffff8800c7f97b88 ffff8800c7f97fd8 ffff8800c7f8e710 0000000000000246 Call Trace: [] wq_worker_sleeping+0x15/0xa0 [] __schedule+0x6e0/0x940 [] schedule+0x37/0x90 [] do_exit+0x6bc/0xb40 [] oops_end+0x9f/0xe0 [] die+0x4b/0x70 [] do_general_protection+0xe2/0x170 [] general_protection+0x28/0x30 [] ? __schedule+0x361/0x940 [] ? fw_free_buf+0xc/0x40 [] ? __schedule+0x361/0x940 [] release_firmware+0x58/0x80 [] ? __schedule+0x361/0x940 [] test_mod_cb+0x2c/0x43 [test] [] request_firmware_work_func+0x5e/0x80 [] ? __schedule+0x361/0x940 [] process_one_work+0x14a/0x3f0 [] worker_thread+0x121/0x460 [] ? rescuer_thread+0x310/0x310 [] kthread+0xc9/0xe0 [] ? kthread_create_on_node+0x180/0x180 [] ret_from_fork+0x58/0x90 [] ? kthread_create_on_node+0x180/0x180 Code: 00 48 89 e5 5d 48 8b 40 c8 48 c1 e8 02 83 e0 01 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 8b 87 30 05 00 00 55 48 89 e5 <48> 8b 40 d8 5d c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 RIP [] kthread_data+0x10/0x20 RSP CR2: ffffffffffffffd8 ---[ end trace 4e62c56a58d0eac2 ]--- Fixing recursive fault but reboot is needed! Cc: Linus Torvalds Cc: Rusty Russell Cc: David Howells Cc: Ming Lei Cc: Seth Forshee Cc: Kyle McMartin Generated-by: Coccinelle SmPL Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman

firmware: check for file truncation on direct firmware loading

2015-05-24T19:36:34Z

When direct firmware loading is used we iterate over a list of possible firmware paths and concatenate the desired firmware name with each path and look for the file there. Should the passed firmware name be too long we end up truncating the file we want to look for, the search however is still done. Add a check for truncation instead of looking for a truncated firmware filename. Cc: Linus Torvalds Cc: Ming Lei Cc: Rusty Russell Cc: David Howells Cc: Kyle McMartin Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman

firmware: fix __getname() missing failure check

2015-05-24T19:36:34Z

The request_firmware*() APIs uses __getname() to iterate over the list of paths possible for firmware to be found, the code however never checked for failure on __getname(). Although *very unlikely*, this can still happen. Add the missing check. There is still no checks on the concatenation of the path and filename passed, that requires a bit more work and subsequent patches address this. The commit that introduced this is abb139e7 ("firmware: teach the kernel to load firmware files directly from the filesystem"). mcgrof@ergon ~/linux (git::firmware-fixes) $ git describe --contains abb139e7 v3.7-rc1~120 Cc: Linus Torvalds Cc: Ming Lei Cc: Rusty Russell Cc: David Howells Cc: Kyle McMartin Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman

drivers: base: fw: fix ret value when loading fw

2015-03-25T13:49:10Z

When using the user mode helper to load firmwares the function _request_firmware gets a positive return value from fw_load_from_user_helper and because of this the firmware buffer is not assigned. This happens only when the return value is zero. This patch fixes this problem in _request_firmware_load. When the completion is ready the return value is set to zero. Signed-off-by: Zahari Doychev Cc: Ming Lei Signed-off-by: Greg Kroah-Hartman

firmware: Avoid manual device_create_file() calls

2015-03-25T13:41:48Z

Use the static attribute groups assigned to the device instead of manual device_create_file() & co calls. It simplifies the code and can avoid possible races, too. Signed-off-by: Takashi Iwai Signed-off-by: Greg Kroah-Hartman