Messages in this thread Patch in this message | | | Date | Wed, 8 Jul 2015 20:17:52 -0700 | Subject | Re: Linux 4.2-rc1 | From | Linus Torvalds <> |
| |
On Wed, Jul 8, 2015 at 5:58 PM, Ming Lei <ming.lei@canonical.com> wrote: > On Thu, Jul 9, 2015 at 1:29 AM, Linus Torvalds > <torvalds@linux-foundation.org> wrote: >> Also, it looks like you need to hold the "fw_lock" to even look at >> that pointer, since the buffer can get reallocated etc. > > Yes, the above code with holding 'fw_lock' is right fix for the issue since > sysfs read can happen anytime, and there is one race between firmware > request abort and reading uevent of sysfs.
So if fw_priv->buf is NULL, what should we do?
Should we skip the TIMEOUT= and ASYNC= fields too?
Something like the attached, perhaps?
Shuah, how reproducible is this? Does this (completely untested) patch make any difference?
Linus drivers/base/firmware_class.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index 9c4288362a8e..894bda114224 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -563,10 +563,8 @@ static void fw_dev_release(struct device *dev) kfree(fw_priv); } -static int firmware_uevent(struct device *dev, struct kobj_uevent_env *env) +static int do_firmware_uevent(struct firmware_priv *fw_priv, 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)) return -ENOMEM; if (add_uevent_var(env, "TIMEOUT=%i", loading_timeout)) @@ -577,6 +575,18 @@ static int firmware_uevent(struct device *dev, struct kobj_uevent_env *env) return 0; } +static int firmware_uevent(struct device *dev, struct kobj_uevent_env *env) +{ + struct firmware_priv *fw_priv = to_firmware_priv(dev); + int err = 0; + + mutex_lock(&fw_lock); + if (fw_priv->buf) + err = do_firmware_uevent(fw_priv, env); + mutex_unlock(&fw_lock); + return err; +} + static struct class firmware_class = { .name = "firmware", .class_attrs = firmware_class_attrs, | |