The ancient commits from times of yore lead to the following static
checker warning:
drivers/gpu/drm/omapdrm/omap_fb.c:429 omap_framebuffer_init()
warn: untrusted unsigned subtract. 'omap_gem_mmap_size(bos[i]) -
mode_cmd->offsets[i]' '0-u64max minus 0-u32max(user_rl)'
drivers/gpu/drm/omapdrm/omap_fb.c
368 struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
369 const struct drm_format_info *info,
370 const struct drm_mode_fb_cmd2 *mode_cmd, struct
drm_gem_object **bos)
371 {
372 struct omap_framebuffer *omap_fb = NULL;
373 struct drm_framebuffer *fb = NULL;
374 unsigned int pitch = mode_cmd->pitches[0];
375 int ret, i;
376
377 DBG("create framebuffer: dev=%p, mode_cmd=%p (%dx%d@%4.4s)",
378 dev, mode_cmd, mode_cmd->width,
mode_cmd->height,
379 (char *)&mode_cmd->pixel_format);
380
381 for (i = 0; i < ARRAY_SIZE(formats); i++) {
382 if (formats[i] == mode_cmd->pixel_format)
383 break;
384 }
385
386 if (i == ARRAY_SIZE(formats)) {
387 dev_dbg(dev->dev, "unsupported pixel format: %4.4s\n",
388 (char *)&mode_cmd->pixel_format);
389 ret = -EINVAL;
390 goto fail;
391 }
392
393 omap_fb = kzalloc(sizeof(*omap_fb), GFP_KERNEL);
394 if (!omap_fb) {
395 ret = -ENOMEM;
396 goto fail;
397 }
398
399 fb = &omap_fb->base;
400 omap_fb->format = info;
401 mutex_init(&omap_fb->lock);
402
403 /*
404 * The code below assumes that no format use more than two
planes, and
405 * that the two planes of multiplane formats need the same
number of
406 * bytes per pixel.
407 */
408 if (info->num_planes == 2 && pitch != mode_cmd->pitches[1]) {
409 dev_dbg(dev->dev, "pitches differ between planes 0 and
1\n");
410 ret = -EINVAL;
411 goto fail;
412 }
413
414 if (pitch % info->cpp[0]) {
415 dev_dbg(dev->dev,
416 "buffer pitch (%u bytes) is not a multiple of
pixel size (%u bytes)\n",
417 pitch, info->cpp[0]);
418 ret = -EINVAL;
419 goto fail;
420 }
421
422 for (i = 0; i < info->num_planes; i++) {
423 struct plane *plane = &omap_fb->planes[i];
424 unsigned int vsub = i == 0 ? 1 : info->vsub;
425 unsigned int size;
426
427 size = pitch * mode_cmd->height / vsub;
428
--> 429 if (size > omap_gem_mmap_size(bos[i]) -
mode_cmd->offsets[i]) {
The mode_cmd->offsets[i] value comes from the user via the drm_ioctl()
and it hasn't been checked at all so far as I can see. If it's larger
than omap_gem_mmap_size(bos[i]) then the result is unsigned so it's a
huge unsigned value which means that "size" can be anything.
It would be simple enough to add a check:
if (mode_cmd->offsets[i] > omap_gem_mmap_size(bos[i]))
return -EINVAL;
But I don't know the code very well so maybe there is a better place
to check for this?
430 dev_dbg(dev->dev,
431 "provided buffer object is too small!
%zu < %d\n",
432 bos[i]->size - mode_cmd->offsets[i],
size);
433 ret = -EINVAL;
434 goto fail;
435 }
436
437 fb->obj[i] = bos[i];
438 plane->dma_addr = 0;
439 }
440
441 drm_helper_mode_fill_fb_struct(dev, fb, info, mode_cmd);
442
443 ret = drm_framebuffer_init(dev, fb, &omap_framebuffer_funcs);
444 if (ret) {
445 dev_err(dev->dev, "framebuffer init failed: %d\n", ret);
446 goto fail;
447 }
448
449 DBG("create: FB ID: %d (%p)", fb->base.id, fb);
450
451 return fb;
452
453 fail:
454 kfree(omap_fb);
455
456 return ERR_PTR(ret);
457 }
regards,
dan carpenter
