| Message ID | 1451856076-29045-1-git-send-email-ivo.g.dimitrov.75@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Sun, Jan 03, 2016 at 11:21:16PM +0200, Ivaylo Dimitrov wrote: > Commit 4ea14a53d8f881034fa9e186653821c4e3d9a8fb ("Input: gpio-keys - > report error when disabling unsupported key") tried to prevent an > unsupported key to disabled. Unfortunately it effectively broke the driver > in a way so no key is possible to be disabled. Fix that by providing the > correct verify logic. Hmm, indeed... > > Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> > --- > drivers/input/keyboard/gpio_keys.c | 28 +++++++++++++++++++--------- > 1 file changed, 19 insertions(+), 9 deletions(-) > > diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c > index bef317f..a371805 100644 > --- a/drivers/input/keyboard/gpio_keys.c > +++ b/drivers/input/keyboard/gpio_keys.c > @@ -226,22 +226,32 @@ static ssize_t gpio_keys_attr_store_helper(struct gpio_keys_drvdata *ddata, > goto out; > > /* First validate */ > - for (i = 0; i < ddata->pdata->nbuttons; i++) { > - struct gpio_button_data *bdata = &ddata->data[i]; > + for (i = 0; i < n_events; i++) { for_each_set_bit()? OTOH maybe we should do bitmap = get_bitmap_events_by_type(type); // new, return keybit or swbit if (!bitmap_subset(bits, bitmap, n_events)) { error = -EINVAL; goto out; } ... and leave the rest of the loop as is? Thanks.
Hi, On 5.01.2016 03:19, Dmitry Torokhov wrote: >> /* First validate */ >> - for (i = 0; i < ddata->pdata->nbuttons; i++) { >> - struct gpio_button_data *bdata = &ddata->data[i]; >> + for (i = 0; i < n_events; i++) { > > for_each_set_bit()? Yeah, seems I must have overslept that helper, will send an updated version. > > OTOH maybe we should do > > bitmap = get_bitmap_events_by_type(type); // new, return keybit or swbit new helper function? or static function in gpio-keys? who allocates/frees the bitmap memory? or this is static data? Maybe I don't get the idea :) . > if (!bitmap_subset(bits, bitmap, n_events)) { > error = -EINVAL; > goto out; > } > > ... and leave the rest of the loop as is? > Not sure about that. Unless I miss something, what we want is: 1. make sure that what user has written is within the range of the event type. I hope bitmap_parselist already does it for us. 2. Make sure that for every bit in bits set based on what user has provided, there is a matching gpio in this particular gpio-keys device. 3. Make sure that every gpio user wants disabled is actually allowed to be disabled. I don't see how 2 is achieved with ^^^ code. So, shall I send a new version of the patch with for_each_set_bit() used, or you'll fix the $subject problem with whatever magic you think is needed? Thanks, Ivo -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c index bef317f..a371805 100644 --- a/drivers/input/keyboard/gpio_keys.c +++ b/drivers/input/keyboard/gpio_keys.c @@ -226,22 +226,32 @@ static ssize_t gpio_keys_attr_store_helper(struct gpio_keys_drvdata *ddata, goto out; /* First validate */ - for (i = 0; i < ddata->pdata->nbuttons; i++) { - struct gpio_button_data *bdata = &ddata->data[i]; + for (i = 0; i < n_events; i++) { + int j; - if (bdata->button->type != type) + if (!test_bit(i, bits)) continue; - if (test_bit(bdata->button->code, bits) && - !bdata->button->can_disable) { + for (j = 0; j < ddata->pdata->nbuttons; j++) { + struct gpio_button_data *bdata = &ddata->data[j]; + + if (bdata->button->type != type) + continue; + + if (bdata->button->code == i) { + if (!bdata->button->can_disable) { + error = -EINVAL; + goto out; + } + break; + } + } + + if (j == ddata->pdata->nbuttons) { error = -EINVAL; goto out; } - } - if (i == ddata->pdata->nbuttons) { - error = -EINVAL; - goto out; } mutex_lock(&ddata->disable_lock);
Commit 4ea14a53d8f881034fa9e186653821c4e3d9a8fb ("Input: gpio-keys - report error when disabling unsupported key") tried to prevent an unsupported key to disabled. Unfortunately it effectively broke the driver in a way so no key is possible to be disabled. Fix that by providing the correct verify logic. Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> --- drivers/input/keyboard/gpio_keys.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-)