Commit bd9eb7fb authored by Chuansheng Liu's avatar Chuansheng Liu Committed by Greg Kroah-Hartman

firmware loader: Fix the concurrent request_firmware() race for kref_get/put

There is one race that both request_firmware() with the same
firmware name.

The race scenerio is as below:
CPU1                                                  CPU2
request_firmware() -->
_request_firmware_load() return err                   another request_firmware() is coming -->
_request_firmware_cleanup is called -->               _request_firmware_prepare -->
release_firmware --->                                 fw_lookup_and_allocate_buf -->
                                                      spin_lock(&fwc->lock)
...                                                   __fw_lookup_buf() return true
fw_free_buf() will be called -->                      ...
kref_put -->
decrease the refcount to 0
                                                      kref_get(&tmp->ref) ==> it will trigger warning
                                                                              due to refcount == 0
__fw_free_buf() -->
...                                                   spin_unlock(&fwc->lock)
spin_lock(&fwc->lock)
list_del(&buf->list)
spin_unlock(&fwc->lock)
kfree(buf)
                                                      After that, the freed buf will be used.

The key race is decreasing refcount to 0 and list_del is not protected together by
fwc->lock, and it is possible another thread try to get it between refcount==0
and list_del.

Fix it here to protect it together.
Acked-by: default avatarMing Lei <ming.lei@canonical.com>
Signed-off-by: default avatarliu chuansheng <chuansheng.liu@intel.com>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent ce2fcbd9
...@@ -246,7 +246,6 @@ static void __fw_free_buf(struct kref *ref) ...@@ -246,7 +246,6 @@ static void __fw_free_buf(struct kref *ref)
__func__, buf->fw_id, buf, buf->data, __func__, buf->fw_id, buf, buf->data,
(unsigned int)buf->size); (unsigned int)buf->size);
spin_lock(&fwc->lock);
list_del(&buf->list); list_del(&buf->list);
spin_unlock(&fwc->lock); spin_unlock(&fwc->lock);
...@@ -263,7 +262,10 @@ static void __fw_free_buf(struct kref *ref) ...@@ -263,7 +262,10 @@ static void __fw_free_buf(struct kref *ref)
static void fw_free_buf(struct firmware_buf *buf) static void fw_free_buf(struct firmware_buf *buf)
{ {
kref_put(&buf->ref, __fw_free_buf); struct firmware_cache *fwc = buf->fwc;
spin_lock(&fwc->lock);
if (!kref_put(&buf->ref, __fw_free_buf))
spin_unlock(&fwc->lock);
} }
/* direct firmware loading support */ /* direct firmware loading support */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment