From patchwork Wed Mar 8 08:22:17 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TWljaGHFgiBLxJlwaWXFhA==?= X-Patchwork-Id: 9610527 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 98E4F6046A for ; Wed, 8 Mar 2017 08:22:27 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 98C1225D99 for ; Wed, 8 Mar 2017 08:22:27 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8D71926E3A; Wed, 8 Mar 2017 08:22:27 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 78D9F25D99 for ; Wed, 8 Mar 2017 08:22:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751340AbdCHIWZ (ORCPT ); Wed, 8 Mar 2017 03:22:25 -0500 Received: from mail-lf0-f65.google.com ([209.85.215.65]:32779 "EHLO mail-lf0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750770AbdCHIWY (ORCPT ); Wed, 8 Mar 2017 03:22:24 -0500 Received: by mail-lf0-f65.google.com with SMTP id r36so1835467lfi.0 for ; Wed, 08 Mar 2017 00:22:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kempniu.pl; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=nAX3nHEiRpDckUgoLXgSAcFoEbTCRKXU56Kj9NU6Bfg=; b=eZAxYiuFL+J1Tpp1kfmwGEeZESpPEGKY0hAvd953+Ko7+SSNUKdV7MlecB0uBLB4bj ZvFedEm7Oe5pLsFyMyW1On5uTZgUdcBCnCCSipgDc4o9uCuChZOIVJ+kyuW5F6cVHkYV ca6y38Qh7aveU1cXjXEVe7Oe7kyoLAB8xM2BY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=nAX3nHEiRpDckUgoLXgSAcFoEbTCRKXU56Kj9NU6Bfg=; b=rjc897BusTjhxFgypWwm/8cQ5V64RJQxFa4DjPZcK0IFxMnvD4YPW6kreomI5ULH8v qkNXhgK1lKNZ2pxI6UrxDtkBECoUPyOI9ycgEMqicdPQQ0TrNgbBpE5EUkvD7tki37G/ Tq6OubpBBYLjKWWzmJY6EOKINqYAacET5XGbVdkIzfeOhFJZ4evfOMH0HUCtiUBffcBS /KygST8YAKL83u0UNj5tvvA0MC13qVHVimXuAFnnpO6LPyhK26SLVyZyZPjr+dX4bk2S xTY9lLEm1hrFDEItiuWwq7LU/3Ea6PG2snmO/rDq8GnsaQDBL4lPF9PpXQTcaZH1YOeK DGpQ== X-Gm-Message-State: AMke39mS9OKXt3mH8OF5k79RYk3600BXmyOaZ0x+vgSXO5FsNZoqk9208CE6W60B75TIuQ== X-Received: by 10.46.84.73 with SMTP id y9mr1682508ljd.6.1488961341611; Wed, 08 Mar 2017 00:22:21 -0800 (PST) Received: from ozzy.nask.waw.pl ([2001:a10:160:3::3]) by smtp.googlemail.com with ESMTPSA id s24sm490577ljd.6.2017.03.08.00.22.20 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 08 Mar 2017 00:22:21 -0800 (PST) From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= To: Dmitry Torokhov Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v3] Input: sparse-keymap - use a managed allocation for keymap copy Date: Wed, 8 Mar 2017 09:22:17 +0100 Message-Id: <20170308082217.5466-1-kernel@kempniu.pl> X-Mailer: git-send-email 2.12.0 MIME-Version: 1.0 Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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: Michał Kępień --- Changes from v2: - Always use the input device as the owner of the managed keymap copy, no matter whether that input device is itself managed or non-managed. - Use devm_kmemdup() instead of devm_kcalloc() to avoid a separate call to memcpy(). - Update commit message to reflect the above changes. Changes from v1: - Do not add a new function. Instead, make sparse_keymap_setup() always use a managed allocation, which allows making sparse_keymap_free() a noop and simplifies error handling. - Update subject, commit message and comments accordingly. drivers/input/sparse-keymap.c | 39 +++++++++------------------------------ 1 file changed, 9 insertions(+), 30 deletions(-) diff --git a/drivers/input/sparse-keymap.c b/drivers/input/sparse-keymap.c index e7409c45bdd0..12a3ad83296d 100644 --- a/drivers/input/sparse-keymap.c +++ b/drivers/input/sparse-keymap.c @@ -160,12 +160,12 @@ static int sparse_keymap_setkeycode(struct input_dev *dev, * @keymap: Keymap in form of array of &key_entry structures ending * with %KE_END type entry * @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 * keymap after which sets up input device event bits appropriately. - * Before destroying input device allocated keymap should be freed - * with a call to sparse_keymap_free(). + * The allocated copy of the keymap is automatically freed when it + * is no longer needed. */ int sparse_keymap_setup(struct input_dev *dev, const struct key_entry *keymap, @@ -180,19 +180,18 @@ int sparse_keymap_setup(struct input_dev *dev, for (e = keymap; e->type != KE_END; e++) 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) return -ENOMEM; - memcpy(map, keymap, map_size * sizeof(struct key_entry)); - for (i = 0; i < map_size; i++) { entry = &map[i]; if (setup) { error = setup(dev, entry); if (error) - goto err_out; + return error; } switch (entry->type) { @@ -221,10 +220,6 @@ int sparse_keymap_setup(struct input_dev *dev, dev->setkeycode = sparse_keymap_setkeycode; return 0; - - err_out: - kfree(map); - return error; } EXPORT_SYMBOL(sparse_keymap_setup); @@ -232,29 +227,13 @@ EXPORT_SYMBOL(sparse_keymap_setup); * sparse_keymap_free - free memory allocated for 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(). - * NOTE: It is safe to cal this function while input device is - * still registered (however the drivers should care not to try to - * use freed keymap and thus have to shut off interrupts/polling - * before freeing the keymap). + * Since sparse_keymap_setup() now uses a managed allocation for the + * keymap copy, use of this function is deprecated. */ 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);