From: Arnd Bergmann <arnd@arndb.de> To: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: "Bartosz Golaszewski" <brgl@bgdev.pl>, "Dan Carpenter" <dan.carpenter@oracle.com>, "Linus Walleij" <linus.walleij@linaro.org>, "Peilin Ye" <yepeilin.cs@gmail.com>, "Mauro Carvalho Chehab" <mchehab@kernel.org>, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>, syzkaller-bugs <syzkaller-bugs@googlegroups.com>, "Hans Verkuil" <hverkuil-cisco@xs4all.nl>, "Sakari Ailus" <sakari.ailus@linux.intel.com>, "Laurent Pinchart" <laurent.pinchart@ideasonboard.com>, "Vandana BN" <bnvandana@gmail.com>, "Ezequiel Garcia" <ezequiel@collabora.com>, "Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>, linux-kernel-mentees@lists.linuxfoundation.org, "Linux Media Mailing List" <linux-media@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org> Subject: Re: [Linux-kernel-mentees] [PATCH v3] media/v4l2-core: Fix kernel-infoleak in video_put_user() Date: Thu, 30 Jul 2020 11:18:04 +0200 [thread overview] Message-ID: <CAK8P3a2i2mnRPZ-f9UNxPNJcjyjdTOA7-fgcMqavsV8Ab17RkQ@mail.gmail.com> (raw) In-Reply-To: <20200730083833.GD3703480@smile.fi.intel.com> On Thu, Jul 30, 2020 at 10:38 AM Andy Shevchenko <andriy.shevchenko@linux.intel.com> 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 <brgl@bgdev.pl> wrote: > > > > > > On Tue, Jul 28, 2020 at 3:58 PM Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > > > On Tue, Jul 28, 2020 at 3:06 PM Dan Carpenter <dan.carpenter@oracle.com> 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
WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd@arndb.de> To: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: "Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "Bartosz Golaszewski" <brgl@bgdev.pl>, syzkaller-bugs <syzkaller-bugs@googlegroups.com>, "Linux Media Mailing List" <linux-media@vger.kernel.org>, "Peilin Ye" <yepeilin.cs@gmail.com>, "Laurent Pinchart" <laurent.pinchart@ideasonboard.com>, "Sakari Ailus" <sakari.ailus@linux.intel.com>, "Vandana BN" <bnvandana@gmail.com>, "Hans Verkuil" <hverkuil-cisco@xs4all.nl>, "Mauro Carvalho Chehab" <mchehab@kernel.org>, "Ezequiel Garcia" <ezequiel@collabora.com>, "Linus Walleij" <linus.walleij@linaro.org>, "Dan Carpenter" <dan.carpenter@oracle.com>, linux-kernel-mentees@lists.linuxfoundation.org Subject: Re: [Linux-kernel-mentees] [PATCH v3] media/v4l2-core: Fix kernel-infoleak in video_put_user() Date: Thu, 30 Jul 2020 11:18:04 +0200 [thread overview] Message-ID: <CAK8P3a2i2mnRPZ-f9UNxPNJcjyjdTOA7-fgcMqavsV8Ab17RkQ@mail.gmail.com> (raw) In-Reply-To: <20200730083833.GD3703480@smile.fi.intel.com> On Thu, Jul 30, 2020 at 10:38 AM Andy Shevchenko <andriy.shevchenko@linux.intel.com> 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 <brgl@bgdev.pl> wrote: > > > > > > On Tue, Jul 28, 2020 at 3:58 PM Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > > > On Tue, Jul 28, 2020 at 3:06 PM Dan Carpenter <dan.carpenter@oracle.com> 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
next prev parent reply other threads:[~2020-07-30 9:18 UTC|newest] Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-07-26 16:44 [Linux-kernel-mentees] [PATCH] media/v4l2-core: Fix kernel-infoleak in video_put_user() Peilin Ye 2020-07-26 16:44 ` Peilin Ye 2020-07-26 17:30 ` Laurent Pinchart 2020-07-26 17:30 ` Laurent Pinchart 2020-07-26 18:07 ` Peilin Ye 2020-07-26 18:07 ` Peilin Ye 2020-07-26 22:08 ` Laurent Pinchart 2020-07-26 22:08 ` Laurent Pinchart 2020-07-26 22:15 ` Peilin Ye 2020-07-26 22:15 ` Peilin Ye 2020-07-26 18:12 ` Peilin Ye 2020-07-26 18:12 ` Peilin Ye 2020-07-26 22:05 ` [Linux-kernel-mentees] [PATCH v2] " Peilin Ye 2020-07-26 22:05 ` Peilin Ye 2020-07-26 22:10 ` Laurent Pinchart 2020-07-26 22:10 ` Laurent Pinchart 2020-07-26 22:16 ` Peilin Ye 2020-07-26 22:16 ` Peilin Ye 2020-07-26 22:27 ` [Linux-kernel-mentees] [PATCH v3] " Peilin Ye 2020-07-26 22:27 ` Peilin Ye 2020-07-27 7:25 ` Arnd Bergmann 2020-07-27 7:25 ` Arnd Bergmann 2020-07-27 7:56 ` Peilin Ye 2020-07-27 7:56 ` Peilin Ye 2020-07-27 13:16 ` Dan Carpenter 2020-07-27 13:16 ` Dan Carpenter 2020-07-27 14:05 ` Arnd Bergmann 2020-07-27 14:05 ` Arnd Bergmann 2020-07-27 14:14 ` Peilin Ye 2020-07-27 14:14 ` Peilin Ye 2020-07-27 14:20 ` Arnd Bergmann 2020-07-27 14:20 ` Arnd Bergmann 2020-07-27 14:46 ` Dan Carpenter 2020-07-27 14:46 ` Dan Carpenter 2020-07-27 15:30 ` Peilin Ye 2020-07-27 15:30 ` Peilin Ye 2020-07-27 14:43 ` Dan Carpenter 2020-07-27 14:43 ` Dan Carpenter 2020-07-27 14:55 ` Arnd Bergmann 2020-07-27 14:55 ` Arnd Bergmann 2020-07-27 22:04 ` Peilin Ye 2020-07-27 22:04 ` Peilin Ye 2020-07-28 9:00 ` Arnd Bergmann 2020-07-28 9:00 ` Arnd Bergmann 2020-07-28 10:02 ` Dan Carpenter 2020-07-28 10:02 ` Dan Carpenter 2020-07-27 22:33 ` Peilin Ye 2020-07-27 22:33 ` Peilin Ye 2020-07-28 9:10 ` Arnd Bergmann 2020-07-28 9:10 ` Arnd Bergmann 2020-07-28 9:47 ` Dan Carpenter 2020-07-28 9:47 ` Dan Carpenter 2020-07-28 13:13 ` Peilin Ye 2020-07-28 13:13 ` Peilin Ye 2020-07-28 12:22 ` Linus Walleij 2020-07-28 12:22 ` Linus Walleij 2020-07-28 13:06 ` Dan Carpenter 2020-07-28 13:06 ` Dan Carpenter 2020-07-28 13:58 ` Arnd Bergmann 2020-07-28 13:58 ` Arnd Bergmann 2020-07-30 8:07 ` Bartosz Golaszewski 2020-07-30 8:07 ` Bartosz Golaszewski 2020-07-30 8:15 ` Arnd Bergmann 2020-07-30 8:15 ` Arnd Bergmann 2020-07-30 8:38 ` Andy Shevchenko 2020-07-30 8:38 ` Andy Shevchenko 2020-07-30 9:18 ` Arnd Bergmann [this message] 2020-07-30 9:18 ` Arnd Bergmann 2020-07-30 11:48 ` Andy Shevchenko 2020-07-30 11:48 ` Andy Shevchenko 2020-07-30 13:49 ` Arnd Bergmann 2020-07-30 13:49 ` Arnd Bergmann 2020-08-02 16:55 ` Peilin Ye 2020-08-02 16:55 ` Peilin Ye 2020-07-27 8:00 ` [Linux-kernel-mentees] [PATCH v4] " Peilin Ye 2020-07-27 8:00 ` Peilin Ye
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAK8P3a2i2mnRPZ-f9UNxPNJcjyjdTOA7-fgcMqavsV8Ab17RkQ@mail.gmail.com \ --to=arnd@arndb.de \ --cc=andriy.shevchenko@linux.intel.com \ --cc=bnvandana@gmail.com \ --cc=brgl@bgdev.pl \ --cc=dan.carpenter@oracle.com \ --cc=ezequiel@collabora.com \ --cc=gregkh@linuxfoundation.org \ --cc=hverkuil-cisco@xs4all.nl \ --cc=laurent.pinchart@ideasonboard.com \ --cc=linus.walleij@linaro.org \ --cc=linux-kernel-mentees@lists.linuxfoundation.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=mchehab@kernel.org \ --cc=niklas.soderlund+renesas@ragnatech.se \ --cc=sakari.ailus@linux.intel.com \ --cc=syzkaller-bugs@googlegroups.com \ --cc=yepeilin.cs@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.