From: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
To: Tomasz Figa <tfiga@chromium.org>
Cc: Ricardo Ribalda <ribalda@chromium.org>,
Marek Szyprowski <m.szyprowski@samsung.com>,
linux-media <linux-media@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>, Jiri Slaby <jslaby@suse.cz>,
Sergey Senozhatsky <senozhatsky@chromium.org>,
stable@vger.kernel.org
Subject: Re: [PATCH v2] media: videobuf2: Fix integer overrun in vb2_mmap
Date: Sun, 6 Jun 2021 21:45:06 +0200 [thread overview]
Message-ID: <CAPybu_2u-5nEy0ayT0-wrEmHpjx+UNR+iS-9+sPYb-yuRW9+qQ@mail.gmail.com> (raw)
In-Reply-To: <YLoWzSoSUQ6HYsyO@chromium.org>
Hi Tomasz
Thanks for your review!
We finally figured out that we would not like to support allocations
of exactly 4GiB. The rest of the code is not designed for it.
And you will get a descriptive enough error if you overrun (at least
in recent kernels):
if (length < (vma->vm_end - vma->vm_start)) {
Best regards!
On Fri, Jun 4, 2021 at 2:06 PM Tomasz Figa <tfiga@chromium.org> wrote:
>
> On Wed, Mar 10, 2021 at 08:51:27AM +0100, Ricardo Ribalda wrote:
> > The plane_length is an unsigned integer. So, if we have a size of
> > 0xffffffff bytes we incorrectly allocate 0 bytes instead of 1 << 32.
> >
> > Suggested-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> > Cc: stable@vger.kernel.org
> > Fixes: 7f8414594e47 ("[media] media: videobuf2: fix the length check for mmap")
> > Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> > drivers/media/common/videobuf2/videobuf2-core.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> > index 543da515c761..89c8bacb94a7 100644
> > --- a/drivers/media/common/videobuf2/videobuf2-core.c
> > +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> > @@ -2256,7 +2256,7 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma)
> > * The buffer length was page_aligned at __vb2_buf_mem_alloc(),
> > * so, we need to do the same here.
> > */
> > - length = PAGE_ALIGN(vb->planes[plane].length);
> > + length = PAGE_ALIGN((unsigned long)vb->planes[plane].length);
> > if (length < (vma->vm_end - vma->vm_start)) {
> > dprintk(q, 1,
> > "MMAP invalid, as it would overflow buffer length\n");
> > --
> > 2.30.1.766.gb4fecdf3b7-goog
> >
>
> Acked-by: Tomasz Figa <tfiga@chromium.org>
>
> Best regards,
> Tomasz
--
Ricardo Ribalda
prev parent reply other threads:[~2021-06-06 19:46 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-10 7:51 [PATCH v2] media: videobuf2: Fix integer overrun in vb2_mmap Ricardo Ribalda
2021-06-04 12:04 ` Tomasz Figa
2021-06-06 19:45 ` Ricardo Ribalda Delgado [this message]
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=CAPybu_2u-5nEy0ayT0-wrEmHpjx+UNR+iS-9+sPYb-yuRW9+qQ@mail.gmail.com \
--to=ricardo.ribalda@gmail.com \
--cc=jslaby@suse.cz \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=ribalda@chromium.org \
--cc=senozhatsky@chromium.org \
--cc=stable@vger.kernel.org \
--cc=tfiga@chromium.org \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).