From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C0344C433E1 for ; Thu, 30 Jul 2020 09:18:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A5C4220672 for ; Thu, 30 Jul 2020 09:18:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729344AbgG3JSZ (ORCPT ); Thu, 30 Jul 2020 05:18:25 -0400 Received: from mout.kundenserver.de ([212.227.17.10]:36165 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726819AbgG3JSY (ORCPT ); Thu, 30 Jul 2020 05:18:24 -0400 Received: from mail-qt1-f170.google.com ([209.85.160.170]) by mrelayeu.kundenserver.de (mreue107 [212.227.15.145]) with ESMTPSA (Nemesis) id 1MadGG-1kbnQi0lz8-00c9c4; Thu, 30 Jul 2020 11:18:22 +0200 Received: by mail-qt1-f170.google.com with SMTP id s23so19782055qtq.12; Thu, 30 Jul 2020 02:18:21 -0700 (PDT) X-Gm-Message-State: AOAM533p0yK1gTQ9QUiK587ON7jYuaun+ALSI7zan6ney44yqsTYO7HG osekkEzZS57fSxxW8pkJla/XyRw0Ewjl0OQ4Ve8= X-Google-Smtp-Source: ABdhPJxTFkRhqtuLhXAsNUlZ9J0TZfkHL99w5pWpprWMX0g7dJ5QSuxxz8xtNYzXVB7FSGpCxdmyitm5GsMtlvmucAc= X-Received: by 2002:aed:33e7:: with SMTP id v94mr1965217qtd.18.1596100700959; Thu, 30 Jul 2020 02:18:20 -0700 (PDT) MIME-Version: 1.0 References: <20200726220557.102300-1-yepeilin.cs@gmail.com> <20200726222703.102701-1-yepeilin.cs@gmail.com> <20200727131608.GD1913@kadam> <20200728130632.GI1913@kadam> <20200730083833.GD3703480@smile.fi.intel.com> In-Reply-To: <20200730083833.GD3703480@smile.fi.intel.com> From: Arnd Bergmann Date: Thu, 30 Jul 2020 11:18:04 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [Linux-kernel-mentees] [PATCH v3] media/v4l2-core: Fix kernel-infoleak in video_put_user() To: Andy Shevchenko Cc: Bartosz Golaszewski , Dan Carpenter , Linus Walleij , Peilin Ye , Mauro Carvalho Chehab , Greg Kroah-Hartman , syzkaller-bugs , Hans Verkuil , Sakari Ailus , Laurent Pinchart , Vandana BN , Ezequiel Garcia , =?UTF-8?Q?Niklas_S=C3=B6derlund?= , linux-kernel-mentees@lists.linuxfoundation.org, Linux Media Mailing List , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset="UTF-8" X-Provags-ID: V03:K1:SnTdMroix1s5FQaL0KTdz3IDTPnV5/yB9vXvjJw3mf0wp298IeX AOfGeYooFp6ID9lexXvpqcu/i1mrWTvyhDpMLb/CPbcITlETeCgkvKoF/1RJYAH9cEPg8mD uZKJGSh31C/z0/DgUrnqXw8rG/gNauCEkZe2lJYcT37EUFx4oDjOHyxxfaPRdrW7xeQaYx8 LGeyEI/V3dSdG3q3FN/bg== X-UI-Out-Filterresults: notjunk:1;V03:K0:FC+afytnCgs=:Ss3/fHQWDIcTLfLQrklC7S MHk2++ryNfDpGZf0O5DTDHr7sQmEqNJVWds6+F0sCCDuRVVoNQ8QjUPEQKykkVbFi1lCOzB/I aBXyebQhVubFbh13mVDorVsrvQ7ZrRTBkqa4A5g3svu289cf8X+KQDX45ndG6aNrCO35cGYa+ FQTz3rnVsOCo1uuuYb3I6aeVskaauijS3bDMTLPsMCEHgLCcKfxRLB+NuxblJMK+KoqZ2NHb1 V+hyhJC15wMjhpSUO232Llb8foTYIiScN16xUaPCQwUKpdQd1k03A2SDM5T/qmPZbUihrYb0m /HwQ+YeGOc0uTUs6J+l+UoVodfXVr558TJ4/ilP32BdRO26g8+hUthwg6R9NjaayYsqPtyH9x MW8JmPjXLtmVZUsktVYQ7j8Q9EYLa8Y2JSM02FvVDaPyvydnt2it3rJwOQAh5Bn6TjQKvYmA9 rU1Q6Pumz+qMNA/eC3zurIEFm0PUGnSjUUJeZBPnlO2BxJvfaAwopncIkXjOnwrLA725kJO/i /ovbFQt94yDgv/2QFeCNNqNQL+ZJQD0Ik2yKxLCKqvwtJMJQEyWcuNcuiCCkYmcw1pJbuLk82 S/1B5xoGQM3/3V/swX4MOmj3uixcMgCHNNSfzXL+TRBoFfAz5GEWqKf4QnNmgVhhll3BgWota NlKfg5ecZTJv69FtnaTpWM3QdPK2vc1GaeKlcwHtGNB9371cg8f9I7dDlBRwHMOuIQejXVXy6 99dkMzzUlQitMD8fFmtodY/xRgGvZ1lCw2gYtFJ6pUgepmicXWY++Ln2zAOoy71rhNliX+Acf utgjv05/ugpxvyUXsmTOYSlMn8OY0X4kQN/u/i5xGFFgLHNzWsdf18E22tI6j8P5+JCxKqQzg Fo/cIDBOlW0ERHDvlmQInhXVjnmYlFb10Rz6vM4ahu6n1D8YU6CUb6ZzisGlCsk/N8v+0gZc2 2/l8uQSdcfkRLQKOz5RjusM9RxHl+9EmqzaPPDOVITVoAyXD9D7Cy Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jul 30, 2020 at 10:38 AM Andy Shevchenko wrote: > On Thu, Jul 30, 2020 at 10:15:24AM +0200, Arnd Bergmann wrote: > > On Thu, Jul 30, 2020 at 10:07 AM Bartosz Golaszewski wrote: > > > > > > On Tue, Jul 28, 2020 at 3:58 PM Arnd Bergmann wrote: > > > > > > > > On Tue, Jul 28, 2020 at 3:06 PM Dan Carpenter wrote: > > > > Something like > > > > > > > > static int lineevent_put_data(void __user *uptr, struct gpioevent_data *ge) > > > > { > > > > #ifdef __x86_64__ > > > > /* i386 has no padding after 'id' */ > > > > if (in_ia32_syscall()) { > > > > struct { > > > > compat_u64 timestamp __packed; > > > > u32 id; > > > > } compat_ge = { ge->timestamp, ge->id }; > > > > > > > > if (copy_to_user(uptr, &compat_ge, sizeof(compat_ge))) > > > > return -EFAULT; > > > > > > > > return sizeof(compat_ge); > > > > } > > > > #endif > > > > > > > > if (copy_to_user(uptr, ge, sizeof(*ge)) > > > > return -EFAULT; > > > > > > > > return sizeof(*ge); > > > > } > > > > > > > > Arnd > > > > > > Hi Arnd, > > > > > > Andy actually had a patch for that but since this isn't a regression > > > (it never worked), we decided to leave it as it is and get it right in > > > v2 API. > > > > I would argue that it needs to be fixed anyway, unless you also want > > to remove the v1 interface for native mode. If this works on 32-bit > > kernels, on 64-bit kernels with 64-bit user space and on compat > > 32-bit user space on 64-bit non-x86 architectures, I see no reason > > to leave it broken specifically on x86 compat user space. There are > > still reasons to use 32-bit x86 user space for low-memory machines > > even though native i386 kernels are getting increasingly silly. > > It was possible to "fix" (mitigate to some extent) before libgpiod got support > for several events in a request. Now it seems to be impossible to fix. AFAIU we > must discard any request to more than one event in it. Any reason why the workaround I suggested above would not work? The in_ia32_syscall() check should be completely reliable in telling whether we are called from read() by an ia32 task or not, and we use the same logic for input_event, which has a similar problem (on all compat architectures, not just x86). > However I'm not an expert in compat IOCTL code (you are :-) and perhaps you may > provide ideas better than mine. What makes this interface tricky is that this is actually a read() call, not ioctl() which is usually easier because it encodes the data length in the command code. As far as I could tell from skimming the interface, the ioctls are actually fine here. Arnd From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CF1ACC433E0 for ; Thu, 30 Jul 2020 09:18:29 +0000 (UTC) Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A76D120672 for ; Thu, 30 Jul 2020 09:18:29 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A76D120672 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arndb.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linux-kernel-mentees-bounces@lists.linuxfoundation.org Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 7F57F87D65; Thu, 30 Jul 2020 09:18:29 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id pbYxjDv0TBhw; Thu, 30 Jul 2020 09:18:28 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id B486687D63; Thu, 30 Jul 2020 09:18:28 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 8828FC004F; Thu, 30 Jul 2020 09:18:28 +0000 (UTC) Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 2D69AC004D for ; Thu, 30 Jul 2020 09:18:27 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 1A44484B26 for ; Thu, 30 Jul 2020 09:18:27 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id brluLIgU09Wv for ; Thu, 30 Jul 2020 09:18:26 +0000 (UTC) X-Greylist: from auto-whitelisted by SQLgrey-1.7.6 Received: from mout.kundenserver.de (mout.kundenserver.de [212.227.17.13]) by fraxinus.osuosl.org (Postfix) with ESMTPS id 708B684789 for ; Thu, 30 Jul 2020 09:18:25 +0000 (UTC) Received: from mail-qt1-f174.google.com ([209.85.160.174]) by mrelayeu.kundenserver.de (mreue107 [212.227.15.145]) with ESMTPSA (Nemesis) id 1MGygv-1jwQ8r1pIJ-00E9UO for ; Thu, 30 Jul 2020 11:18:22 +0200 Received: by mail-qt1-f174.google.com with SMTP id o22so19569644qtt.13 for ; Thu, 30 Jul 2020 02:18:21 -0700 (PDT) X-Gm-Message-State: AOAM532+p0kLWGfsH2lNCWAgODcglnBA5KAfeMc4AXr9hcThpM4Iujv6 u27txctSoTJHk4yD75lSgbViTdT9YT4thUHyfCI= X-Google-Smtp-Source: ABdhPJxTFkRhqtuLhXAsNUlZ9J0TZfkHL99w5pWpprWMX0g7dJ5QSuxxz8xtNYzXVB7FSGpCxdmyitm5GsMtlvmucAc= X-Received: by 2002:aed:33e7:: with SMTP id v94mr1965217qtd.18.1596100700959; Thu, 30 Jul 2020 02:18:20 -0700 (PDT) MIME-Version: 1.0 References: <20200726220557.102300-1-yepeilin.cs@gmail.com> <20200726222703.102701-1-yepeilin.cs@gmail.com> <20200727131608.GD1913@kadam> <20200728130632.GI1913@kadam> <20200730083833.GD3703480@smile.fi.intel.com> In-Reply-To: <20200730083833.GD3703480@smile.fi.intel.com> From: Arnd Bergmann Date: Thu, 30 Jul 2020 11:18:04 +0200 X-Gmail-Original-Message-ID: Message-ID: To: Andy Shevchenko X-Provags-ID: V03:K1:lcO8QGGef0nMF7rKjitvRoMagyTrGJC8x/iS6laXyNkzKmoaSbf SCKpHINsKXjS56Gxk7atFADD8imhaUygPco09WWdvZgJwH3G/tsaY/gvwHWN/kl/42ID13G clLdU1Id1baRJtapmswPylLOiubxKjTOLZuz4QSmZCF6nVkFiD21lbKJpwFL+AY6el6E3Q6 9lpvZdGnCIBne2oqjFsFg== X-UI-Out-Filterresults: notjunk:1;V03:K0:GMohn1DQvsg=:e0q3zcFTcN7E/QCtvCyI/T aJW6qe3FhcpYIDjzyGt+x7u2cK22r5EZiv794cQjlIASMhuiwadCjIclOK7FzNEQb1e+82ZWI JT9PWlf2yENJnlKs2Aw0R4PdPk4E3pjQ/Cs7XCnSktlo69pSiQO447qAwrd1+PqEZ6Tog2wn4 P3hJuWfrDSjm1nIlEReebxxIOoN75ZqpYTjiqtHQCzXn6QsObwpSm3gwQjHRxUSqdTk8oOl+D oYhSj2DUbc00siD6n5qK+/aBqPJE+yldMFoTfUm72UIElf/wmKxjRX8V8VKEI/sCRf/ZeSo6D hEBDZojiz8sPehp2DNF0XQXOXGO/myGWGpqQkZdl9bknYOR8NitRFanbtUXqvJxGOgW2XkNgR KYwnQth8uSg7XYDsmYoEeB0KyslTcPI1LvddbwwyZo1FOce9tnvOxMp5wvEuyNHdhP5d1mgD7 Tz+44ws7vqIiUK4jASnjWxUlWoRLgBqZkVKFyQ7on7+hV16d1Q5YsprUJZChAcvBlpwMoCh+x rJDANpz8mzTDne9ge8D5Bl5K6f0kwCoR+6je6ML54cyQ1JJnvXE+VEwFVhDTIC1uQSDBh9B0W Hz1cPJwb4CXRYShtyw2NAUBiajUiSetoN8DUviBb0hUxg+lWHxZZDNQQMBm61/PalBKkNHaO4 NdS0nqK+sxHNaaYVPB6MKoEvykvDUy1QcEF4+R1t2s8fSuvXQZ8ea5cWaIgACScIxxZmrqNzj aF+yGo1LbjtDr0xJui65h0hClW6untdNFtKRYuRuYcViYvcxiOk5qh3SCawiHA+gza5f8PuGd KyPR2mrrN7py911iYUZ+AUrmxth2o9hgMs85FHa9WkuFSHtA4oR0JYwXnQc8buX10EfYvxV Cc: =?UTF-8?Q?Niklas_S=C3=B6derlund?= , "linux-kernel@vger.kernel.org" , Bartosz Golaszewski , syzkaller-bugs , Linux Media Mailing List , Peilin Ye , Laurent Pinchart , Sakari Ailus , Vandana BN , Hans Verkuil , Mauro Carvalho Chehab , Ezequiel Garcia , Linus Walleij , Dan Carpenter , linux-kernel-mentees@lists.linuxfoundation.org Subject: Re: [Linux-kernel-mentees] [PATCH v3] media/v4l2-core: Fix kernel-infoleak in video_put_user() X-BeenThere: linux-kernel-mentees@lists.linuxfoundation.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: linux-kernel-mentees-bounces@lists.linuxfoundation.org Sender: "Linux-kernel-mentees" On Thu, Jul 30, 2020 at 10:38 AM Andy Shevchenko wrote: > On Thu, Jul 30, 2020 at 10:15:24AM +0200, Arnd Bergmann wrote: > > On Thu, Jul 30, 2020 at 10:07 AM Bartosz Golaszewski wrote: > > > > > > On Tue, Jul 28, 2020 at 3:58 PM Arnd Bergmann wrote: > > > > > > > > On Tue, Jul 28, 2020 at 3:06 PM Dan Carpenter wrote: > > > > Something like > > > > > > > > static int lineevent_put_data(void __user *uptr, struct gpioevent_data *ge) > > > > { > > > > #ifdef __x86_64__ > > > > /* i386 has no padding after 'id' */ > > > > if (in_ia32_syscall()) { > > > > struct { > > > > compat_u64 timestamp __packed; > > > > u32 id; > > > > } compat_ge = { ge->timestamp, ge->id }; > > > > > > > > if (copy_to_user(uptr, &compat_ge, sizeof(compat_ge))) > > > > return -EFAULT; > > > > > > > > return sizeof(compat_ge); > > > > } > > > > #endif > > > > > > > > if (copy_to_user(uptr, ge, sizeof(*ge)) > > > > return -EFAULT; > > > > > > > > return sizeof(*ge); > > > > } > > > > > > > > Arnd > > > > > > Hi Arnd, > > > > > > Andy actually had a patch for that but since this isn't a regression > > > (it never worked), we decided to leave it as it is and get it right in > > > v2 API. > > > > I would argue that it needs to be fixed anyway, unless you also want > > to remove the v1 interface for native mode. If this works on 32-bit > > kernels, on 64-bit kernels with 64-bit user space and on compat > > 32-bit user space on 64-bit non-x86 architectures, I see no reason > > to leave it broken specifically on x86 compat user space. There are > > still reasons to use 32-bit x86 user space for low-memory machines > > even though native i386 kernels are getting increasingly silly. > > It was possible to "fix" (mitigate to some extent) before libgpiod got support > for several events in a request. Now it seems to be impossible to fix. AFAIU we > must discard any request to more than one event in it. Any reason why the workaround I suggested above would not work? The in_ia32_syscall() check should be completely reliable in telling whether we are called from read() by an ia32 task or not, and we use the same logic for input_event, which has a similar problem (on all compat architectures, not just x86). > However I'm not an expert in compat IOCTL code (you are :-) and perhaps you may > provide ideas better than mine. What makes this interface tricky is that this is actually a read() call, not ioctl() which is usually easier because it encodes the data length in the command code. As far as I could tell from skimming the interface, the ioctls are actually fine here. Arnd _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees