linux-kernel - [PATCH V2] atmel: fix a race between fw

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 15 May 2015 19:19:21 +0800
From:	Pan Xinhui <xinhuix.pan@...el.com>
To:	linux-kernel@...r.kernel.org
CC:	dmitry.torokhov@...il.com, nick.dyer@...ev.co.uk,
	yanmin_zhang@...ux.intel.com, mnipxh@....com
Subject: [PATCH V2] atmel: fix a race between fw_load and data free


mxt_probe() may fail at last step, or we jsut unload mxt module soon.
the queue_work scheduled by request_firmware_nowait may run later,
and then access some data which is freed.
To handle this issue, add fw_load_completion field in mxt_data.
then we wait for it complete both in probe error path and mxt_remove().

here is the detail in probe, similar in remove.

module load:                                                             worker_thread:
mxt_probe -> mxt_initialize -> request_firmware_nowait (schedule_work)
                    |
             sysfs_create_group (fails)                                    mxt_config_cb -> mxt_configure_objects (may access data freed)
                    |
             err_free_object: some cleanup work, like free(data).

Signed-off-by: xinhui.pan <xinhuix.pan@...el.com>
---
change in V2:
  use fw_load_completion instead of statics.
  fix a race both in mxt_remove and mxt_probe.

  drivers/input/touchscreen/atmel_mxt_ts.c | 10 ++++++++++
  1 file changed, 10 insertions(+)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 40b98dd..3da040d 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -313,6 +313,9 @@ struct mxt_data {
  
  	/* for config update handling */
  	struct completion crc_completion;
+
+	/* for fw load handling */
+	struct completion fw_load_completion;
  };
  
  static size_t mxt_obj_size(const struct mxt_object *obj)
@@ -1982,8 +1985,10 @@ static int mxt_configure_objects(struct mxt_data *data,
  
  static void mxt_config_cb(const struct firmware *cfg, void *ctx)
  {
+	struct mxt_data *data = ctx;
  	mxt_configure_objects(ctx, cfg);
  	release_firmware(cfg);
+	complete(&data->fw_load_completion);
  }
  
  static int mxt_initialize(struct mxt_data *data)
@@ -2556,6 +2561,7 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
  	init_completion(&data->bl_completion);
  	init_completion(&data->reset_completion);
  	init_completion(&data->crc_completion);
+	init_completion(&data->fw_load_completion);
  
  	error = request_threaded_irq(client->irq, NULL, mxt_interrupt,
  				     pdata->irqflags | IRQF_ONESHOT,
@@ -2581,6 +2587,8 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
  	return 0;
  
  err_free_object:
+	mxt_wait_for_completion(data, &data->fw_load_completion,
+					MXT_FW_RESET_TIME);
  	mxt_free_input_device(data);
  	mxt_free_object_table(data);
  err_free_irq:
@@ -2594,6 +2602,8 @@ static int mxt_remove(struct i2c_client *client)
  {
  	struct mxt_data *data = i2c_get_clientdata(client);
  
+	mxt_wait_for_completion(data, &data->fw_load_completion,
+					MXT_FW_RESET_TIME);
  	sysfs_remove_group(&client->dev.kobj, &mxt_attr_group);
  	free_irq(data->irq, data);
  	mxt_free_input_device(data);
-- 
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/