<feed xmlns='http://www.w3.org/2005/Atom'>
<title>linux-stable.git/drivers/base/firmware_loader, branch linux-4.19.y</title>
<subtitle>Linux kernel stable tree</subtitle>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/'/>
<entry>
<title>firmware_loader: Block path traversal</title>
<updated>2024-11-08T15:19:09+00:00</updated>
<author>
<name>Jann Horn</name>
<email>jannh@google.com</email>
</author>
<published>2024-08-27T23:45:48+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=d1768e5535d3ded59f888637016e6f821f4e069f'/>
<id>d1768e5535d3ded59f888637016e6f821f4e069f</id>
<content type='text'>
commit f0e5311aa8022107d63c54e2f03684ec097d1394 upstream.

Most firmware names are hardcoded strings, or are constructed from fairly
constrained format strings where the dynamic parts are just some hex
numbers or such.

However, there are a couple codepaths in the kernel where firmware file
names contain string components that are passed through from a device or
semi-privileged userspace; the ones I could find (not counting interfaces
that require root privileges) are:

 - lpfc_sli4_request_firmware_update() seems to construct the firmware
   filename from "ModelName", a string that was previously parsed out of
   some descriptor ("Vital Product Data") in lpfc_fill_vpd()
 - nfp_net_fw_find() seems to construct a firmware filename from a model
   name coming from nfp_hwinfo_lookup(pf-&gt;hwinfo, "nffw.partno"), which I
   think parses some descriptor that was read from the device.
   (But this case likely isn't exploitable because the format string looks
   like "netronome/nic_%s", and there shouldn't be any *folders* starting
   with "netronome/nic_". The previous case was different because there,
   the "%s" is *at the start* of the format string.)
 - module_flash_fw_schedule() is reachable from the
   ETHTOOL_MSG_MODULE_FW_FLASH_ACT netlink command, which is marked as
   GENL_UNS_ADMIN_PERM (meaning CAP_NET_ADMIN inside a user namespace is
   enough to pass the privilege check), and takes a userspace-provided
   firmware name.
   (But I think to reach this case, you need to have CAP_NET_ADMIN over a
   network namespace that a special kind of ethernet device is mapped into,
   so I think this is not a viable attack path in practice.)

Fix it by rejecting any firmware names containing ".." path components.

For what it's worth, I went looking and haven't found any USB device
drivers that use the firmware loader dangerously.

Cc: stable@vger.kernel.org
Reviewed-by: Danilo Krummrich &lt;dakr@kernel.org&gt;
Fixes: abb139e75c2c ("firmware: teach the kernel to load firmware files directly from the filesystem")
Signed-off-by: Jann Horn &lt;jannh@google.com&gt;
Acked-by: Luis Chamberlain &lt;mcgrof@kernel.org&gt;
Link: https://lore.kernel.org/r/20240828-firmware-traversal-v3-1-c76529c63b5f@google.com
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
commit f0e5311aa8022107d63c54e2f03684ec097d1394 upstream.

Most firmware names are hardcoded strings, or are constructed from fairly
constrained format strings where the dynamic parts are just some hex
numbers or such.

However, there are a couple codepaths in the kernel where firmware file
names contain string components that are passed through from a device or
semi-privileged userspace; the ones I could find (not counting interfaces
that require root privileges) are:

 - lpfc_sli4_request_firmware_update() seems to construct the firmware
   filename from "ModelName", a string that was previously parsed out of
   some descriptor ("Vital Product Data") in lpfc_fill_vpd()
 - nfp_net_fw_find() seems to construct a firmware filename from a model
   name coming from nfp_hwinfo_lookup(pf-&gt;hwinfo, "nffw.partno"), which I
   think parses some descriptor that was read from the device.
   (But this case likely isn't exploitable because the format string looks
   like "netronome/nic_%s", and there shouldn't be any *folders* starting
   with "netronome/nic_". The previous case was different because there,
   the "%s" is *at the start* of the format string.)
 - module_flash_fw_schedule() is reachable from the
   ETHTOOL_MSG_MODULE_FW_FLASH_ACT netlink command, which is marked as
   GENL_UNS_ADMIN_PERM (meaning CAP_NET_ADMIN inside a user namespace is
   enough to pass the privilege check), and takes a userspace-provided
   firmware name.
   (But I think to reach this case, you need to have CAP_NET_ADMIN over a
   network namespace that a special kind of ethernet device is mapped into,
   so I think this is not a viable attack path in practice.)

Fix it by rejecting any firmware names containing ".." path components.

For what it's worth, I went looking and haven't found any USB device
drivers that use the firmware loader dangerously.

Cc: stable@vger.kernel.org
Reviewed-by: Danilo Krummrich &lt;dakr@kernel.org&gt;
Fixes: abb139e75c2c ("firmware: teach the kernel to load firmware files directly from the filesystem")
Signed-off-by: Jann Horn &lt;jannh@google.com&gt;
Acked-by: Luis Chamberlain &lt;mcgrof@kernel.org&gt;
Link: https://lore.kernel.org/r/20240828-firmware-traversal-v3-1-c76529c63b5f@google.com
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Revert "drivers core: Use sysfs_emit and sysfs_emit_at for show(device *...) functions"</title>
<updated>2023-10-10T19:45:00+00:00</updated>
<author>
<name>Greg Kroah-Hartman</name>
<email>gregkh@linuxfoundation.org</email>
</author>
<published>2023-10-07T11:46:47+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=ce63d45f45ae8b03f28b3329f6b6e4d072f7d2c5'/>
<id>ce63d45f45ae8b03f28b3329f6b6e4d072f7d2c5</id>
<content type='text'>
This reverts commit 3ce2cd63e8ee037644db0cbea65e6c40ab6cc178 which is
commit aa838896d87af561a33ecefea1caa4c15a68bc47 upstream.

Ben writes:
	When I looked into the referenced security issue, it seemed to only be
	exploitable through wakelock names, and in the upstream kernel only
	after commit c8377adfa781 "PM / wakeup: Show wakeup sources stats in
	sysfs" (first included in 5.4).  So I would be interested to know if
	and why a fix was needed for 4.19.

	More importantly, this backported version uniformly converts to
	sysfs_emit(), but there are 3 places sysfs_emit_at() must be used
	instead:

Reported-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
Link: https://lore.kernel.org/r/95831df76c41a53bc3e1ac8ece64915dd63763a1.camel@decadent.org.uk
Cc: Joe Perches &lt;joe@perches.com&gt;
Cc: Brennan Lamoreaux &lt;blamoreaux@vmware.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This reverts commit 3ce2cd63e8ee037644db0cbea65e6c40ab6cc178 which is
commit aa838896d87af561a33ecefea1caa4c15a68bc47 upstream.

Ben writes:
	When I looked into the referenced security issue, it seemed to only be
	exploitable through wakelock names, and in the upstream kernel only
	after commit c8377adfa781 "PM / wakeup: Show wakeup sources stats in
	sysfs" (first included in 5.4).  So I would be interested to know if
	and why a fix was needed for 4.19.

	More importantly, this backported version uniformly converts to
	sysfs_emit(), but there are 3 places sysfs_emit_at() must be used
	instead:

Reported-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
Link: https://lore.kernel.org/r/95831df76c41a53bc3e1ac8ece64915dd63763a1.camel@decadent.org.uk
Cc: Joe Perches &lt;joe@perches.com&gt;
Cc: Brennan Lamoreaux &lt;blamoreaux@vmware.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>drivers core: Use sysfs_emit and sysfs_emit_at for show(device *...) functions</title>
<updated>2023-08-11T09:45:40+00:00</updated>
<author>
<name>Joe Perches</name>
<email>joe@perches.com</email>
</author>
<published>2020-09-16T20:40:39+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=3ce2cd63e8ee037644db0cbea65e6c40ab6cc178'/>
<id>3ce2cd63e8ee037644db0cbea65e6c40ab6cc178</id>
<content type='text'>
commit aa838896d87af561a33ecefea1caa4c15a68bc47 upstream.

Convert the various sprintf fmaily calls in sysfs device show functions
to sysfs_emit and sysfs_emit_at for PAGE_SIZE buffer safety.

Done with:

$ spatch -sp-file sysfs_emit_dev.cocci --in-place --max-width=80 .

And cocci script:

$ cat sysfs_emit_dev.cocci
@@
identifier d_show;
identifier dev, attr, buf;
@@

ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
	&lt;...
	return
-	sprintf(buf,
+	sysfs_emit(buf,
	...);
	...&gt;
}

@@
identifier d_show;
identifier dev, attr, buf;
@@

ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
	&lt;...
	return
-	snprintf(buf, PAGE_SIZE,
+	sysfs_emit(buf,
	...);
	...&gt;
}

@@
identifier d_show;
identifier dev, attr, buf;
@@

ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
	&lt;...
	return
-	scnprintf(buf, PAGE_SIZE,
+	sysfs_emit(buf,
	...);
	...&gt;
}

@@
identifier d_show;
identifier dev, attr, buf;
expression chr;
@@

ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
	&lt;...
	return
-	strcpy(buf, chr);
+	sysfs_emit(buf, chr);
	...&gt;
}

@@
identifier d_show;
identifier dev, attr, buf;
identifier len;
@@

ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
	&lt;...
	len =
-	sprintf(buf,
+	sysfs_emit(buf,
	...);
	...&gt;
	return len;
}

@@
identifier d_show;
identifier dev, attr, buf;
identifier len;
@@

ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
	&lt;...
	len =
-	snprintf(buf, PAGE_SIZE,
+	sysfs_emit(buf,
	...);
	...&gt;
	return len;
}

@@
identifier d_show;
identifier dev, attr, buf;
identifier len;
@@

ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
	&lt;...
	len =
-	scnprintf(buf, PAGE_SIZE,
+	sysfs_emit(buf,
	...);
	...&gt;
	return len;
}

@@
identifier d_show;
identifier dev, attr, buf;
identifier len;
@@

ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
	&lt;...
-	len += scnprintf(buf + len, PAGE_SIZE - len,
+	len += sysfs_emit_at(buf, len,
	...);
	...&gt;
	return len;
}

@@
identifier d_show;
identifier dev, attr, buf;
expression chr;
@@

ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
	...
-	strcpy(buf, chr);
-	return strlen(buf);
+	return sysfs_emit(buf, chr);
}

Signed-off-by: Joe Perches &lt;joe@perches.com&gt;
Link: https://lore.kernel.org/r/3d033c33056d88bbe34d4ddb62afd05ee166ab9a.1600285923.git.joe@perches.com
[ Brennan : Regenerated for 4.19 to fix CVE-2022-20166 ]
Signed-off-by: Brennan Lamoreaux &lt;blamoreaux@vmware.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
commit aa838896d87af561a33ecefea1caa4c15a68bc47 upstream.

Convert the various sprintf fmaily calls in sysfs device show functions
to sysfs_emit and sysfs_emit_at for PAGE_SIZE buffer safety.

Done with:

$ spatch -sp-file sysfs_emit_dev.cocci --in-place --max-width=80 .

And cocci script:

$ cat sysfs_emit_dev.cocci
@@
identifier d_show;
identifier dev, attr, buf;
@@

ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
	&lt;...
	return
-	sprintf(buf,
+	sysfs_emit(buf,
	...);
	...&gt;
}

@@
identifier d_show;
identifier dev, attr, buf;
@@

ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
	&lt;...
	return
-	snprintf(buf, PAGE_SIZE,
+	sysfs_emit(buf,
	...);
	...&gt;
}

@@
identifier d_show;
identifier dev, attr, buf;
@@

ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
	&lt;...
	return
-	scnprintf(buf, PAGE_SIZE,
+	sysfs_emit(buf,
	...);
	...&gt;
}

@@
identifier d_show;
identifier dev, attr, buf;
expression chr;
@@

ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
	&lt;...
	return
-	strcpy(buf, chr);
+	sysfs_emit(buf, chr);
	...&gt;
}

@@
identifier d_show;
identifier dev, attr, buf;
identifier len;
@@

ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
	&lt;...
	len =
-	sprintf(buf,
+	sysfs_emit(buf,
	...);
	...&gt;
	return len;
}

@@
identifier d_show;
identifier dev, attr, buf;
identifier len;
@@

ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
	&lt;...
	len =
-	snprintf(buf, PAGE_SIZE,
+	sysfs_emit(buf,
	...);
	...&gt;
	return len;
}

@@
identifier d_show;
identifier dev, attr, buf;
identifier len;
@@

ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
	&lt;...
	len =
-	scnprintf(buf, PAGE_SIZE,
+	sysfs_emit(buf,
	...);
	...&gt;
	return len;
}

@@
identifier d_show;
identifier dev, attr, buf;
identifier len;
@@

ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
	&lt;...
-	len += scnprintf(buf + len, PAGE_SIZE - len,
+	len += sysfs_emit_at(buf, len,
	...);
	...&gt;
	return len;
}

@@
identifier d_show;
identifier dev, attr, buf;
expression chr;
@@

ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
	...
-	strcpy(buf, chr);
-	return strlen(buf);
+	return sysfs_emit(buf, chr);
}

Signed-off-by: Joe Perches &lt;joe@perches.com&gt;
Link: https://lore.kernel.org/r/3d033c33056d88bbe34d4ddb62afd05ee166ab9a.1600285923.git.joe@perches.com
[ Brennan : Regenerated for 4.19 to fix CVE-2022-20166 ]
Signed-off-by: Brennan Lamoreaux &lt;blamoreaux@vmware.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>firmware_loader: fix pre-allocated buf built-in firmware use</title>
<updated>2021-11-26T10:36:20+00:00</updated>
<author>
<name>Luis Chamberlain</name>
<email>mcgrof@kernel.org</email>
</author>
<published>2021-09-17T18:22:13+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=f2d22b13baa6d61d28788a5754a12e0c08ff213d'/>
<id>f2d22b13baa6d61d28788a5754a12e0c08ff213d</id>
<content type='text'>
[ Upstream commit f7a07f7b96033df7709042ff38e998720a3f7119 ]

The firmware_loader can be used with a pre-allocated buffer
through the use of the API calls:

  o request_firmware_into_buf()
  o request_partial_firmware_into_buf()

If the firmware was built-in and present, our current check
for if the built-in firmware fits into the pre-allocated buffer
does not return any errors, and we proceed to tell the caller
that everything worked fine. It's a lie and no firmware would
end up being copied into the pre-allocated buffer. So if the
caller trust the result it may end up writing a bunch of 0's
to a device!

Fix this by making the function that checks for the pre-allocated
buffer return non-void. Since the typical use case is when no
pre-allocated buffer is provided make this return successfully
for that case. If the built-in firmware does *not* fit into the
pre-allocated buffer size return a failure as we should have
been doing before.

I'm not aware of users of the built-in firmware using the API
calls with a pre-allocated buffer, as such I doubt this fixes
any real life issue. But you never know... perhaps some oddball
private tree might use it.

In so far as upstream is concerned this just fixes our code for
correctness.

Signed-off-by: Luis Chamberlain &lt;mcgrof@kernel.org&gt;
Link: https://lore.kernel.org/r/20210917182226.3532898-2-mcgrof@kernel.org
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
Signed-off-by: Sasha Levin &lt;sashal@kernel.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
[ Upstream commit f7a07f7b96033df7709042ff38e998720a3f7119 ]

The firmware_loader can be used with a pre-allocated buffer
through the use of the API calls:

  o request_firmware_into_buf()
  o request_partial_firmware_into_buf()

If the firmware was built-in and present, our current check
for if the built-in firmware fits into the pre-allocated buffer
does not return any errors, and we proceed to tell the caller
that everything worked fine. It's a lie and no firmware would
end up being copied into the pre-allocated buffer. So if the
caller trust the result it may end up writing a bunch of 0's
to a device!

Fix this by making the function that checks for the pre-allocated
buffer return non-void. Since the typical use case is when no
pre-allocated buffer is provided make this return successfully
for that case. If the built-in firmware does *not* fit into the
pre-allocated buffer size return a failure as we should have
been doing before.

I'm not aware of users of the built-in firmware using the API
calls with a pre-allocated buffer, as such I doubt this fixes
any real life issue. But you never know... perhaps some oddball
private tree might use it.

In so far as upstream is concerned this just fixes our code for
correctness.

Signed-off-by: Luis Chamberlain &lt;mcgrof@kernel.org&gt;
Link: https://lore.kernel.org/r/20210917182226.3532898-2-mcgrof@kernel.org
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
Signed-off-by: Sasha Levin &lt;sashal@kernel.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>firmware_loader: fix use-after-free in firmware_fallback_sysfs</title>
<updated>2021-08-12T11:19:41+00:00</updated>
<author>
<name>Anirudh Rayabharam</name>
<email>mail@anirudhrb.com</email>
</author>
<published>2021-07-28T08:51:07+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=67cf0fbcac0d42d4d4686cddc1e39f465bbfec37'/>
<id>67cf0fbcac0d42d4d4686cddc1e39f465bbfec37</id>
<content type='text'>
commit 75d95e2e39b27f733f21e6668af1c9893a97de5e upstream.

This use-after-free happens when a fw_priv object has been freed but
hasn't been removed from the pending list (pending_fw_head). The next
time fw_load_sysfs_fallback tries to insert into the list, it ends up
accessing the pending_list member of the previously freed fw_priv.

The root cause here is that all code paths that abort the fw load
don't delete it from the pending list. For example:

        _request_firmware()
          -&gt; fw_abort_batch_reqs()
              -&gt; fw_state_aborted()

To fix this, delete the fw_priv from the list in __fw_set_state() if
the new state is DONE or ABORTED. This way, all aborts will remove
the fw_priv from the list. Accordingly, remove calls to list_del_init
that were being made before calling fw_state_(aborted|done).

Also, in fw_load_sysfs_fallback, don't add the fw_priv to the pending
list if it is already aborted. Instead, just jump out and return early.

Fixes: bcfbd3523f3c ("firmware: fix a double abort case with fw_load_sysfs_fallback")
Cc: stable &lt;stable@vger.kernel.org&gt;
Reported-by: syzbot+de271708674e2093097b@syzkaller.appspotmail.com
Tested-by: syzbot+de271708674e2093097b@syzkaller.appspotmail.com
Reviewed-by: Shuah Khan &lt;skhan@linuxfoundation.org&gt;
Acked-by: Luis Chamberlain &lt;mcgrof@kernel.org&gt;
Signed-off-by: Anirudh Rayabharam &lt;mail@anirudhrb.com&gt;
Link: https://lore.kernel.org/r/20210728085107.4141-3-mail@anirudhrb.com
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
commit 75d95e2e39b27f733f21e6668af1c9893a97de5e upstream.

This use-after-free happens when a fw_priv object has been freed but
hasn't been removed from the pending list (pending_fw_head). The next
time fw_load_sysfs_fallback tries to insert into the list, it ends up
accessing the pending_list member of the previously freed fw_priv.

The root cause here is that all code paths that abort the fw load
don't delete it from the pending list. For example:

        _request_firmware()
          -&gt; fw_abort_batch_reqs()
              -&gt; fw_state_aborted()

To fix this, delete the fw_priv from the list in __fw_set_state() if
the new state is DONE or ABORTED. This way, all aborts will remove
the fw_priv from the list. Accordingly, remove calls to list_del_init
that were being made before calling fw_state_(aborted|done).

Also, in fw_load_sysfs_fallback, don't add the fw_priv to the pending
list if it is already aborted. Instead, just jump out and return early.

Fixes: bcfbd3523f3c ("firmware: fix a double abort case with fw_load_sysfs_fallback")
Cc: stable &lt;stable@vger.kernel.org&gt;
Reported-by: syzbot+de271708674e2093097b@syzkaller.appspotmail.com
Tested-by: syzbot+de271708674e2093097b@syzkaller.appspotmail.com
Reviewed-by: Shuah Khan &lt;skhan@linuxfoundation.org&gt;
Acked-by: Luis Chamberlain &lt;mcgrof@kernel.org&gt;
Signed-off-by: Anirudh Rayabharam &lt;mail@anirudhrb.com&gt;
Link: https://lore.kernel.org/r/20210728085107.4141-3-mail@anirudhrb.com
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>firmware_loader: use -ETIMEDOUT instead of -EAGAIN in fw_load_sysfs_fallback</title>
<updated>2021-08-12T11:19:41+00:00</updated>
<author>
<name>Anirudh Rayabharam</name>
<email>mail@anirudhrb.com</email>
</author>
<published>2021-07-28T08:51:06+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=ce699ac03ec0e41347363e1cf0924669f5449e34'/>
<id>ce699ac03ec0e41347363e1cf0924669f5449e34</id>
<content type='text'>
commit 0d6434e10b5377a006f6dd995c8fc5e2d82acddc upstream.

The only motivation for using -EAGAIN in commit 0542ad88fbdd81bb
("firmware loader: Fix _request_firmware_load() return val for fw load
abort") was to distinguish the error from -ENOMEM, and so there is no
real reason in keeping it. -EAGAIN is typically used to tell the
userspace to try something again and in this case re-using the sysfs
loading interface cannot be retried when a timeout happens, so the
return value is also bogus.

-ETIMEDOUT is received when the wait times out and returning that
is much more telling of what the reason for the failure was. So, just
propagate that instead of returning -EAGAIN.

Suggested-by: Luis Chamberlain &lt;mcgrof@kernel.org&gt;
Reviewed-by: Shuah Khan &lt;skhan@linuxfoundation.org&gt;
Acked-by: Luis Chamberlain &lt;mcgrof@kernel.org&gt;
Signed-off-by: Anirudh Rayabharam &lt;mail@anirudhrb.com&gt;
Cc: stable &lt;stable@vger.kernel.org&gt;
Link: https://lore.kernel.org/r/20210728085107.4141-2-mail@anirudhrb.com
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
commit 0d6434e10b5377a006f6dd995c8fc5e2d82acddc upstream.

The only motivation for using -EAGAIN in commit 0542ad88fbdd81bb
("firmware loader: Fix _request_firmware_load() return val for fw load
abort") was to distinguish the error from -ENOMEM, and so there is no
real reason in keeping it. -EAGAIN is typically used to tell the
userspace to try something again and in this case re-using the sysfs
loading interface cannot be retried when a timeout happens, so the
return value is also bogus.

-ETIMEDOUT is received when the wait times out and returning that
is much more telling of what the reason for the failure was. So, just
propagate that instead of returning -EAGAIN.

Suggested-by: Luis Chamberlain &lt;mcgrof@kernel.org&gt;
Reviewed-by: Shuah Khan &lt;skhan@linuxfoundation.org&gt;
Acked-by: Luis Chamberlain &lt;mcgrof@kernel.org&gt;
Signed-off-by: Anirudh Rayabharam &lt;mail@anirudhrb.com&gt;
Cc: stable &lt;stable@vger.kernel.org&gt;
Link: https://lore.kernel.org/r/20210728085107.4141-2-mail@anirudhrb.com
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>firmware: fix a double abort case with fw_load_sysfs_fallback</title>
<updated>2020-04-17T08:48:42+00:00</updated>
<author>
<name>Junyong Sun</name>
<email>sunjy516@gmail.com</email>
</author>
<published>2020-03-03T02:36:08+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=1a5613b54b3780b22f540c1ebe1e3e4e5149144f'/>
<id>1a5613b54b3780b22f540c1ebe1e3e4e5149144f</id>
<content type='text'>
[ Upstream commit bcfbd3523f3c6eea51a74d217a8ebc5463bcb7f4 ]

fw_sysfs_wait_timeout may return err with -ENOENT
at fw_load_sysfs_fallback and firmware is already
in abort status, no need to abort again, so skip it.

This issue is caused by concurrent situation like below:
when thread 1# wait firmware loading, thread 2# may write
-1 to abort loading and wakeup thread 1# before it timeout.
so wait_for_completion_killable_timeout of thread 1# would
return remaining time which is != 0 with fw_st-&gt;status
FW_STATUS_ABORTED.And the results would be converted into
err -ENOENT in __fw_state_wait_common and transfered to
fw_load_sysfs_fallback in thread 1#.
The -ENOENT means firmware status is already at ABORTED,
so fw_load_sysfs_fallback no need to get mutex to abort again.
-----------------------------
thread 1#,wait for loading
fw_load_sysfs_fallback
 -&gt;fw_sysfs_wait_timeout
    -&gt;__fw_state_wait_common
       -&gt;wait_for_completion_killable_timeout

in __fw_state_wait_common,
...
93    ret = wait_for_completion_killable_timeout(&amp;fw_st-&gt;completion, timeout);
94    if (ret != 0 &amp;&amp; fw_st-&gt;status == FW_STATUS_ABORTED)
95       return -ENOENT;
96    if (!ret)
97	 return -ETIMEDOUT;
98
99    return ret &lt; 0 ? ret : 0;
-----------------------------
thread 2#, write -1 to abort loading
firmware_loading_store
 -&gt;fw_load_abort
   -&gt;__fw_load_abort
     -&gt;fw_state_aborted
       -&gt;__fw_state_set
         -&gt;complete_all

in __fw_state_set,
...
111    if (status == FW_STATUS_DONE || status == FW_STATUS_ABORTED)
112       complete_all(&amp;fw_st-&gt;completion);
-------------------------------------------
BTW,the double abort issue would not cause kernel panic or create an issue,
but slow down it sometimes.The change is just a minor optimization.

Signed-off-by: Junyong Sun &lt;sunjunyong@xiaomi.com&gt;
Acked-by: Luis Chamberlain &lt;mcgrof@kernel.org&gt;
Link: https://lore.kernel.org/r/1583202968-28792-1-git-send-email-sunjunyong@xiaomi.com
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
Signed-off-by: Sasha Levin &lt;sashal@kernel.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
[ Upstream commit bcfbd3523f3c6eea51a74d217a8ebc5463bcb7f4 ]

fw_sysfs_wait_timeout may return err with -ENOENT
at fw_load_sysfs_fallback and firmware is already
in abort status, no need to abort again, so skip it.

This issue is caused by concurrent situation like below:
when thread 1# wait firmware loading, thread 2# may write
-1 to abort loading and wakeup thread 1# before it timeout.
so wait_for_completion_killable_timeout of thread 1# would
return remaining time which is != 0 with fw_st-&gt;status
FW_STATUS_ABORTED.And the results would be converted into
err -ENOENT in __fw_state_wait_common and transfered to
fw_load_sysfs_fallback in thread 1#.
The -ENOENT means firmware status is already at ABORTED,
so fw_load_sysfs_fallback no need to get mutex to abort again.
-----------------------------
thread 1#,wait for loading
fw_load_sysfs_fallback
 -&gt;fw_sysfs_wait_timeout
    -&gt;__fw_state_wait_common
       -&gt;wait_for_completion_killable_timeout

in __fw_state_wait_common,
...
93    ret = wait_for_completion_killable_timeout(&amp;fw_st-&gt;completion, timeout);
94    if (ret != 0 &amp;&amp; fw_st-&gt;status == FW_STATUS_ABORTED)
95       return -ENOENT;
96    if (!ret)
97	 return -ETIMEDOUT;
98
99    return ret &lt; 0 ? ret : 0;
-----------------------------
thread 2#, write -1 to abort loading
firmware_loading_store
 -&gt;fw_load_abort
   -&gt;__fw_load_abort
     -&gt;fw_state_aborted
       -&gt;__fw_state_set
         -&gt;complete_all

in __fw_state_set,
...
111    if (status == FW_STATUS_DONE || status == FW_STATUS_ABORTED)
112       complete_all(&amp;fw_st-&gt;completion);
-------------------------------------------
BTW,the double abort issue would not cause kernel panic or create an issue,
but slow down it sometimes.The change is just a minor optimization.

Signed-off-by: Junyong Sun &lt;sunjunyong@xiaomi.com&gt;
Acked-by: Luis Chamberlain &lt;mcgrof@kernel.org&gt;
Link: https://lore.kernel.org/r/1583202968-28792-1-git-send-email-sunjunyong@xiaomi.com
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
Signed-off-by: Sasha Levin &lt;sashal@kernel.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>firmware: improve LSM/IMA security behaviour</title>
<updated>2019-07-21T07:03:03+00:00</updated>
<author>
<name>Sven Van Asbroeck</name>
<email>thesven73@gmail.com</email>
</author>
<published>2019-06-17T18:23:54+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=244db54441a110906d770f777d78abffa732b915'/>
<id>244db54441a110906d770f777d78abffa732b915</id>
<content type='text'>
commit 2472d64af2d3561954e2f05365a67692bb852f2a upstream.

The firmware loader queries if LSM/IMA permits it to load firmware
via the sysfs fallback. Unfortunately, the code does the opposite:
it expressly permits sysfs fw loading if security_kernel_load_data(
LOADING_FIRMWARE) returns -EACCES. This happens because a
zero-on-success return value is cast to a bool that's true on success.

Fix the return value handling so we get the correct behaviour.

Fixes: 6e852651f28e ("firmware: add call to LSM hook before firmware sysfs fallback")
Cc: Stable &lt;stable@vger.kernel.org&gt;
Cc: Mimi Zohar &lt;zohar@linux.vnet.ibm.com&gt;
Cc: Kees Cook &lt;keescook@chromium.org&gt;
To: Luis Chamberlain &lt;mcgrof@kernel.org&gt;
Cc: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
Cc: "Rafael J. Wysocki" &lt;rafael@kernel.org&gt;
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Sven Van Asbroeck &lt;TheSven73@gmail.com&gt;
Reviewed-by: Mimi Zohar &lt;zohar@linux.ibm.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;

</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
commit 2472d64af2d3561954e2f05365a67692bb852f2a upstream.

The firmware loader queries if LSM/IMA permits it to load firmware
via the sysfs fallback. Unfortunately, the code does the opposite:
it expressly permits sysfs fw loading if security_kernel_load_data(
LOADING_FIRMWARE) returns -EACCES. This happens because a
zero-on-success return value is cast to a bool that's true on success.

Fix the return value handling so we get the correct behaviour.

Fixes: 6e852651f28e ("firmware: add call to LSM hook before firmware sysfs fallback")
Cc: Stable &lt;stable@vger.kernel.org&gt;
Cc: Mimi Zohar &lt;zohar@linux.vnet.ibm.com&gt;
Cc: Kees Cook &lt;keescook@chromium.org&gt;
To: Luis Chamberlain &lt;mcgrof@kernel.org&gt;
Cc: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
Cc: "Rafael J. Wysocki" &lt;rafael@kernel.org&gt;
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Sven Van Asbroeck &lt;TheSven73@gmail.com&gt;
Reviewed-by: Mimi Zohar &lt;zohar@linux.ibm.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;

</pre>
</div>
</content>
</entry>
<entry>
<title>firmware: Always initialize the fw_priv list object</title>
<updated>2018-09-30T15:49:55+00:00</updated>
<author>
<name>Bjorn Andersson</name>
<email>bjorn.andersson@linaro.org</email>
</author>
<published>2018-09-20T01:09:38+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=7012040576c6ae25a47035659ee48673612c2c27'/>
<id>7012040576c6ae25a47035659ee48673612c2c27</id>
<content type='text'>
When freeing the fw_priv the item is taken off the list. This causes an
oops in the FW_OPT_NOCACHE case as the list object is not initialized.

Make sure to initialize the list object regardless of this flag.

Fixes: 422b3db2a503 ("firmware: Fix security issue with request_firmware_into_buf()")
Cc: stable@vger.kernel.org
Cc: Rishabh Bhatnagar &lt;rishabhb@codeaurora.org&gt;
Signed-off-by: Bjorn Andersson &lt;bjorn.andersson@linaro.org&gt;
Reviewed-by: Rafael J. Wysocki &lt;rafael.j.wysocki@intel.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When freeing the fw_priv the item is taken off the list. This causes an
oops in the FW_OPT_NOCACHE case as the list object is not initialized.

Make sure to initialize the list object regardless of this flag.

Fixes: 422b3db2a503 ("firmware: Fix security issue with request_firmware_into_buf()")
Cc: stable@vger.kernel.org
Cc: Rishabh Bhatnagar &lt;rishabhb@codeaurora.org&gt;
Signed-off-by: Bjorn Andersson &lt;bjorn.andersson@linaro.org&gt;
Reviewed-by: Rafael J. Wysocki &lt;rafael.j.wysocki@intel.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>firmware: Fix security issue with request_firmware_into_buf()</title>
<updated>2018-09-12T07:31:00+00:00</updated>
<author>
<name>Rishabh Bhatnagar</name>
<email>rishabhb@codeaurora.org</email>
</author>
<published>2018-08-31T15:43:31+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=422b3db2a5036add39a82425b1dd9fb6c96481e8'/>
<id>422b3db2a5036add39a82425b1dd9fb6c96481e8</id>
<content type='text'>
When calling request_firmware_into_buf() with the FW_OPT_NOCACHE flag
it is expected that firmware is loaded into buffer from memory.
But inside alloc_lookup_fw_priv every new firmware that is loaded is
added to the firmware cache (fwc) list head. So if any driver requests
a firmware that is already loaded the code iterates over the above
mentioned list and it can end up giving a pointer to other device driver's
firmware buffer.
Also the existing copy may either be modified by drivers, remote processors
or even freed. This causes a potential security issue with batched requests
when using request_firmware_into_buf.

Fix alloc_lookup_fw_priv to not add to the fwc head list if FW_OPT_NOCACHE
is set, and also don't do the lookup in the list.

Fixes: 0e742e9275 ("firmware: provide infrastructure to make fw caching optional")
[mcgrof: broken since feature introduction on v4.8]

Cc: stable@vger.kernel.org # v4.8+
Signed-off-by: Vikram Mulukutla &lt;markivx@codeaurora.org&gt;
Signed-off-by: Rishabh Bhatnagar &lt;rishabhb@codeaurora.org&gt;
Signed-off-by: Luis Chamberlain &lt;mcgrof@kernel.org&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When calling request_firmware_into_buf() with the FW_OPT_NOCACHE flag
it is expected that firmware is loaded into buffer from memory.
But inside alloc_lookup_fw_priv every new firmware that is loaded is
added to the firmware cache (fwc) list head. So if any driver requests
a firmware that is already loaded the code iterates over the above
mentioned list and it can end up giving a pointer to other device driver's
firmware buffer.
Also the existing copy may either be modified by drivers, remote processors
or even freed. This causes a potential security issue with batched requests
when using request_firmware_into_buf.

Fix alloc_lookup_fw_priv to not add to the fwc head list if FW_OPT_NOCACHE
is set, and also don't do the lookup in the list.

Fixes: 0e742e9275 ("firmware: provide infrastructure to make fw caching optional")
[mcgrof: broken since feature introduction on v4.8]

Cc: stable@vger.kernel.org # v4.8+
Signed-off-by: Vikram Mulukutla &lt;markivx@codeaurora.org&gt;
Signed-off-by: Rishabh Bhatnagar &lt;rishabhb@codeaurora.org&gt;
Signed-off-by: Luis Chamberlain &lt;mcgrof@kernel.org&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</pre>
</div>
</content>
</entry>
</feed>
