Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

Input: goodix - fix race on driver unbind

Because there is no way to detect if the touchscreen has pen support,
the driver is allocating and registering the input_pen input_dev on
receiving the first pen event.

But this means that the input_dev gets allocated after the request_irq()
call which means that the devm framework will free it before disabling
the irq, leaving a window where the irq handler may run and reference the
free-ed input_dev.

To fix this move the allocation of the input_pen input_dev to before
the request_irq() call, while still only registering it on the first pen
event so that the driver does not advertise pen capability on touchscreens
without it (most goodix touchscreens do not have pen support).

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20220131143539.109142-4-hdegoede@redhat.com
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

authored by

Hans de Goede and committed by
Dmitry Torokhov
65de58c2 ae8e80c5

+23 -13
+22 -13
drivers/input/touchscreen/goodix.c
··· 297 297 return -ENOMSG; 298 298 } 299 299 300 - static struct input_dev *goodix_create_pen_input(struct goodix_ts_data *ts) 300 + static int goodix_create_pen_input(struct goodix_ts_data *ts) 301 301 { 302 302 struct device *dev = &ts->client->dev; 303 303 struct input_dev *input; 304 304 305 305 input = devm_input_allocate_device(dev); 306 306 if (!input) 307 - return NULL; 307 + return -ENOMEM; 308 308 309 309 input_copy_abs(input, ABS_X, ts->input_dev, ABS_MT_POSITION_X); 310 310 input_copy_abs(input, ABS_Y, ts->input_dev, ABS_MT_POSITION_Y); ··· 331 331 input->id.product = 0x1001; 332 332 input->id.version = ts->version; 333 333 334 - if (input_register_device(input) != 0) { 335 - input_free_device(input); 336 - return NULL; 337 - } 338 - 339 - return input; 334 + ts->input_pen = input; 335 + return 0; 340 336 } 341 337 342 338 static void goodix_ts_report_pen_down(struct goodix_ts_data *ts, u8 *data) 343 339 { 344 - int input_x, input_y, input_w; 340 + int input_x, input_y, input_w, error; 345 341 u8 key_value; 346 342 347 - if (!ts->input_pen) { 348 - ts->input_pen = goodix_create_pen_input(ts); 349 - if (!ts->input_pen) 350 - return; 343 + if (!ts->pen_input_registered) { 344 + error = input_register_device(ts->input_pen); 345 + ts->pen_input_registered = (error == 0) ? 1 : error; 351 346 } 347 + 348 + if (ts->pen_input_registered < 0) 349 + return; 352 350 353 351 if (ts->contact_size == 9) { 354 352 input_x = get_unaligned_le16(&data[4]); ··· 1204 1206 "Failed to register input device: %d", error); 1205 1207 return error; 1206 1208 } 1209 + 1210 + /* 1211 + * Create the input_pen device before goodix_request_irq() calls 1212 + * devm_request_threaded_irq() so that the devm framework frees 1213 + * it after disabling the irq. 1214 + * Unfortunately there is no way to detect if the touchscreen has pen 1215 + * support, so registering the dev is delayed till the first pen event. 1216 + */ 1217 + error = goodix_create_pen_input(ts); 1218 + if (error) 1219 + return error; 1207 1220 1208 1221 ts->irq_flags = goodix_irq_flags[ts->int_trigger_type] | IRQF_ONESHOT; 1209 1222 error = goodix_request_irq(ts);
+1
drivers/input/touchscreen/goodix.h
··· 94 94 u16 version; 95 95 bool reset_controller_at_probe; 96 96 bool load_cfg_from_disk; 97 + int pen_input_registered; 97 98 struct completion firmware_loading_complete; 98 99 unsigned long irq_flags; 99 100 enum goodix_irq_pin_access_method irq_pin_access_method;