On Fri, 2017-03-10 at 11:11 +0000, Dave Stevenson wrote: > On 10 March 2017 at 05:08, Michael Zoran <mzoran@xxxxxxxxxxxx> 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@xxxxxxxxxxxx> > > --- > > 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 > > I actually tracked it while debugging down to these structures. I was going to let you know, but I went with the band-aid for now. The firmware thinks the structure should be 8 bytes longer. I'm wondering if it should be 4 cameras and 4 flashes. That would explain the extra 8 bytes. #define MMAL_PARAMETER_CAMERA_INFO_MAX_CAMERAS 4 #define MMAL_PARAMETER_CAMERA_INFO_MAX_FLASHES 2 #define MMAL_PARAMETER_CAMERA_INFO_MAX_STR_LEN 16 struct mmal_parameter_camera_info_camera_t { u32 port_id; u32 max_width; u32 max_height; u32 lens_present; u8 camera_name[MMAL_PARAMETER_CAMERA_INFO_MAX_STR_LEN]; }; enum mmal_parameter_camera_info_flash_type_t { /* Make values explicit to ensure they match values in config ini */ MMAL_PARAMETER_CAMERA_INFO_FLASH_TYPE_XENON = 0, MMAL_PARAMETER_CAMERA_INFO_FLASH_TYPE_LED = 1, MMAL_PARAMETER_CAMERA_INFO_FLASH_TYPE_OTHER = 2, MMAL_PARAMETER_CAMERA_INFO_FLASH_TYPE_MAX = 0x7FFFFFFF }; struct mmal_parameter_camera_info_flash_t { enum mmal_parameter_camera_info_flash_type_t flash_type; }; struct mmal_parameter_camera_info_t { u32 num_cameras; u32 num_flashes; struct mmal_parameter_camera_info_camera_t cameras[MMAL_PARAMETER_CAMERA_INFO_MAX_ CAMERAS]; struct mmal_parameter_camera_info_flash_t flashes[MMAL_PARAMETER_CAMERA_INFO_MAX_ FLASHES]; }; _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
