| Message ID | 20190118122020.27596-24-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/qxl: ttm fixes, cleanups, allocation tweaks, multihead, fbdev | expand |
On Fri, Jan 18, 2019 at 01:20:20PM +0100, Gerd Hoffmann wrote: > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> We already do all reasonable overflow checks in drm_mode_create_dumb(). If you don't trust them I think would be better time spent typing an igt to test this than adding redundant check in all drivers. You're also missing one check for bpp underflows :-) -Daniel > --- > drivers/gpu/drm/qxl/qxl_dumb.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/qxl/qxl_dumb.c b/drivers/gpu/drm/qxl/qxl_dumb.c > index 272d19b677..bed6d06ee4 100644 > --- a/drivers/gpu/drm/qxl/qxl_dumb.c > +++ b/drivers/gpu/drm/qxl/qxl_dumb.c > @@ -37,11 +37,13 @@ int qxl_mode_dumb_create(struct drm_file *file_priv, > uint32_t handle; > int r; > struct qxl_surface surf; > - uint32_t pitch, format; > + uint32_t pitch, size, format; > > - pitch = args->width * ((args->bpp + 1) / 8); > - args->size = pitch * args->height; > - args->size = ALIGN(args->size, PAGE_SIZE); > + if (check_mul_overflow(args->width, ((args->bpp + 1) / 8), &pitch)) > + return -EINVAL; > + if (check_mul_overflow(pitch, args->height, &size)) > + return -EINVAL; > + args->size = ALIGN(size, PAGE_SIZE); > > switch (args->bpp) { > case 16: > -- > 2.9.3 >
On Fri, Jan 18, 2019 at 04:49:44PM +0100, Daniel Vetter wrote: > On Fri, Jan 18, 2019 at 01:20:20PM +0100, Gerd Hoffmann wrote: > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > > We already do all reasonable overflow checks in drm_mode_create_dumb(). If > you don't trust them I think would be better time spent typing an igt to > test this than adding redundant check in all drivers. > > You're also missing one check for bpp underflows :-) BTW I just noticed that we don't seem to validating create_dumb->flags at all. Someone should probably add some checks for that, or mark it as deprecated in case we already lost the battle with userspace stack garbage. > -Daniel > > > --- > > drivers/gpu/drm/qxl/qxl_dumb.c | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/qxl/qxl_dumb.c b/drivers/gpu/drm/qxl/qxl_dumb.c > > index 272d19b677..bed6d06ee4 100644 > > --- a/drivers/gpu/drm/qxl/qxl_dumb.c > > +++ b/drivers/gpu/drm/qxl/qxl_dumb.c > > @@ -37,11 +37,13 @@ int qxl_mode_dumb_create(struct drm_file *file_priv, > > uint32_t handle; > > int r; > > struct qxl_surface surf; > > - uint32_t pitch, format; > > + uint32_t pitch, size, format; > > > > - pitch = args->width * ((args->bpp + 1) / 8); > > - args->size = pitch * args->height; > > - args->size = ALIGN(args->size, PAGE_SIZE); > > + if (check_mul_overflow(args->width, ((args->bpp + 1) / 8), &pitch)) > > + return -EINVAL; > > + if (check_mul_overflow(pitch, args->height, &size)) > > + return -EINVAL; > > + args->size = ALIGN(size, PAGE_SIZE); > > > > switch (args->bpp) { > > case 16: > > -- > > 2.9.3 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, Jan 18, 2019 at 5:32 PM Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > On Fri, Jan 18, 2019 at 04:49:44PM +0100, Daniel Vetter wrote: > > On Fri, Jan 18, 2019 at 01:20:20PM +0100, Gerd Hoffmann wrote: > > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > > > > We already do all reasonable overflow checks in drm_mode_create_dumb(). If > > you don't trust them I think would be better time spent typing an igt to > > test this than adding redundant check in all drivers. > > > > You're also missing one check for bpp underflows :-) > > BTW I just noticed that we don't seem to validating > create_dumb->flags at all. Someone should probably add some > checks for that, or mark it as deprecated in case we already > lost the battle with userspace stack garbage. Given that every kms client/compositor under the sun uses this (or well, all the generic ones at least) I think we can safely assume to have lost that battle :-/ -Daniel > > > -Daniel > > > > > --- > > > drivers/gpu/drm/qxl/qxl_dumb.c | 10 ++++++---- > > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/qxl/qxl_dumb.c b/drivers/gpu/drm/qxl/qxl_dumb.c > > > index 272d19b677..bed6d06ee4 100644 > > > --- a/drivers/gpu/drm/qxl/qxl_dumb.c > > > +++ b/drivers/gpu/drm/qxl/qxl_dumb.c > > > @@ -37,11 +37,13 @@ int qxl_mode_dumb_create(struct drm_file *file_priv, > > > uint32_t handle; > > > int r; > > > struct qxl_surface surf; > > > - uint32_t pitch, format; > > > + uint32_t pitch, size, format; > > > > > > - pitch = args->width * ((args->bpp + 1) / 8); > > > - args->size = pitch * args->height; > > > - args->size = ALIGN(args->size, PAGE_SIZE); > > > + if (check_mul_overflow(args->width, ((args->bpp + 1) / 8), &pitch)) > > > + return -EINVAL; > > > + if (check_mul_overflow(pitch, args->height, &size)) > > > + return -EINVAL; > > > + args->size = ALIGN(size, PAGE_SIZE); > > > > > > switch (args->bpp) { > > > case 16: > > > -- > > > 2.9.3 > > > > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Ville Syrjälä > Intel > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/qxl/qxl_dumb.c b/drivers/gpu/drm/qxl/qxl_dumb.c index 272d19b677..bed6d06ee4 100644 --- a/drivers/gpu/drm/qxl/qxl_dumb.c +++ b/drivers/gpu/drm/qxl/qxl_dumb.c @@ -37,11 +37,13 @@ int qxl_mode_dumb_create(struct drm_file *file_priv, uint32_t handle; int r; struct qxl_surface surf; - uint32_t pitch, format; + uint32_t pitch, size, format; - pitch = args->width * ((args->bpp + 1) / 8); - args->size = pitch * args->height; - args->size = ALIGN(args->size, PAGE_SIZE); + if (check_mul_overflow(args->width, ((args->bpp + 1) / 8), &pitch)) + return -EINVAL; + if (check_mul_overflow(pitch, args->height, &size)) + return -EINVAL; + args->size = ALIGN(size, PAGE_SIZE); switch (args->bpp) { case 16:
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- drivers/gpu/drm/qxl/qxl_dumb.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)