[PATCH] firmware: Fix dereferencing NULL pointer of fw_priv->buf->fw_id

[PATCH] firmware: Fix dereferencing NULL pointer of fw_priv->buf->fw_id

[Steven Rostedt]

Commit e0fd9b1d9c44c0 ("firmware: use const for remaining firmware
names") converted struct firware_buf fw_id from a string within the
structure to a pointer to a constant string.

Unfortunately, this caused the following BUG:

 BUG: unable to handle kernel NULL pointer dereference at 00000000000000c0
 IP: [<ffffffff81823d20>] firmware_uevent+0x2f/0xaa
 PGD d325c067 PUD d3fc8067 PMD 0 
 Oops: 0000 [#1] SMP 
 Modules linked in: ip6t_REJECT nf_reject_ipv6 xt_tcpudp nf_conntrack_ipv6 nf_defrag_ipv6 xt_state ip6table_filter ip6_tables x_tables microcode r8169
 CPU: 0 PID: 1019 Comm: NetworkManager Not tainted 4.1.0-rc4-test+ #48
 Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 02/22/2014
 task: ffff8800d35fc000 ti: ffff8800d363c000 task.ti: ffff8800d363c000
 RIP: 0010:[<ffffffff81823d20>]  [<ffffffff81823d20>] firmware_uevent+0x2f/0xaa
 RSP: 0018:ffff8800d363f648  EFLAGS: 00010202
 RAX: 0000000000000000 RBX: ffff880211d17008 RCX: 0000000000000003
 RDX: 0000000000000000 RSI: ffffffff828614b4 RDI: ffff880211d17008
 RBP: ffff8800d363f668 R08: 00000000fffffffd R09: 0000000000000202
 R10: ffffffffffffffff R11: 000000000000005f R12: 00000000fffffff4
 R13: ffff880212484010 R14: ffff880212484010 R15: ffffffff823b88e0
 FS:  00007f849a3107e0(0000) GS:ffff88021dc00000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 00000000000000c0 CR3: 00000000d31a4000 CR4: 00000000001406f0
 Stack:
  00000000fffffffd ffff880212484020 0000000000000000 ffff880211d17008
  ffff8800d363f6b8 ffffffff8180ecad ffff8800d363f6b8 ffffffff814f6a9e
  ffffffff82867bfb 0000000000000002 0000000000000000 ffff880211d17008
 Call Trace:
  [<ffffffff8180ecad>] dev_uevent+0x273/0x374
  [<ffffffff814f6a9e>] ? kobject_get_path+0xbe/0x126
  [<ffffffff814f82e4>] kobject_uevent_env+0x3af/0x80b
  [<ffffffff814f8752>] kobject_uevent+0x12/0x1b
  [<ffffffff8180cc36>] device_del+0x29f/0x2e6
  [<ffffffff81825cae>] _request_firmware+0xa2c/0xd03
  [<ffffffff8182602e>] request_firmware+0x15/0x1e
  [<ffffffffa000da96>] rtl_open+0x417/0xaf5 [r8169]
  [<ffffffff81ee0666>] __dev_open+0x110/0x18e
  [<ffffffff81ee03e2>] __dev_change_flags+0xec/0x1cd
  [<ffffffff81ee04ed>] dev_change_flags+0x2a/0x75
  [<ffffffff81ef2050>] do_setlink+0x3c2/0x9a6^M
  [<ffffffff810a106f>] ? mark_lock+0x34/0x363
  [<ffffffff81ef275d>] rtnl_setlink+0x129/0x13e
  [<ffffffff81f2dca4>] ? __netlink_ns_capable+0x4b/0x72
  [<ffffffff81ef4585>] rtnetlink_rcv_msg+0x241/0x257
  [<ffffffff81ef2ef9>] ? rtnl_lock+0x19/0x22
  [<ffffffff81ef4344>] ? __rtnl_unlock+0x20/0x20
  [<ffffffff81f30775>] netlink_rcv_skb+0x72/0xf0
  [<ffffffff81ef2f71>] rtnetlink_rcv+0x2f/0x44^M
  [<ffffffff81f303f8>] netlink_unicast+0x160/0x24f
  [<ffffffff81f30bcc>] netlink_sendmsg+0x3d9/0x42e
  [<ffffffff81ebb5f8>] sock_sendmsg_nosec+0x16/0x36
  [<ffffffff81ebdd33>] ___sys_sendmsg+0x19d/0x229
  [<ffffffff81ebb5f8>] ? sock_sendmsg_nosec+0x16/0x36
  [<ffffffff81ebdee8>] ? SYSC_sendto+0x129/0x146
  [<ffffffff8100adf7>] ? emulate_vsyscall+0x313/0x437
  [<ffffffff811947ea>] ? __fget_light+0x5c/0xbc
  [<ffffffff81ebe856>] __sys_sendmsg+0x52/0x7e
  [<ffffffff81ebe892>] SyS_sendmsg+0x10/0x19
  [<ffffffff82190617>] system_call_fastpath+0x12/0x6f

This was caused by firware_uevent() that dereferenced buf:

 fw_priv->buf->fw_id.

I guess when fw_id was part of buf, if buf was NULL, this pointer too
was NULL. But now that it is a pointer to a string and not just part of
buf, it is a dereference of NULL when buf is NULL causing the above
crash.

Add a check to see if buf is NULL before dereferencing it.

Fixes: e0fd9b1d9c44 ("firmware: use const for remaining firmware names")
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---

Now it seems that I have workarounds to test against v4.2-rc1 after
fixing/finding 3 bugs!

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 9c4288362a8e..071f22a630ce 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -567,7 +567,8 @@ static int firmware_uevent(struct device *dev, struct kobj_uevent_env *env)
 {
 	struct firmware_priv *fw_priv = to_firmware_priv(dev);
 
-	if (add_uevent_var(env, "FIRMWARE=%s", fw_priv->buf->fw_id))
+	if (add_uevent_var(env, "FIRMWARE=%s",
+			   fw_priv->buf ? fw_priv->buf->fw_id : NULL))
 		return -ENOMEM;
 	if (add_uevent_var(env, "TIMEOUT=%i", loading_timeout))
 		return -ENOMEM;

Re: [PATCH] firmware: Fix dereferencing NULL pointer of fw_priv->buf->fw_id

[Linus Torvalds]

On Sun, Jul 12, 2015 at 11:49 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
>
> Unfortunately, this caused the following BUG:

I'm hoping you didn't waste too much time on this.

Because check commit 6f957724b94c, which fixes the NULL pointer
dereference _and_ the lack of proper locking.

             Linus

Re: [PATCH] firmware: Fix dereferencing NULL pointer of fw_priv->buf->fw_id

[Steven Rostedt]

On Sun, 12 Jul 2015 13:50:19 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Sun, Jul 12, 2015 at 11:49 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > Unfortunately, this caused the following BUG:
> 
> I'm hoping you didn't waste too much time on this.

Well, not that much. I was able to automate the bisects. But it did
cause me to waste time in my own testing, which needs to be restarted
because this bug caused my tests to fail.

> 
> Because check commit 6f957724b94c, which fixes the NULL pointer
> dereference _and_ the lack of proper locking.
> 

Thanks, I'll take note.

-- Steve