From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailout1.w1.samsung.com ([210.118.77.11]:43471 "EHLO mailout1.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727649AbeIZV73 (ORCPT ); Wed, 26 Sep 2018 17:59:29 -0400 Received: from eucas1p2.samsung.com (unknown [182.198.249.207]) by mailout1.w1.samsung.com (KnoxPortal) with ESMTP id 20180926154555euoutp01bf1f162ce5d65ac536a5005494dd7492~X-bXMSao21537115371euoutp01t for ; Wed, 26 Sep 2018 15:45:55 +0000 (GMT) Subject: Re: [PATCH] fbdev/omapfb: fix omapfb_memory_read infoleak To: Tomi Valkeinen Cc: linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org, stable@vger.kernel.org, security@kernel.org, Will Deacon , Jann Horn , Tony Lindgren From: Bartlomiej Zolnierkiewicz Date: Wed, 26 Sep 2018 17:45:52 +0200 MIME-Version: 1.0 In-Reply-To: <20180912073046.26475-1-tomi.valkeinen@ti.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Message-Id: <20180926154553eucas1p11b97be93a7e9218945a45e2a23dfb099~X-bV9uhhW2059120591eucas1p1F@eucas1p1.samsung.com> References: <20180912073046.26475-1-tomi.valkeinen@ti.com> Sender: stable-owner@vger.kernel.org List-ID: [ added dri-devel@lists.freedesktop.org to Cc: ] On 09/12/2018 09:30 AM, Tomi Valkeinen wrote: > OMAPFB_MEMORY_READ ioctl reads pixels from the LCD's memory and copies > them to a userspace buffer. The code has two issues: > > - The user provided width and height could be large enough to overflow > the calculations > - The copy_to_user() can copy uninitialized memory to the userspace, > which might contain sensitive kernel information. > > Fix these by limiting the width & height parameters, and only copying > the amount of data that we actually received from the LCD. > > Signed-off-by: Tomi Valkeinen > Reported-by: Jann Horn > Cc: stable@vger.kernel.org > Cc: security@kernel.org > Cc: Will Deacon > Cc: Jann Horn > Cc: Tony Lindgren Patch queued for 4.19, thanks. > --- > drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c b/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c > index ef69273074ba..a3edb20ea4c3 100644 > --- a/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c > +++ b/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c > @@ -496,6 +496,9 @@ static int omapfb_memory_read(struct fb_info *fbi, > if (!access_ok(VERIFY_WRITE, mr->buffer, mr->buffer_size)) > return -EFAULT; > > + if (mr->w > 4096 || mr->h > 4096) > + return -EINVAL; > + > if (mr->w * mr->h * 3 > mr->buffer_size) > return -EINVAL; > > @@ -509,7 +512,7 @@ static int omapfb_memory_read(struct fb_info *fbi, > mr->x, mr->y, mr->w, mr->h); > > if (r > 0) { > - if (copy_to_user(mr->buffer, buf, mr->buffer_size)) > + if (copy_to_user(mr->buffer, buf, r)) > r = -EFAULT; > } Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bartlomiej Zolnierkiewicz Date: Wed, 26 Sep 2018 15:45:52 +0000 Subject: Re: [PATCH] fbdev/omapfb: fix omapfb_memory_read infoleak Message-Id: <20180926154553eucas1p11b97be93a7e9218945a45e2a23dfb099~X-bV9uhhW2059120591eucas1p1F@eucas1p1.samsung.com> List-Id: References: <20180912073046.26475-1-tomi.valkeinen@ti.com> In-Reply-To: <20180912073046.26475-1-tomi.valkeinen@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Tomi Valkeinen Cc: linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org, stable@vger.kernel.org, security@kernel.org, Will Deacon , Jann Horn , Tony Lindgren [ added dri-devel@lists.freedesktop.org to Cc: ] On 09/12/2018 09:30 AM, Tomi Valkeinen wrote: > OMAPFB_MEMORY_READ ioctl reads pixels from the LCD's memory and copies > them to a userspace buffer. The code has two issues: > > - The user provided width and height could be large enough to overflow > the calculations > - The copy_to_user() can copy uninitialized memory to the userspace, > which might contain sensitive kernel information. > > Fix these by limiting the width & height parameters, and only copying > the amount of data that we actually received from the LCD. > > Signed-off-by: Tomi Valkeinen > Reported-by: Jann Horn > Cc: stable@vger.kernel.org > Cc: security@kernel.org > Cc: Will Deacon > Cc: Jann Horn > Cc: Tony Lindgren Patch queued for 4.19, thanks. > --- > drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c b/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c > index ef69273074ba..a3edb20ea4c3 100644 > --- a/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c > +++ b/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c > @@ -496,6 +496,9 @@ static int omapfb_memory_read(struct fb_info *fbi, > if (!access_ok(VERIFY_WRITE, mr->buffer, mr->buffer_size)) > return -EFAULT; > > + if (mr->w > 4096 || mr->h > 4096) > + return -EINVAL; > + > if (mr->w * mr->h * 3 > mr->buffer_size) > return -EINVAL; > > @@ -509,7 +512,7 @@ static int omapfb_memory_read(struct fb_info *fbi, > mr->x, mr->y, mr->w, mr->h); > > if (r > 0) { > - if (copy_to_user(mr->buffer, buf, mr->buffer_size)) > + if (copy_to_user(mr->buffer, buf, r)) > r = -EFAULT; > } Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bartlomiej Zolnierkiewicz Subject: Re: [PATCH] fbdev/omapfb: fix omapfb_memory_read infoleak Date: Wed, 26 Sep 2018 17:45:52 +0200 Message-ID: <20180926154553eucas1p11b97be93a7e9218945a45e2a23dfb099~X-bV9uhhW2059120591eucas1p1F@eucas1p1.samsung.com> References: <20180912073046.26475-1-tomi.valkeinen@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20180912073046.26475-1-tomi.valkeinen@ti.com> Sender: stable-owner@vger.kernel.org To: Tomi Valkeinen Cc: linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org, stable@vger.kernel.org, security@kernel.org, Will Deacon , Jann Horn , Tony Lindgren List-Id: dri-devel@lists.freedesktop.org [ added dri-devel@lists.freedesktop.org to Cc: ] On 09/12/2018 09:30 AM, Tomi Valkeinen wrote: > OMAPFB_MEMORY_READ ioctl reads pixels from the LCD's memory and copies > them to a userspace buffer. The code has two issues: > > - The user provided width and height could be large enough to overflow > the calculations > - The copy_to_user() can copy uninitialized memory to the userspace, > which might contain sensitive kernel information. > > Fix these by limiting the width & height parameters, and only copying > the amount of data that we actually received from the LCD. > > Signed-off-by: Tomi Valkeinen > Reported-by: Jann Horn > Cc: stable@vger.kernel.org > Cc: security@kernel.org > Cc: Will Deacon > Cc: Jann Horn > Cc: Tony Lindgren Patch queued for 4.19, thanks. > --- > drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c b/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c > index ef69273074ba..a3edb20ea4c3 100644 > --- a/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c > +++ b/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c > @@ -496,6 +496,9 @@ static int omapfb_memory_read(struct fb_info *fbi, > if (!access_ok(VERIFY_WRITE, mr->buffer, mr->buffer_size)) > return -EFAULT; > > + if (mr->w > 4096 || mr->h > 4096) > + return -EINVAL; > + > if (mr->w * mr->h * 3 > mr->buffer_size) > return -EINVAL; > > @@ -509,7 +512,7 @@ static int omapfb_memory_read(struct fb_info *fbi, > mr->x, mr->y, mr->w, mr->h); > > if (r > 0) { > - if (copy_to_user(mr->buffer, buf, mr->buffer_size)) > + if (copy_to_user(mr->buffer, buf, r)) > r = -EFAULT; > } Best regards,