[PATCH 3/3] staging: vchiq: Have vchiq_dump_* functions return an error code

Marcelo Diop-Gonzalez marcgonzalez at google.com
Thu Nov 21 21:25:20 UTC 2019


On Wed, Nov 20, 2019 at 1:51 PM Dan Carpenter <dan.carpenter at oracle.com> wrote:
>
> Thanks for the patch.  Looks good.
>
> Reviewed-by: Dan Carpenter <dan.carpenter at oracle.com>

Appreciate the review!

-Marcelo
>
> On Wed, Nov 20, 2019 at 10:36:48AM -0500, Marcelo Diop-Gonzalez wrote:
> >       copy_bytes = min_t(int, len, context->space - context->actual);
> >       if (copy_bytes == 0)
> > -             return;
> > +             return 0;
> >       if (copy_to_user(context->buf + context->actual, str,
> >                        copy_bytes))
> > -             context->actual = -EFAULT;
>                 ^^^^^^^^^^^^^^^^^^^^^^^^^
>
> > +             return -EFAULT;
> >       context->actual += copy_bytes;
>         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> In the original code we saved the error code to context->actual, then
> we added some bytes
>
> >       len -= copy_bytes;
> >
> > @@ -2109,8 +2108,9 @@ vchiq_dump(void *dump_context, const char *str, int len)
> >
> >               if (copy_to_user(context->buf + context->actual - 1,
>                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> and then we tried to use the result to corrupt the user space program.
> So this patch is a nice bug fix.
>
> >                                &cr, 1))
> > -                     context->actual = -EFAULT;
> > +                     return -EFAULT;
> >       }
> > +     return 0;
> >  }
>
> regards,
> dan carpenter


More information about the devel mailing list