firmware: Improve logging of success/failure for direct file loading
Now that the user-mode helper is disabled, we can report specific errors from the direct loading path. svn path=/dists/sid/linux/; revision=20883
This commit is contained in:
Ben Hutchings
parent
b5b063978c
commit
ed74162bfd
|
|
@ -33,6 +33,7 @@ linux (3.12.4-1) UNRELEASED; urgency=medium
|
|||
- [/parisc64-smp] Enable PATA_SIL680, AGP, AGP_PARISC, DRM,
|
||||
DRM_RADEON as modules
|
||||
* firmware: Disable FW_LOADER_USER_HELPER (see #725714)
|
||||
* firmware: Improve logging of success/failure for direct file loading
|
||||
|
||||
[ Ian Campbell ]
|
||||
* [armel/kirkwood+orion] Reenable MARVELL_PHY (Closes: #723177)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,37 @@
|
|||
From: Ben Hutchings <ben@decadent.org.uk>
|
||||
Date: Sat, 14 Dec 2013 17:05:45 +0000
|
||||
Subject: firmware_class: Fix read size check
|
||||
|
||||
We expect to read firmware blobs with a single call to kernel_read(),
|
||||
which returns int. Therefore the size must be within the range of
|
||||
int, not long.
|
||||
|
||||
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
|
||||
---
|
||||
--- a/drivers/base/firmware_class.c
|
||||
+++ b/drivers/base/firmware_class.c
|
||||
@@ -270,21 +270,21 @@ module_param_string(path, fw_path_para,
|
||||
MODULE_PARM_DESC(path, "customized firmware image search path with a higher priority than default path");
|
||||
|
||||
/* Don't inline this: 'struct kstat' is biggish */
|
||||
-static noinline_for_stack long fw_file_size(struct file *file)
|
||||
+static noinline_for_stack int fw_file_size(struct file *file)
|
||||
{
|
||||
struct kstat st;
|
||||
if (vfs_getattr(&file->f_path, &st))
|
||||
return -1;
|
||||
if (!S_ISREG(st.mode))
|
||||
return -1;
|
||||
- if (st.size != (long)st.size)
|
||||
+ if (st.size != (int)st.size)
|
||||
return -1;
|
||||
return st.size;
|
||||
}
|
||||
|
||||
static bool fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf)
|
||||
{
|
||||
- long size;
|
||||
+ int size;
|
||||
char *buf;
|
||||
|
||||
size = fw_file_size(file);
|
||||
|
|
@ -12,88 +12,31 @@ aren't currently logged.
|
|||
In case of a driver that tries multiple names, this may result in the
|
||||
impression that it failed to initialise. Therefore, also log successes.
|
||||
|
||||
Change existing log messages to consistently use the given device, not
|
||||
the temporary child device we create.
|
||||
|
||||
This makes many error messages in drivers redundant, which will be
|
||||
removed in later patches.
|
||||
|
||||
This does not cover the case where we fall back to a user-mode helper
|
||||
(which is no longer enabled in Debian).
|
||||
---
|
||||
--- a/drivers/base/firmware_class.c
|
||||
+++ b/drivers/base/firmware_class.c
|
||||
@@ -636,15 +636,24 @@ static ssize_t firmware_loading_store(st
|
||||
* is completed.
|
||||
* */
|
||||
fw_map_pages_buf(fw_buf);
|
||||
+ if (!fw_buf->data)
|
||||
+ dev_err(dev->parent, "%s: vmap() failed\n",
|
||||
+ __func__);
|
||||
list_del_init(&fw_buf->pending_list);
|
||||
complete_all(&fw_buf->completion);
|
||||
break;
|
||||
}
|
||||
/* fallthrough */
|
||||
default:
|
||||
- dev_err(dev, "%s: unexpected value (%d)\n", __func__, loading);
|
||||
- /* fallthrough */
|
||||
+ dev_err(dev->parent, "%s: unexpected value (%d)\n",
|
||||
+ __func__, loading);
|
||||
+ goto err;
|
||||
case -1:
|
||||
+ dev_err(dev->parent,
|
||||
+ "firmware: agent aborted loading %s (not found?)\n",
|
||||
+ fw_priv->buf->fw_id);
|
||||
+ /* fallthrough */
|
||||
+ err:
|
||||
fw_load_abort(fw_priv);
|
||||
break;
|
||||
@@ -337,9 +337,12 @@ static int fw_get_filesystem_firmware(st
|
||||
}
|
||||
@@ -810,6 +819,9 @@ static void firmware_class_timeout_work(
|
||||
struct firmware_priv, timeout_work.work);
|
||||
__putname(path);
|
||||
|
||||
mutex_lock(&fw_lock);
|
||||
+ dev_err(fw_priv->dev.parent,
|
||||
+ "firmware: agent did not handle request for %s\n",
|
||||
+ fw_priv->buf->fw_id);
|
||||
fw_load_abort(fw_priv);
|
||||
mutex_unlock(&fw_lock);
|
||||
}
|
||||
@@ -858,13 +870,15 @@ static int _request_firmware_load(struct
|
||||
|
||||
retval = device_add(f_dev);
|
||||
if (retval) {
|
||||
- dev_err(f_dev, "%s: device_register failed\n", __func__);
|
||||
+ dev_err(f_dev->parent, "%s: device_register failed\n",
|
||||
+ __func__);
|
||||
goto err_put_dev;
|
||||
}
|
||||
|
||||
retval = device_create_bin_file(f_dev, &firmware_attr_data);
|
||||
if (retval) {
|
||||
- dev_err(f_dev, "%s: sysfs_create_bin_file failed\n", __func__);
|
||||
+ dev_err(f_dev->parent, "%s: sysfs_create_bin_file failed\n",
|
||||
+ __func__);
|
||||
goto err_del_dev;
|
||||
}
|
||||
|
||||
@@ -877,14 +891,15 @@ static int _request_firmware_load(struct
|
||||
- if (!ret) {
|
||||
- dev_dbg(device, "firmware: direct-loading firmware %s\n",
|
||||
- buf->fw_id);
|
||||
+ if (ret) {
|
||||
+ dev_err(device, "firmware: failed to load %s (%d)\n",
|
||||
+ buf->fw_id, ret);
|
||||
+ } else {
|
||||
+ dev_info(device, "firmware: direct-loading firmware %s\n",
|
||||
+ buf->fw_id);
|
||||
mutex_lock(&fw_lock);
|
||||
list_del_init(&buf->pending_list);
|
||||
mutex_unlock(&fw_lock);
|
||||
- dev_err(f_dev, "%s: device_create_file failed\n", __func__);
|
||||
+ dev_err(f_dev->parent, "%s: device_create_file failed\n",
|
||||
+ __func__);
|
||||
goto err_del_bin_attr;
|
||||
}
|
||||
|
||||
if (uevent) {
|
||||
buf->need_uevent = true;
|
||||
dev_set_uevent_suppress(f_dev, false);
|
||||
- dev_dbg(f_dev, "firmware: requesting %s\n", buf->fw_id);
|
||||
+ dev_dbg(f_dev->parent, "firmware: requesting %s\n", buf->fw_id);
|
||||
if (timeout != MAX_SCHEDULE_TIMEOUT)
|
||||
schedule_delayed_work(&fw_priv->timeout_work, timeout);
|
||||
|
||||
@@ -993,7 +1008,8 @@ _request_firmware_prepare(struct firmwar
|
||||
set_bit(FW_STATUS_DONE, &buf->status);
|
||||
complete_all(&buf->completion);
|
||||
@@ -992,7 +995,8 @@ _request_firmware_prepare(struct firmwar
|
||||
}
|
||||
|
||||
if (fw_get_builtin_firmware(firmware, name)) {
|
||||
|
|
@ -103,25 +46,7 @@ removed in later patches.
|
|||
return 0; /* assigned */
|
||||
}
|
||||
|
||||
@@ -1024,9 +1040,16 @@ static int assign_firmware_buf(struct fi
|
||||
struct firmware_buf *buf = fw->priv;
|
||||
|
||||
mutex_lock(&fw_lock);
|
||||
- if (!buf->size || is_fw_load_aborted(buf)) {
|
||||
+ if (is_fw_load_aborted(buf)) {
|
||||
+ /* failure has already been logged */
|
||||
mutex_unlock(&fw_lock);
|
||||
return -ENOENT;
|
||||
+ } else if (!buf->size) {
|
||||
+ dev_err(device->parent,
|
||||
+ "firmware: agent loaded no data for %s (not found?)\n",
|
||||
+ buf->fw_id);
|
||||
+ mutex_unlock(&fw_lock);
|
||||
+ return -ENOENT;
|
||||
}
|
||||
|
||||
/*
|
||||
@@ -1075,7 +1098,7 @@ _request_firmware(const struct firmware
|
||||
@@ -1073,7 +1077,7 @@ _request_firmware(const struct firmware
|
||||
if (nowait) {
|
||||
timeout = usermodehelper_read_lock_wait(timeout);
|
||||
if (!timeout) {
|
||||
|
|
@ -130,13 +55,3 @@ removed in later patches.
|
|||
name);
|
||||
ret = -EBUSY;
|
||||
goto out;
|
||||
@@ -1103,6 +1126,9 @@ _request_firmware(const struct firmware
|
||||
if (ret < 0) {
|
||||
release_firmware(fw);
|
||||
fw = NULL;
|
||||
+ } else {
|
||||
+ dev_info(device, "firmware: agent loaded %s into memory\n",
|
||||
+ name);
|
||||
}
|
||||
|
||||
*firmware_p = fw;
|
||||
|
|
|
|||
148
debian/patches/bugfix/all/firmware_class-return-specific-errors-from-file-read.patch
vendored
Normal file
148
debian/patches/bugfix/all/firmware_class-return-specific-errors-from-file-read.patch
vendored
Normal file
|
|
@ -0,0 +1,148 @@
|
|||
From: Ben Hutchings <ben@decadent.org.uk>
|
||||
Date: Sat, 14 Dec 2013 17:14:39 +0000
|
||||
Subject: firmware_class: Return specific errors from file read
|
||||
|
||||
Currently the various failure cases are not distinguished and are all
|
||||
reported as -ENOENT.
|
||||
|
||||
Change fw_file_size(), fw_read_file_contents() and
|
||||
fw_get_filesystem_firmware() to return an error code on failure.
|
||||
|
||||
Change _request_firmware() to return the error code from
|
||||
fw_get_filesystem_firmware() if CONFIG_FW_LOADER_USER_HELPER is not
|
||||
enabled. (If it is enabled and also fails, unfortunately we can't
|
||||
tell why.)
|
||||
|
||||
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
|
||||
---
|
||||
--- a/drivers/base/firmware_class.c
|
||||
+++ b/drivers/base/firmware_class.c
|
||||
@@ -273,40 +273,47 @@ MODULE_PARM_DESC(path, "customized firmw
|
||||
static noinline_for_stack int fw_file_size(struct file *file)
|
||||
{
|
||||
struct kstat st;
|
||||
- if (vfs_getattr(&file->f_path, &st))
|
||||
- return -1;
|
||||
+ int ret;
|
||||
+
|
||||
+ ret = vfs_getattr(&file->f_path, &st);
|
||||
+ if (ret)
|
||||
+ return ret;
|
||||
if (!S_ISREG(st.mode))
|
||||
- return -1;
|
||||
+ return -ENOTTY;
|
||||
if (st.size != (int)st.size)
|
||||
- return -1;
|
||||
+ return -EFBIG;
|
||||
return st.size;
|
||||
}
|
||||
|
||||
-static bool fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf)
|
||||
+static int fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf)
|
||||
{
|
||||
+ int read_size;
|
||||
int size;
|
||||
char *buf;
|
||||
|
||||
size = fw_file_size(file);
|
||||
- if (size <= 0)
|
||||
- return false;
|
||||
+ if (size < 0)
|
||||
+ return size;
|
||||
+ if (size == 0)
|
||||
+ return -EINVAL;
|
||||
buf = vmalloc(size);
|
||||
if (!buf)
|
||||
- return false;
|
||||
- if (kernel_read(file, 0, buf, size) != size) {
|
||||
+ return -ENOMEM;
|
||||
+ read_size = kernel_read(file, 0, buf, size);
|
||||
+ if (read_size != size) {
|
||||
vfree(buf);
|
||||
- return false;
|
||||
+ return read_size < 0 ? read_size : -EIO;
|
||||
}
|
||||
fw_buf->data = buf;
|
||||
fw_buf->size = size;
|
||||
- return true;
|
||||
+ return 0;
|
||||
}
|
||||
|
||||
-static bool fw_get_filesystem_firmware(struct device *device,
|
||||
+static int fw_get_filesystem_firmware(struct device *device,
|
||||
struct firmware_buf *buf)
|
||||
{
|
||||
int i;
|
||||
- bool success = false;
|
||||
+ int ret = -ENOENT;
|
||||
char *path = __getname();
|
||||
|
||||
for (i = 0; i < ARRAY_SIZE(fw_path); i++) {
|
||||
@@ -319,16 +326,18 @@ static bool fw_get_filesystem_firmware(s
|
||||
snprintf(path, PATH_MAX, "%s/%s", fw_path[i], buf->fw_id);
|
||||
|
||||
file = filp_open(path, O_RDONLY, 0);
|
||||
- if (IS_ERR(file))
|
||||
+ if (IS_ERR(file)) {
|
||||
+ ret = PTR_ERR(file);
|
||||
continue;
|
||||
- success = fw_read_file_contents(file, buf);
|
||||
+ }
|
||||
+ ret = fw_read_file_contents(file, buf);
|
||||
fput(file);
|
||||
- if (success)
|
||||
+ if (!ret)
|
||||
break;
|
||||
}
|
||||
__putname(path);
|
||||
|
||||
- if (success) {
|
||||
+ if (!ret) {
|
||||
dev_dbg(device, "firmware: direct-loading firmware %s\n",
|
||||
buf->fw_id);
|
||||
mutex_lock(&fw_lock);
|
||||
@@ -337,7 +346,7 @@ static bool fw_get_filesystem_firmware(s
|
||||
mutex_unlock(&fw_lock);
|
||||
}
|
||||
|
||||
- return success;
|
||||
+ return ret;
|
||||
}
|
||||
|
||||
/* firmware holds the ownership of pages */
|
||||
@@ -933,13 +942,6 @@ static void kill_requests_without_uevent
|
||||
#endif
|
||||
|
||||
#else /* CONFIG_FW_LOADER_USER_HELPER */
|
||||
-static inline int
|
||||
-fw_load_from_user_helper(struct firmware *firmware, const char *name,
|
||||
- struct device *device, bool uevent, bool nowait,
|
||||
- long timeout)
|
||||
-{
|
||||
- return -ENOENT;
|
||||
-}
|
||||
|
||||
/* No abort during direct loading */
|
||||
#define is_fw_load_aborted(buf) false
|
||||
@@ -1067,7 +1069,6 @@ _request_firmware(const struct firmware
|
||||
if (ret <= 0) /* error or already assigned */
|
||||
goto out;
|
||||
|
||||
- ret = 0;
|
||||
timeout = firmware_loading_timeout();
|
||||
if (nowait) {
|
||||
timeout = usermodehelper_read_lock_wait(timeout);
|
||||
@@ -1086,9 +1087,12 @@ _request_firmware(const struct firmware
|
||||
}
|
||||
}
|
||||
|
||||
- if (!fw_get_filesystem_firmware(device, fw->priv))
|
||||
+ ret = fw_get_filesystem_firmware(device, fw->priv);
|
||||
+#ifdef CONFIG_FW_LOADER_USER_HELPER
|
||||
+ if (ret)
|
||||
ret = fw_load_from_user_helper(fw, name, device,
|
||||
uevent, nowait, timeout);
|
||||
+#endif
|
||||
|
||||
/* don't cache firmware handled without uevent */
|
||||
if (!ret)
|
||||
|
|
@ -13,6 +13,8 @@ features/all/Kbuild-kconfig-Verbose-version-of-listnewconfig.patch
|
|||
features/all/drivers-media-dvb-usb-af9005-request_firmware.patch
|
||||
features/all/sound-pci-cs46xx-request_firmware.patch
|
||||
debian/iwlwifi-do-not-request-unreleased-firmware.patch
|
||||
bugfix/all/firmware_class-fix-read-size-check.patch
|
||||
bugfix/all/firmware_class-return-specific-errors-from-file-read.patch
|
||||
bugfix/all/firmware_class-log-every-success-and-failure.patch
|
||||
bugfix/all/firmware-remove-redundant-log-messages-from-drivers.patch
|
||||
bugfix/all/radeon-firmware-is-required-for-drm-and-kms-on-r600-onward.patch
|
||||
|
|
|
|||
Loading…
Reference in New Issue