Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> writes: > drm_mode_create_dumb() calculates the number of characters per pixel > from the number of bits per pixel by rounding up, which is not correct > as the actual value of cpp may be non-integer. While we do not need to > care here about complex formats like YUV, bpp < 8 is a valid use case. > > - The overflow check for the buffer width is not correct if bpp < 8. > However, it doesn't hurt, as widths larger than U32_MAX / 8 should > not happen for real anyway. Add a comment to clarify. > - Calculating the stride from the number of characters per pixel is > not correct. Fix this by calculating it from the number of bits per > pixel instead. > > Signed-off-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> > --- > Why is drm_mode_create_dumb.size __u64? The test for "args->height > I don't think can be changed since is a DRM_IOCTL_MODE_CREATE_DUMB uAPI ? > U32_MAX / stride" rejects all sizes not fitting in __u32 anyway. Hmm, wonder if should be U64_MAX instead ? > --- > drivers/gpu/drm/drm_dumb_buffers.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c > index 70032bba1c97e787..21a04c32a5e3d785 100644 > --- a/drivers/gpu/drm/drm_dumb_buffers.c > +++ b/drivers/gpu/drm/drm_dumb_buffers.c > @@ -71,10 +71,11 @@ int drm_mode_create_dumb(struct drm_device *dev, > /* overflow checks for 32bit size calculations */ > if (args->bpp > U32_MAX - 8) > return -EINVAL; > + /* Incorrect (especially if bpp < 8), but doesn't hurt much */ > cpp = DIV_ROUND_UP(args->bpp, 8); > if (cpp > U32_MAX / args->width) > return -EINVAL; > - stride = cpp * args->width; > + stride = DIV_ROUND_UP(args->bpp * args->width, 8); > if (args->height > U32_MAX / stride) > return -EINVAL; > Good catch. Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx> -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
