From patchwork Sun Oct 25 00:17:09 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dmitry Torokhov X-Patchwork-Id: 7481701 Return-Path: X-Original-To: patchwork-linux-input@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 33C5B9F2F7 for ; Sun, 25 Oct 2015 03:10:21 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id ED2AA207A0 for ; Sun, 25 Oct 2015 03:10:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 99BB42079C for ; Sun, 25 Oct 2015 03:10:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752571AbbJYDKR (ORCPT ); Sat, 24 Oct 2015 23:10:17 -0400 Received: from mail-qk0-f169.google.com ([209.85.220.169]:32786 "EHLO mail-qk0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752366AbbJYDKQ (ORCPT ); Sat, 24 Oct 2015 23:10:16 -0400 Received: by qkcy65 with SMTP id y65so97073671qkc.0 for ; Sat, 24 Oct 2015 20:10:15 -0700 (PDT) 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=q5x/McQFF/OHu/6rbTb/CJg96HAWdha3PWgm3vj9P4A=; b=UqK9QouARTSs6vNUbiv3SMaekIL2P7AT/9NWdgzYib0yRlXExOEcIO0j/B9Xac9Dtf t88r07YOPRQI+SZmjW8KIKAh+9Di4zWo6X5icPR5WbuMNlxqnxN4tRZd3+eZFu31XQ23 xwpl2CUBf0dYHxqBFvnU9HrxPABUCR5a5owqG1FJJ6NycHWK4A/TgAjGdqEr8oyUZj1u iF3Eogm+Ru9xqaMZpGSXAxjaiJu/SjrzUv0wF7Qdq7IXZ45iT4vMRT0CkbYBo40tM5kg 5eZPHoAyO/lo5GBoISVnkecmxtAizDh0xqAeQ/gK77ogapuWHzTnAxWY1AigSfEUu1uG 1Rzw== X-Received: by 10.55.209.205 with SMTP id o74mr34234443qkl.66.1445742615513; Sat, 24 Oct 2015 20:10:15 -0700 (PDT) Received: from localhost ([64.88.227.134]) by smtp.gmail.com with ESMTPSA id 187sm2777028qhd.40.2015.10.24.20.09.38 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Sat, 24 Oct 2015 20:10:14 -0700 (PDT) Date: Sat, 24 Oct 2015 17:17:09 -0700 From: Dmitry Torokhov To: David Herrmann Cc: linux-input@vger.kernel.org, Benjamin Tissoires , Peter Hutterer Subject: Re: [PATCH] Input: evdev - add event-mask API Message-ID: <20151025001709.GA27533@dtor-pixel> References: <1441296841-16894-1-git-send-email-dh.herrmann@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1441296841-16894-1-git-send-email-dh.herrmann@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 Hi David, On Thu, Sep 03, 2015 at 06:14:01PM +0200, David Herrmann wrote: > +static int bits_from_user(unsigned long *bits, unsigned int maxbit, > + unsigned int maxlen, const void __user *p, int compat) > +{ > + int len; > + > +#if IS_ENABLED(CONFIG_COMPAT) > + if (compat) { > + if (maxlen % sizeof(compat_long_t)) > + return -EINVAL; > + len = BITS_TO_LONGS_COMPAT(maxbit) * sizeof(compat_long_t); > + } else > +#endif > + { > + if (maxlen % sizeof(long)) > + return -EINVAL; > + len = BITS_TO_LONGS(maxbit) * sizeof(long); > + } > + > + if (len > maxlen) > + len = maxlen; > + > +#if IS_ENABLED(CONFIG_COMPAT) && defined(__BIG_ENDIAN) > + if (compat) { > + int i; > + > + for (i = 0; i < len / sizeof(compat_long_t); i++) > + if (copy_from_user((compat_long_t *) bits + > + i + 1 - ((i % 2) << 1), > + (compat_long_t __user *) p + i, > + sizeof(compat_long_t))) > + return -EFAULT; > + if (i % 2) > + *((compat_long_t *) bits + i - 1) = 0; > + } else > +#endif > + if (copy_from_user(bits, p, len)) > + return -EFAULT; > + > + return len; > +} I do not quite like how we sprinkle ifdefs throughout, I prefer the way we have bits_to_user defined, even if it is more verbose. > + > static int str_to_user(const char *str, unsigned int maxlen, void __user *p) > { > int len; > @@ -854,6 +953,86 @@ static int evdev_revoke(struct evdev *evdev, struct evdev_client *client, > return 0; > } > > +/* must be called with evdev-mutex held */ > +static int evdev_set_mask(struct evdev_client *client, > + unsigned int type, > + const void __user *codes, > + u32 codes_size, > + int compat) > +{ > + unsigned long flags, *mask, *oldmask; > + size_t cnt, size, min; > + int error; > + > + /* we allow unknown types and 'codes_size > size' for forward-compat */ > + cnt = evdev_get_mask_cnt(type); > + if (!cnt) > + return 0; > + > + size = sizeof(unsigned long) * BITS_TO_LONGS(cnt); > + min = min_t(size_t, codes_size, size); > + > + mask = kzalloc(size, GFP_KERNEL); > + if (!mask) > + return -ENOMEM; > + > + error = bits_from_user(mask, cnt - 1, min, codes, compat); I do not think we need to calculate and pass min here: bits_from_user() will limit the output for us already. Does it still work if you apply the patch below? Thanks! diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c index 83d699f..ce35ea3 100644 --- a/drivers/input/evdev.c +++ b/drivers/input/evdev.c @@ -685,7 +685,46 @@ static int bits_to_user(unsigned long *bits, unsigned int maxbit, return len; } + +static int bits_from_user(unsigned long *bits, unsigned int maxbit, + unsigned int maxlen, const void __user *p, int compat) +{ + int len, i; + + if (compat) { + if (maxlen % sizeof(compat_long_t)) + return -EINVAL; + + len = BITS_TO_LONGS_COMPAT(maxbit) * sizeof(compat_long_t); + if (len > maxlen) + len = maxlen; + + for (i = 0; i < len / sizeof(compat_long_t); i++) + if (copy_from_user((compat_long_t *) bits + + i + 1 - ((i % 2) << 1), + (compat_long_t __user *) p + i, + sizeof(compat_long_t))) + return -EFAULT; + if (i % 2) + *((compat_long_t *) bits + i - 1) = 0; + + } else { + if (maxlen % sizeof(long)) + return -EINVAL; + + len = BITS_TO_LONGS(maxbit) * sizeof(long); + if (len > maxlen) + len = maxlen; + + if (copy_from_user(p, bits, len)) + return -EFAULT; + } + + return len; +} + #else + static int bits_to_user(unsigned long *bits, unsigned int maxbit, unsigned int maxlen, void __user *p, int compat) { @@ -698,6 +737,24 @@ static int bits_to_user(unsigned long *bits, unsigned int maxbit, return copy_to_user(p, bits, len) ? -EFAULT : len; } + +static int bits_from_user(unsigned long *bits, unsigned int maxbit, + unsigned int maxlen, const void __user *p, int compat) +{ + size_t chunk_size = compat ? sizeof(compat_long_t) : sizeof(long); + int len; + + if (maxlen % chunk_size) + return -EINVAL; + + len = compat ? BITS_TO_LONGS_COMPAT(maxbit) : BITS_TO_LONGS(maxbit); + len *= chunk_size; + if (len > maxlen) + len = maxlen; + + return copy_from_user(bits, p, len) ? -EFAULT : len; +} + #endif /* __BIG_ENDIAN */ #else @@ -713,49 +770,23 @@ static int bits_to_user(unsigned long *bits, unsigned int maxbit, return copy_to_user(p, bits, len) ? -EFAULT : len; } -#endif /* CONFIG_COMPAT */ - static int bits_from_user(unsigned long *bits, unsigned int maxbit, unsigned int maxlen, const void __user *p, int compat) { int len; -#if IS_ENABLED(CONFIG_COMPAT) - if (compat) { - if (maxlen % sizeof(compat_long_t)) - return -EINVAL; - len = BITS_TO_LONGS_COMPAT(maxbit) * sizeof(compat_long_t); - } else -#endif - { - if (maxlen % sizeof(long)) - return -EINVAL; - len = BITS_TO_LONGS(maxbit) * sizeof(long); - } + if (maxlen % sizeof(long)) + return -EINVAL; + len = BITS_TO_LONGS(maxbit) * sizeof(long); if (len > maxlen) len = maxlen; -#if IS_ENABLED(CONFIG_COMPAT) && defined(__BIG_ENDIAN) - if (compat) { - int i; - - for (i = 0; i < len / sizeof(compat_long_t); i++) - if (copy_from_user((compat_long_t *) bits + - i + 1 - ((i % 2) << 1), - (compat_long_t __user *) p + i, - sizeof(compat_long_t))) - return -EFAULT; - if (i % 2) - *((compat_long_t *) bits + i - 1) = 0; - } else -#endif - if (copy_from_user(bits, p, len)) - return -EFAULT; - - return len; + return copy_from_user(bits, p, len) ? -EFAULT : len; } +#endif /* CONFIG_COMPAT */ + static int str_to_user(const char *str, unsigned int maxlen, void __user *p) { int len; @@ -956,7 +987,7 @@ static int evdev_set_mask(struct evdev_client *client, int compat) { unsigned long flags, *mask, *oldmask; - size_t cnt, size, min; + size_t cnt; int error; /* we allow unknown types and 'codes_size > size' for forward-compat */ @@ -964,14 +995,11 @@ static int evdev_set_mask(struct evdev_client *client, if (!cnt) return 0; - size = sizeof(unsigned long) * BITS_TO_LONGS(cnt); - min = min_t(size_t, codes_size, size); - - mask = kzalloc(size, GFP_KERNEL); + mask = kcalloc(sizeof(unsigned long), BITS_TO_LONGS(cnt), GFP_KERNEL); if (!mask) return -ENOMEM; - error = bits_from_user(mask, cnt - 1, min, codes, compat); + error = bits_from_user(mask, cnt - 1, codes_size, codes, compat); if (error < 0) { kfree(mask); return error; @@ -995,35 +1023,33 @@ static int evdev_get_mask(struct evdev_client *client, int compat) { unsigned long *mask; - size_t cnt, size, min, i; - u8 __user *out; + size_t cnt, size, xfer_size; + int i; int error; /* we allow unknown types and 'codes_size > size' for forward-compat */ cnt = evdev_get_mask_cnt(type); size = sizeof(unsigned long) * BITS_TO_LONGS(cnt); - min = min_t(size_t, codes_size, size); + xfer_size = min_t(size_t, codes_size, size); if (cnt > 0) { mask = client->evmasks[type]; if (mask) { - error = bits_to_user(mask, cnt - 1, min, codes, compat); + error = bits_to_user(mask, cnt - 1, + xfer_size, codes, compat); if (error < 0) return error; } else { /* fake mask with all bits set */ - out = (u8 __user*)codes; - for (i = 0; i < min; ++i) - if (put_user((u8)0xff, out + i)) + for (i = 0; i < xfer_size; i++) + if (put_user(0xffU, (u8 __user *)codes + i)) return -EFAULT; } } - codes = (u8*)codes + min; - codes_size -= min; - - if (codes_size > 0 && clear_user(codes, codes_size)) - return -EFAULT; + if (xfer_size < codes_size) + if (clear_user(codes + xfer_size, codes_size - xfer_size)) + return -EFAULT; return 0; } @@ -1097,27 +1123,29 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd, else return evdev_revoke(evdev, client, file); - case EVIOCGMASK: + case EVIOCGMASK: { + void __user *codes_ptr; + if (copy_from_user(&mask, p, sizeof(mask))) return -EFAULT; + codes_ptr = (void __user *)(unsigned long)mask.codes_ptr; return evdev_get_mask(client, - mask.type, - (void __user*) - (unsigned long)mask.codes_ptr, - mask.codes_size, + mask.type, codes_ptr, mask.codes_size, compat_mode); + } + + case EVIOCSMASK: { + const void __user *codes_ptr; - case EVIOCSMASK: if (copy_from_user(&mask, p, sizeof(mask))) return -EFAULT; + codes_ptr = (const void __user *)(unsigned long)mask.codes_ptr; return evdev_set_mask(client, - mask.type, - (const void __user*) - (unsigned long)mask.codes_ptr, - mask.codes_size, + mask.type, codes_ptr, mask.codes_size, compat_mode); + } case EVIOCSCLOCKID: if (copy_from_user(&i, p, sizeof(unsigned int)))