Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ce2fcbd9 authored by Chuansheng Liu's avatar Chuansheng Liu Committed by Greg Kroah-Hartman
Browse files

firmware loader: Fix the race FW_STATUS_DONE is followed by class_timeout 



There is a race as below when calling request_firmware():
CPU1                                   CPU2
write 0 > loading
mutex_lock(&fw_lock)
...
set_bit FW_STATUS_DONE                 class_timeout is coming
                                       set_bit FW_STATUS_ABORT
complete_all &completion
...
mutex_unlock(&fw_lock)

In this time, the bit FW_STATUS_DONE and FW_STATUS_ABORT are set,
and request_firmware() will return failure due to condition in
_request_firmware_load():
	if (!buf->size || test_bit(FW_STATUS_ABORT, &buf->status))
		retval = -ENOENT;

But from the above scenerio, it should be a successful requesting.
So we need judge if the bit FW_STATUS_DONE is already set before
calling fw_load_abort() in timeout function.

As Ming's proposal, we need change the timer into sched_work to
benefit from using &fw_lock mutex also.

Signed-off-by: default avatarliu chuansheng <chuansheng.liu@intel.com>
Acked-by: default avatarMing Lei <ming.lei@canonical.com>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent ecdca043

drivers/base/firmware_class.c

+16 −8
Original line number Original line Diff line number Diff line
@@ -143,7 +143,7 @@ struct fw_cache_entry { 
};

};





struct firmware_priv {

struct firmware_priv {

	struct timer_list timeout;

	struct delayed_work timeout_work;

	bool nowait;

	bool nowait;

	struct device dev;

	struct device dev;

	struct firmware_buf *buf;

	struct firmware_buf *buf;
@@ -667,11 +667,18 @@ static struct bin_attribute firmware_attr_data = { 
	.write = firmware_data_write,

	.write = firmware_data_write,

};

};





static void firmware_class_timeout(u_long data)

static void firmware_class_timeout_work(struct work_struct *work)

{

{

	struct firmware_priv *fw_priv = (struct firmware_priv *) data;

	struct firmware_priv *fw_priv = container_of(work,

			struct firmware_priv, timeout_work.work);





	mutex_lock(&fw_lock);

	if (test_bit(FW_STATUS_DONE, &(fw_priv->buf->status))) {

		mutex_unlock(&fw_lock);

		return;

	}

	fw_load_abort(fw_priv);

	fw_load_abort(fw_priv);

	mutex_unlock(&fw_lock);

}

}





static struct firmware_priv *

static struct firmware_priv *
@@ -690,8 +697,8 @@ fw_create_instance(struct firmware *firmware, const char *fw_name, 




	fw_priv->nowait = nowait;

	fw_priv->nowait = nowait;

	fw_priv->fw = firmware;

	fw_priv->fw = firmware;

	setup_timer(&fw_priv->timeout,

	INIT_DELAYED_WORK(&fw_priv->timeout_work,

		    firmware_class_timeout, (u_long) fw_priv);

		firmware_class_timeout_work);





	f_dev = &fw_priv->dev;

	f_dev = &fw_priv->dev;
@@ -858,7 +865,9 @@ static int _request_firmware_load(struct firmware_priv *fw_priv, bool uevent, 
		dev_dbg(f_dev->parent, "firmware: direct-loading"

		dev_dbg(f_dev->parent, "firmware: direct-loading"

			" firmware %s\n", buf->fw_id);

			" firmware %s\n", buf->fw_id);





		mutex_lock(&fw_lock);

		set_bit(FW_STATUS_DONE, &buf->status);

		set_bit(FW_STATUS_DONE, &buf->status);

		mutex_unlock(&fw_lock);

		complete_all(&buf->completion);

		complete_all(&buf->completion);

		direct_load = 1;

		direct_load = 1;

		goto handle_fw;

		goto handle_fw;
@@ -894,15 +903,14 @@ static int _request_firmware_load(struct firmware_priv *fw_priv, bool uevent, 
		dev_set_uevent_suppress(f_dev, false);

		dev_set_uevent_suppress(f_dev, false);

		dev_dbg(f_dev, "firmware: requesting %s\n", buf->fw_id);

		dev_dbg(f_dev, "firmware: requesting %s\n", buf->fw_id);

		if (timeout != MAX_SCHEDULE_TIMEOUT)

		if (timeout != MAX_SCHEDULE_TIMEOUT)

			mod_timer(&fw_priv->timeout,

			schedule_delayed_work(&fw_priv->timeout_work, timeout);

				  round_jiffies_up(jiffies + timeout));





		kobject_uevent(&fw_priv->dev.kobj, KOBJ_ADD);

		kobject_uevent(&fw_priv->dev.kobj, KOBJ_ADD);

	}

	}





	wait_for_completion(&buf->completion);

	wait_for_completion(&buf->completion);





	del_timer_sync(&fw_priv->timeout);

	cancel_delayed_work_sync(&fw_priv->timeout_work);





handle_fw:

handle_fw:

	mutex_lock(&fw_lock);

	mutex_lock(&fw_lock);