[virglrenderer-devel] [PATCH] renderer: fix integer overflow in create shader

[Li Qiang]

Hello Marc-André,

2017-01-09 20:43 GMT+08:00 Marc-André Lureau <mlureau at redhat.com>:

> Hi
>
> ----- Original Message -----
> > As the 'pkt_length' and 'offlen' can be malicious from guest,
> > the vrend_create_shader function has an integer overflow, this
> > will make the next 'memcpy' oob access. This patch avoid this.
> >
> > Signed-off-by: Li Qiang <liq3ea at gmail.com>
> > ---
> >  src/vrend_renderer.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
> > index 00b61eb..a92bc83 100644
> > --- a/src/vrend_renderer.c
> > +++ b/src/vrend_renderer.c
> > @@ -2211,6 +2211,15 @@ int vrend_create_shader(struct vrend_context *ctx,
> >           ret = EINVAL;
> >           goto error;
> >        }
> > +
> > +      /*make sure no overflow */
> > +      if (pkt_length * 4 < pkt_length ||
> > +          pkt_length * 4 + sel->buf_offset < pkt_length * 4 ||
> > +          pkt_length * 4 + sel->buf_offset < sel->buf_offset) {
> > +            ret = EINVAL;
> > +            goto error;
> > +          }
>
> That looks okay, wouldn't it be simpler to do the arithmetic on 64 bits
> instead?
>
>
Right, this is a little complicated. But as the 'pkt_length' and
'sel->buf_offset' can both set by the guest.
I think this is the most safe way, avoid all the overflow.


> > +
> >        if ((pkt_length * 4 + sel->buf_offset) > sel->buf_len) {
> >           fprintf(stderr, "Got too large shader continuation %d vs %d\n",
> >                   pkt_length * 4 + sel->buf_offset, sel->buf_len);
> > --
> > 2.7.4
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/virglrenderer-devel/attachments/20170110/55921368/attachment-0001.html>