diff --git a/debian/changelog b/debian/changelog index 6e85c9594..9c93a52cd 100644 --- a/debian/changelog +++ b/debian/changelog @@ -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) diff --git a/debian/patches/bugfix/all/firmware_class-fix-read-size-check.patch b/debian/patches/bugfix/all/firmware_class-fix-read-size-check.patch new file mode 100644 index 000000000..794900edb --- /dev/null +++ b/debian/patches/bugfix/all/firmware_class-fix-read-size-check.patch @@ -0,0 +1,37 @@ +From: Ben Hutchings +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 +--- +--- 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); diff --git a/debian/patches/bugfix/all/firmware_class-log-every-success-and-failure.patch b/debian/patches/bugfix/all/firmware_class-log-every-success-and-failure.patch index 33343ad9d..441c9f841 100644 --- a/debian/patches/bugfix/all/firmware_class-log-every-success-and-failure.patch +++ b/debian/patches/bugfix/all/firmware_class-log-every-success-and-failure.patch @@ -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; diff --git a/debian/patches/bugfix/all/firmware_class-return-specific-errors-from-file-read.patch b/debian/patches/bugfix/all/firmware_class-return-specific-errors-from-file-read.patch new file mode 100644 index 000000000..bca54f7b9 --- /dev/null +++ b/debian/patches/bugfix/all/firmware_class-return-specific-errors-from-file-read.patch @@ -0,0 +1,148 @@ +From: Ben Hutchings +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 +--- +--- 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) diff --git a/debian/patches/series b/debian/patches/series index 38e45edcd..e99483f9d 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -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