From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752995AbcHZNbQ (ORCPT ); Fri, 26 Aug 2016 09:31:16 -0400 Received: from mout.web.de ([212.227.17.11]:51393 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750715AbcHZNbP (ORCPT ); Fri, 26 Aug 2016 09:31:15 -0400 Subject: Re: [PATCH] arm: sys_oabi-compat: Use kmalloc_array() in two functions To: Russell King References: <5deba521-2dd4-cd2e-23d7-226a1a7728c3@users.sourceforge.net> <20160825224632.GL1041@n2100.armlinux.org.uk> Cc: linux-arm-kernel@lists.infradead.org, Al Viro , Dave Weinstein , Jeff Layton , Kees Cook , Nicolas Pitre , LKML , kernel-janitors@vger.kernel.org, Julia Lawall , Paolo Bonzini From: SF Markus Elfring Message-ID: Date: Fri, 26 Aug 2016 15:22:29 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2 MIME-Version: 1.0 In-Reply-To: <20160825224632.GL1041@n2100.armlinux.org.uk> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Provags-ID: V03:K0:MA1ktbBT9QC1UB0oDJdFwaQMZPeSiUA+BMnKSz0iYyCF6zPM+f6 jvHzb1dLW4CYDk6f7sXMwcA8DPFugGsx5JUEI5fhS6yQVjo1ZEalOou+3wVqrkLX/Ikc6I1 okYL9ksyCAk+8rI8zBKUtu4YSJFtb3k53Qzc0XMhVgia7tvK0hqo3tVz8vPq0gniM8rf+vA c287LumgfIvm/2oPYXq8w== X-UI-Out-Filterresults: notjunk:1;V01:K0:KczPWSJW7N4=:1xYHd/Iq+Z0m7PnVcs2oML XHZm/orbxkXDFlPmtFJZ5pTqpwHE8ywbOOFjHIXaOfBPVXagLpQyKEcoo11TxSMjbrUQl5jRE Z9tcOx0eckMZlMOSgXfFSawBLVu5OXfRDwNeWmgr90s80ovYEaJ3ENsRshan9QznvZ62y1ps2 Vd8wkFJ6ecxXWvtE9obCj36y9BwenZx8qv2bL3FsiadAqBKDsik/XRqzwV7UzlvPfbypXMFtz CZxmbKmMJv84ExbOYF73ffsIjj5TWOr1Uxax1fv9Z7avMLveXQD9a4e4kzcc8H7bfAvS2XfAd ypHD8XfO8JYoXD/G7tA2LqorbIHG+uRBjtX+aQOYqCkiJdK6/gW99FeANr+CvhE0zLH6uz/KP qrLIVD6VKTYH0D887GjMn1k2w+5AV/2X5sytPCH4O5SeYQDsQc+vfKr+VpEiZwOW1NquddA4Y pe0leRIsyI0hYPlvLCJP4jWGhEj5uhIOMhroMK0odkOOwUiGYFEXacnOvVbdvuEQYeCNP05vc FdHMMHpAy9JleUf6dLeRo0meiy5x1gK4lrPaLrnhuHfqJ6fD4rOKA6NRfe7Kyvx5gwC5XrxvH ONawIU48jnpkk7HUcLvi4fiViHczVJ9brJ9ry7A9geN++HerJG2is2ySHpVwa3sdty5WTeYc4 D5mqKmICnBeavIXRxTjRvlB3vtAsbcV4i64dH21x5cxKg1hglOvGDJAzRlChqtbPFchd2/aXi z+zZlIOWEgA8d7qp5IN36vLx0WTrSpm3CUZ7+ACmz5LwGACK77MLfL8O09+n8oSx2o2CEzw4+ smQOP7C Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org >> Multiplications for the size determination of memory allocations >> indicated that array data structures should be processed. > > I'm afraid the above comment doesn't mean much to me, can you rephrase? Yes, of course. How verbose should the explanation for this update suggestion become? > Maybe: > > "Multiplications for kmalloc size arguments are liable to overflow, > potentially causing a potential security issue. Using kmalloc_array() > allows the overflow to be caught and the allocation failed. Switch > these callsites to kmalloc_array()." Thanks for your wording variant. >> @@ -285,7 +285,7 @@ asmlinkage long sys_oabi_epoll_wait(int epfd, >> return -EINVAL; >> if (!access_ok(VERIFY_WRITE, events, sizeof(*events) * maxevents)) >> return -EFAULT; >> - kbuf = kmalloc(sizeof(*kbuf) * maxevents, GFP_KERNEL); >> + kbuf = kmalloc_array(maxevents, sizeof(*kbuf), GFP_KERNEL); > > kmalloc_array() here actually buys us no additional safety at either > of the callsites in your patch Can this inline function apply a few sanity checks in a consistent way? http://lxr.free-electrons.com/source/include/linux/slab.h#L564 > - we need to have carefully checked the values to ensure > they don't overflow prior to the kmalloc for other reasons. Are there any more constraints to consider? > That's probably something that should be noted in the commit message too, > so reviewers have the confidence that you're not blindly changing everything... I imagine that a few contributors can get mixed feelings from a bunch of my recent patches. There is a significant patch number in the works for various Linux software modules. Regards, Markus