Commit fabeb165 authored by Michał Kępień's avatar Michał Kępień Committed by Dmitry Torokhov

Input: sparse-keymap - use a managed allocation for keymap copy

Some platform drivers use devm_input_allocate_device() together with
sparse_keymap_setup() in their .probe callbacks.  While using the former
simplifies error handling, using the latter necessitates calling
sparse_keymap_free() in the error path and upon module unloading to
avoid leaking the copy of the keymap allocated by sparse_keymap_setup().

To help prevent such leaks and enable simpler error handling, make
sparse_keymap_setup() use devm_kmemdup() to create the keymap copy so
that it gets automatically freed.

This works for both managed and non-managed input devices as the keymap
is freed after the last reference to the input device is dropped.

Note that actions previously taken by sparse_keymap_free(), i.e. taking
the input device's mutex and zeroing its keycode and keycodemax fields,
are now redundant because the managed keymap will always be freed after
the input device is unregistered.
Signed-off-by: default avatarMichał Kępień <kernel@kempniu.pl>
Signed-off-by: default avatarDmitry Torokhov <dmitry.torokhov@gmail.com>
parent c470abd4
...@@ -160,12 +160,12 @@ static int sparse_keymap_setkeycode(struct input_dev *dev, ...@@ -160,12 +160,12 @@ static int sparse_keymap_setkeycode(struct input_dev *dev,
* @keymap: Keymap in form of array of &key_entry structures ending * @keymap: Keymap in form of array of &key_entry structures ending
* with %KE_END type entry * with %KE_END type entry
* @setup: Function that can be used to adjust keymap entries * @setup: Function that can be used to adjust keymap entries
* depending on device's deeds, may be %NULL * depending on device's needs, may be %NULL
* *
* The function calculates size and allocates copy of the original * The function calculates size and allocates copy of the original
* keymap after which sets up input device event bits appropriately. * keymap after which sets up input device event bits appropriately.
* Before destroying input device allocated keymap should be freed * The allocated copy of the keymap is automatically freed when it
* with a call to sparse_keymap_free(). * is no longer needed.
*/ */
int sparse_keymap_setup(struct input_dev *dev, int sparse_keymap_setup(struct input_dev *dev,
const struct key_entry *keymap, const struct key_entry *keymap,
...@@ -180,19 +180,18 @@ int sparse_keymap_setup(struct input_dev *dev, ...@@ -180,19 +180,18 @@ int sparse_keymap_setup(struct input_dev *dev,
for (e = keymap; e->type != KE_END; e++) for (e = keymap; e->type != KE_END; e++)
map_size++; map_size++;
map = kcalloc(map_size, sizeof(struct key_entry), GFP_KERNEL); map = devm_kmemdup(&dev->dev, keymap, map_size * sizeof(*map),
GFP_KERNEL);
if (!map) if (!map)
return -ENOMEM; return -ENOMEM;
memcpy(map, keymap, map_size * sizeof(struct key_entry));
for (i = 0; i < map_size; i++) { for (i = 0; i < map_size; i++) {
entry = &map[i]; entry = &map[i];
if (setup) { if (setup) {
error = setup(dev, entry); error = setup(dev, entry);
if (error) if (error)
goto err_out; return error;
} }
switch (entry->type) { switch (entry->type) {
...@@ -221,10 +220,6 @@ int sparse_keymap_setup(struct input_dev *dev, ...@@ -221,10 +220,6 @@ int sparse_keymap_setup(struct input_dev *dev,
dev->setkeycode = sparse_keymap_setkeycode; dev->setkeycode = sparse_keymap_setkeycode;
return 0; return 0;
err_out:
kfree(map);
return error;
} }
EXPORT_SYMBOL(sparse_keymap_setup); EXPORT_SYMBOL(sparse_keymap_setup);
...@@ -232,29 +227,13 @@ EXPORT_SYMBOL(sparse_keymap_setup); ...@@ -232,29 +227,13 @@ EXPORT_SYMBOL(sparse_keymap_setup);
* sparse_keymap_free - free memory allocated for sparse keymap * sparse_keymap_free - free memory allocated for sparse keymap
* @dev: Input device using sparse keymap * @dev: Input device using sparse keymap
* *
* This function is used to free memory allocated by sparse keymap * This function used to free memory allocated by sparse keymap
* in an input device that was set up by sparse_keymap_setup(). * in an input device that was set up by sparse_keymap_setup().
* NOTE: It is safe to cal this function while input device is * Since sparse_keymap_setup() now uses a managed allocation for the
* still registered (however the drivers should care not to try to * keymap copy, use of this function is deprecated.
* use freed keymap and thus have to shut off interrupts/polling
* before freeing the keymap).
*/ */
void sparse_keymap_free(struct input_dev *dev) void sparse_keymap_free(struct input_dev *dev)
{ {
unsigned long flags;
/*
* Take event lock to prevent racing with input_get_keycode()
* and input_set_keycode() if we are called while input device
* is still registered.
*/
spin_lock_irqsave(&dev->event_lock, flags);
kfree(dev->keycode);
dev->keycode = NULL;
dev->keycodemax = 0;
spin_unlock_irqrestore(&dev->event_lock, flags);
} }
EXPORT_SYMBOL(sparse_keymap_free); EXPORT_SYMBOL(sparse_keymap_free);
......
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