| Message ID | 5ba8ac5e465121b646a59487dc75b27017e0c769.1689252746.git.geert@linux-m68k.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: fb-helper/ssd130x: Add support for DRM_FORMAT_R1 | expand |
Geert Uytterhoeven <geert@linux-m68k.org> 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@linux-m68k.org> > --- > 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@redhat.com>
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;
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@linux-m68k.org> --- Why is drm_mode_create_dumb.size __u64? The test for "args->height > U32_MAX / stride" rejects all sizes not fitting in __u32 anyway. --- drivers/gpu/drm/drm_dumb_buffers.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)