[PATCH 08/10] staging: bcm2835-camera: Fix buffer overflow calculation on query of camera properties

[Dave Stevenson]

On 10 March 2017 at 05:08, Michael Zoran <mzoran at crowfest.net> wrote:
> The code that queries properties on the camera has a check
> for buffer overruns if the firmware sends too much data.  This
> check is incorrect, and during testing I was seeing stack corruption.
>
> I believe this error can actually happen in normal use, just for
> some reason it doesn't appear on 32 bit as often.  So perhaps
> it's best for the check to be fixed.

That sounds like it is related to a report I couldn't nail down as it
only happened on ArchLinux -
https://github.com/raspberrypi/linux/issues/1447

Comparing against the original MMAL implementation at
https://github.com/raspberrypi/userland/blob/master/interface/mmal/vc/mmal_vc_api.c#L1187
there are a few subtle differences. Your patch is a belt and braces to
ensure that it never copies too much, so reasonable in that regard.

I'm actually more worried that it implies we have a structure mismatch
against the firmware, but I currently can't see where it is. A load of
debug prints for sizeof(struct) required....

> Signed-off-by: Michael Zoran <mzoran at crowfest.net>
> ---
>  drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
> index 41de8956e421..976aa08365f2 100644
> --- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
> +++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
> @@ -1442,7 +1442,7 @@ static int port_parameter_get(struct vchiq_mmal_instance *instance,
>         }
>
>         ret = -rmsg->u.port_parameter_get_reply.status;
> -       if (ret) {
> +       if (ret || (rmsg->u.port_parameter_get_reply.size > *value_size)) {
>                 /* Copy only as much as we have space for
>                  * but report true size of parameter
>                  */
> --
> 2.11.0
>