Skip to content

Commit

Permalink
drm/ssd130x: Fix an oops when attempting to update a disabled plane
Browse files Browse the repository at this point in the history
Geert reports that the following NULL pointer dereference happens for him
after commit 49d7d58 ("drm/ssd130x: Don't allocate buffers on each
plane update"):

    [drm] Initialized ssd130x 1.0.0 20220131 for 0-003c on minor 0
    ssd130x-i2c 0-003c: [drm] surface width(128), height(32), bpp(1)
    and format(R1   little-endian (0x20203152))
    Unable to handle kernel NULL pointer dereference at virtual address 00000000
    Oops [#1]
    CPU: 0 PID: 1 Comm: swapper Not tainted
    6.5.0-rc1-orangecrab-02219-g0a529a1e4bf4 torvalds#565
    epc : ssd130x_update_rect.isra.0+0x13c/0x340
     ra : ssd130x_update_rect.isra.0+0x2bc/0x340
    ...
    status: 00000120 badaddr: 00000000 cause: 0000000f
    [<c0303d90>] ssd130x_update_rect.isra.0+0x13c/0x340
    [<c0304200>] ssd130x_primary_plane_helper_atomic_update+0x26c/0x284
    [<c02f8d54>] drm_atomic_helper_commit_planes+0xfc/0x27c
    [<c02f9314>] drm_atomic_helper_commit_tail+0x5c/0xb4
    [<c02f94fc>] commit_tail+0x190/0x1b8
    [<c02f99fc>] drm_atomic_helper_commit+0x194/0x1c0
    [<c02c5d00>] drm_atomic_commit+0xa4/0xe4
    [<c02cce40>] drm_client_modeset_commit_atomic+0x244/0x278
    [<c02ccef0>] drm_client_modeset_commit_locked+0x7c/0x1bc
    [<c02cd064>] drm_client_modeset_commit+0x34/0x64
    [<c0301a78>] __drm_fb_helper_restore_fbdev_mode_unlocked+0xc4/0xe8
    [<c0303424>] drm_fb_helper_set_par+0x38/0x58
    [<c027c410>] fbcon_init+0x294/0x534
    ...

The problem is that fbcon calls fbcon_init() which triggers a DRM modeset
and this leads to drm_atomic_helper_commit_planes() attempting to commit
the atomic state for all planes, even the ones whose CRTC is not enabled.

Since the primary plane buffer is allocated in the encoder .atomic_enable
callback, this happens after that initial modeset commit and leads to the
mentioned NULL pointer dereference.

Fix this by allocating the buffers in the struct drm_plane_helper_funcs
.begin_fb_access callback and free them in to .end_fb_access callback,
instead of doing it when the encoder is enabled.

Fixes: 49d7d58 ("drm/ssd130x: Don't allocate buffers on each plane update")
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
  • Loading branch information
martinezjavier authored and intel-lab-lkp committed Jul 18, 2023
1 parent 41639b3 commit 40eba39
Showing 1 changed file with 26 additions and 7 deletions.
33 changes: 26 additions & 7 deletions drivers/gpu/drm/solomon/ssd130x.c
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,30 @@ static int ssd130x_fb_blit_rect(struct drm_framebuffer *fb, const struct iosys_m
return ret;
}

static int ssd130x_primary_plane_helper_begin_fb_access(struct drm_plane *plane,
struct drm_plane_state *state)
{
struct drm_device *drm = plane->dev;
struct ssd130x_device *ssd130x = drm_to_ssd130x(drm);
int ret = ssd130x_buf_alloc(ssd130x);

if (ret)
return ret;

return drm_gem_begin_shadow_fb_access(plane, state);
}

static void ssd130x_primary_plane_helper_end_fb_access(struct drm_plane *plane,
struct drm_plane_state *state)
{
struct drm_device *drm = plane->dev;
struct ssd130x_device *ssd130x = drm_to_ssd130x(drm);

ssd130x_buf_free(ssd130x);

return drm_gem_end_shadow_fb_access(plane, state);
}

static void ssd130x_primary_plane_helper_atomic_update(struct drm_plane *plane,
struct drm_atomic_state *state)
{
Expand Down Expand Up @@ -656,7 +680,8 @@ static void ssd130x_primary_plane_helper_atomic_disable(struct drm_plane *plane,
}

static const struct drm_plane_helper_funcs ssd130x_primary_plane_helper_funcs = {
DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
.begin_fb_access = ssd130x_primary_plane_helper_begin_fb_access,
.end_fb_access = ssd130x_primary_plane_helper_end_fb_access,
.atomic_check = drm_plane_helper_atomic_check,
.atomic_update = ssd130x_primary_plane_helper_atomic_update,
.atomic_disable = ssd130x_primary_plane_helper_atomic_disable,
Expand Down Expand Up @@ -719,10 +744,6 @@ static void ssd130x_encoder_helper_atomic_enable(struct drm_encoder *encoder,
if (ret)
goto power_off;

ret = ssd130x_buf_alloc(ssd130x);
if (ret)
goto power_off;

ssd130x_write_cmd(ssd130x, 1, SSD130X_DISPLAY_ON);

backlight_enable(ssd130x->bl_dev);
Expand All @@ -744,8 +765,6 @@ static void ssd130x_encoder_helper_atomic_disable(struct drm_encoder *encoder,

ssd130x_write_cmd(ssd130x, 1, SSD130X_DISPLAY_OFF);

ssd130x_buf_free(ssd130x);

ssd130x_power_off(ssd130x);
}

Expand Down

0 comments on commit 40eba39

Please sign in to comment.