Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

firmware_loader: Block path traversal

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->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 <dakr@kernel.org>
Fixes: abb139e75c2c ("firmware: teach the kernel to load firmware files directly from the filesystem")
Signed-off-by: Jann Horn <jannh@google.com>
Acked-by: Luis Chamberlain <mcgrof@kernel.org>
Link: https://lore.kernel.org/r/20240828-firmware-traversal-v3-1-c76529c63b5f@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Jann Horn and committed by
Greg Kroah-Hartman
f0e5311a 888f67e6

+30
+30
drivers/base/firmware_loader/main.c
··· 849 849 {} 850 850 #endif 851 851 852 + /* 853 + * Reject firmware file names with ".." path components. 854 + * There are drivers that construct firmware file names from device-supplied 855 + * strings, and we don't want some device to be able to tell us "I would like to 856 + * be sent my firmware from ../../../etc/shadow, please". 857 + * 858 + * Search for ".." surrounded by either '/' or start/end of string. 859 + * 860 + * This intentionally only looks at the firmware name, not at the firmware base 861 + * directory or at symlink contents. 862 + */ 863 + static bool name_contains_dotdot(const char *name) 864 + { 865 + size_t name_len = strlen(name); 866 + 867 + return strcmp(name, "..") == 0 || strncmp(name, "../", 3) == 0 || 868 + strstr(name, "/../") != NULL || 869 + (name_len >= 3 && strcmp(name+name_len-3, "/..") == 0); 870 + } 871 + 852 872 /* called from request_firmware() and request_firmware_work_func() */ 853 873 static int 854 874 _request_firmware(const struct firmware **firmware_p, const char *name, ··· 885 865 return -EINVAL; 886 866 887 867 if (!name || name[0] == '\0') { 868 + ret = -EINVAL; 869 + goto out; 870 + } 871 + 872 + if (name_contains_dotdot(name)) { 873 + dev_warn(device, 874 + "Firmware load for '%s' refused, path contains '..' component\n", 875 + name); 888 876 ret = -EINVAL; 889 877 goto out; 890 878 } ··· 974 946 * @name will be used as $FIRMWARE in the uevent environment and 975 947 * should be distinctive enough not to be confused with any other 976 948 * firmware image for this or any other device. 949 + * It must not contain any ".." path components - "foo/bar..bin" is 950 + * allowed, but "foo/../bar.bin" is not. 977 951 * 978 952 * Caller must hold the reference count of @device. 979 953 *