drm/cirrus: Use drm_framebuffer_put to avoid kernel oops in clean-up

In the Cirrus driver, the regular clean-up code also performs the clean-up
of a failed initialization. If the fbdev's framebuffer was not initialized,
the clean-up will fail within drm_framebuffer_unregister_private. Booting
with cirrus.bpp=16 triggers this bug.

The framebuffer is currently stored directly within struct cirrus_fbdev. To
fix the bug, we turn it into a pointer that is only set for initialized
framebuffers. The fbdev's clean-up code skips uninitialized framebuffers.

The memory for struct drm_framebuffer is allocated dynamically. This requires
additional error handling within cirrusfb_create. The framebuffer clean-up is
now performed by drm_framebuffer_put, which also frees the data strcuture's
memory.

Link: https://bugzilla.suse.com/show_bug.cgi?id=1101822
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Link: http://patchwork.freedesktop.org/patch/msgid/20180720112743.27159-1-tzimmermann@suse.de
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
This commit is contained in:
Thomas Zimmermann 2018-07-20 13:27:43 +02:00 committed by Gerd Hoffmann
parent f82aab2d52
commit abf7b30d7f
3 changed files with 29 additions and 23 deletions

View File

@ -146,7 +146,7 @@ struct cirrus_device {
struct cirrus_fbdev {
struct drm_fb_helper helper;
struct drm_framebuffer gfb;
struct drm_framebuffer *gfb;
void *sysram;
int size;
int x1, y1, x2, y2; /* dirty rect */

View File

@ -22,14 +22,14 @@ static void cirrus_dirty_update(struct cirrus_fbdev *afbdev,
struct drm_gem_object *obj;
struct cirrus_bo *bo;
int src_offset, dst_offset;
int bpp = afbdev->gfb.format->cpp[0];
int bpp = afbdev->gfb->format->cpp[0];
int ret = -EBUSY;
bool unmap = false;
bool store_for_later = false;
int x2, y2;
unsigned long flags;
obj = afbdev->gfb.obj[0];
obj = afbdev->gfb->obj[0];
bo = gem_to_cirrus_bo(obj);
/*
@ -82,7 +82,7 @@ static void cirrus_dirty_update(struct cirrus_fbdev *afbdev,
}
for (i = y; i < y + height; i++) {
/* assume equal stride for now */
src_offset = dst_offset = i * afbdev->gfb.pitches[0] + (x * bpp);
src_offset = dst_offset = i * afbdev->gfb->pitches[0] + (x * bpp);
memcpy_toio(bo->kmap.virtual + src_offset, afbdev->sysram + src_offset, width * bpp);
}
@ -192,23 +192,26 @@ static int cirrusfb_create(struct drm_fb_helper *helper,
return -ENOMEM;
info = drm_fb_helper_alloc_fbi(helper);
if (IS_ERR(info))
return PTR_ERR(info);
if (IS_ERR(info)) {
ret = PTR_ERR(info);
goto err_vfree;
}
info->par = gfbdev;
ret = cirrus_framebuffer_init(cdev->dev, &gfbdev->gfb, &mode_cmd, gobj);
fb = kzalloc(sizeof(*fb), GFP_KERNEL);
if (!fb) {
ret = -ENOMEM;
goto err_drm_gem_object_put_unlocked;
}
ret = cirrus_framebuffer_init(cdev->dev, fb, &mode_cmd, gobj);
if (ret)
return ret;
goto err_kfree;
gfbdev->sysram = sysram;
gfbdev->size = size;
fb = &gfbdev->gfb;
if (!fb) {
DRM_INFO("fb is NULL\n");
return -EINVAL;
}
gfbdev->gfb = fb;
/* setup helper */
gfbdev->helper.fb = fb;
@ -241,24 +244,27 @@ static int cirrusfb_create(struct drm_fb_helper *helper,
DRM_INFO(" pitch is %d\n", fb->pitches[0]);
return 0;
err_kfree:
kfree(fb);
err_drm_gem_object_put_unlocked:
drm_gem_object_put_unlocked(gobj);
err_vfree:
vfree(sysram);
return ret;
}
static int cirrus_fbdev_destroy(struct drm_device *dev,
struct cirrus_fbdev *gfbdev)
{
struct drm_framebuffer *gfb = &gfbdev->gfb;
struct drm_framebuffer *gfb = gfbdev->gfb;
drm_fb_helper_unregister_fbi(&gfbdev->helper);
if (gfb->obj[0]) {
drm_gem_object_put_unlocked(gfb->obj[0]);
gfb->obj[0] = NULL;
}
vfree(gfbdev->sysram);
drm_fb_helper_fini(&gfbdev->helper);
drm_framebuffer_unregister_private(gfb);
drm_framebuffer_cleanup(gfb);
if (gfb)
drm_framebuffer_put(gfb);
return 0;
}

View File

@ -127,7 +127,7 @@ static int cirrus_crtc_do_set_base(struct drm_crtc *crtc,
return ret;
}
if (&cdev->mode_info.gfbdev->gfb == crtc->primary->fb) {
if (cdev->mode_info.gfbdev->gfb == crtc->primary->fb) {
/* if pushing console in kmap it */
ret = ttm_bo_kmap(&bo->bo, 0, bo->bo.num_pages, &bo->kmap);
if (ret)