From patchwork Wed Jan 6 22:38:45 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dmitry Torokhov X-Patchwork-Id: 7971701 Return-Path: X-Original-To: patchwork-linux-input@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 4DF32BEEE5 for ; Wed, 6 Jan 2016 22:38:53 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 25E0520154 for ; Wed, 6 Jan 2016 22:38:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 13D3920142 for ; Wed, 6 Jan 2016 22:38:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752166AbcAFWiu (ORCPT ); Wed, 6 Jan 2016 17:38:50 -0500 Received: from mail-pa0-f49.google.com ([209.85.220.49]:35594 "EHLO mail-pa0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752091AbcAFWit (ORCPT ); Wed, 6 Jan 2016 17:38:49 -0500 Received: by mail-pa0-f49.google.com with SMTP id do7so4307711pab.2; Wed, 06 Jan 2016 14:38:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=MuquBrgTBRJ6qjXT44ZQvPOoIonKgi4BloIm4CH+wsw=; b=rQH8av/WuJUEK4s2pQIRRbr40LZ6Aox75zI5AlikfYXdkl0F+UOmAxhMYtkhowHReu bm4rVdXT5rXAgCUJf8FE3CtPgBtDHphpoNi1B/aLqaUz0xHBlUwD3arQfNFjEpFBnGKO cS2C21yz4AQQUUNh25/cvmgUVE0tfq7vvgbIBS6tcan5yQBMWtysHf/DJAbQRQnbAI5E BJxHr0GOK3D+tlxyzVWqIDlnKuFfHDqnKtDEIZRM3LtzVAyMkbPAZ1vIkjdbArStnByt GkoBYmZjGOhvA2sPdLFeN+daWlD6yy+dKSmMoEnHgJpbwauJq8MT4OejDTOyyeLEm09Q VYwA== X-Received: by 10.66.101.36 with SMTP id fd4mr147457695pab.76.1452119928598; Wed, 06 Jan 2016 14:38:48 -0800 (PST) Received: from dtor-ws ([2620:0:1000:1301:ac2b:3665:f5db:75dd]) by smtp.gmail.com with ESMTPSA id tu9sm143778165pac.0.2016.01.06.14.38.46 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Wed, 06 Jan 2016 14:38:47 -0800 (PST) Date: Wed, 6 Jan 2016 14:38:45 -0800 From: Dmitry Torokhov To: Ivaylo Dimitrov Cc: pali.rohar@gmail.com, linux-input@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] input: gpio_keys: Fix check for disabling unsupported key Message-ID: <20160106223845.GA26606@dtor-ws> References: <1451856076-29045-1-git-send-email-ivo.g.dimitrov.75@gmail.com> <20160105011942.GD11801@dtor-ws> <568B6FB8.6000101@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <568B6FB8.6000101@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Jan 05, 2016 at 09:24:40AM +0200, Ivaylo Dimitrov wrote: > 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? How about the patch below (compiled but not tested)? Thanks. Tested-by: Ivaylo Dimitrov diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c index bef317f..b9f01bd 100644 --- a/drivers/input/keyboard/gpio_keys.c +++ b/drivers/input/keyboard/gpio_keys.c @@ -96,7 +96,7 @@ struct gpio_keys_drvdata { * Return value of this function can be used to allocate bitmap * large enough to hold all bits for given type. */ -static inline int get_n_events_by_type(int type) +static int get_n_events_by_type(int type) { BUG_ON(type != EV_SW && type != EV_KEY); @@ -104,6 +104,22 @@ static inline int get_n_events_by_type(int type) } /** + * get_bm_events_by_type() - returns bitmap of supported events per @type + * @input: input device from which bitmap is retrieved + * @type: type of button (%EV_KEY, %EV_SW) + * + * Return value of this function can be used to allocate bitmap + * large enough to hold all bits for given type. + */ +static const unsigned long *get_bm_events_by_type(struct input_dev *dev, + int type) +{ + BUG_ON(type != EV_SW && type != EV_KEY); + + return (type == EV_KEY) ? dev->keybit : dev->swbit; +} + +/** * gpio_keys_disable_button() - disables given GPIO button * @bdata: button data for button to be disabled * @@ -213,6 +229,7 @@ static ssize_t gpio_keys_attr_store_helper(struct gpio_keys_drvdata *ddata, const char *buf, unsigned int type) { int n_events = get_n_events_by_type(type); + const unsigned long *bitmap = get_bm_events_by_type(ddata->input, type); unsigned long *bits; ssize_t error; int i; @@ -226,6 +243,11 @@ static ssize_t gpio_keys_attr_store_helper(struct gpio_keys_drvdata *ddata, goto out; /* First validate */ + if (!bitmap_subset(bits, bitmap, n_events)) { + error = -EINVAL; + goto out; + } + for (i = 0; i < ddata->pdata->nbuttons; i++) { struct gpio_button_data *bdata = &ddata->data[i]; @@ -239,11 +261,6 @@ static ssize_t gpio_keys_attr_store_helper(struct gpio_keys_drvdata *ddata, } } - if (i == ddata->pdata->nbuttons) { - error = -EINVAL; - goto out; - } - mutex_lock(&ddata->disable_lock); for (i = 0; i < ddata->pdata->nbuttons; i++) {