Skip to content

Commit

Permalink
drm/cma-helper: Fix crash in fbdev error path
Browse files Browse the repository at this point in the history 
Sergey Suloev reported a crash happening in drm_client_dev_hotplug()
when fbdev had failed to register.
[    9.124598] vc4_hdmi 3f902000.hdmi: ASoC: Failed to create component debugfs directory
[    9.147667] vc4_hdmi 3f902000.hdmi: vc4-hdmi-hifi <-> 3f902000.hdmi mapping ok
[    9.155184] vc4_hdmi 3f902000.hdmi: ASoC: no DMI vendor name!
[    9.166544] vc4-drm soc:gpu: bound 3f902000.hdmi (ops vc4_hdmi_ops [vc4])
[    9.173840] vc4-drm soc:gpu: bound 3f806000.vec (ops vc4_vec_ops [vc4])
[    9.181029] vc4-drm soc:gpu: bound 3f004000.txp (ops vc4_txp_ops [vc4])
[    9.188519] vc4-drm soc:gpu: bound 3f400000.hvs (ops vc4_hvs_ops [vc4])
[    9.195690] vc4-drm soc:gpu: bound 3f206000.pixelvalve (ops vc4_crtc_ops [vc4])
[    9.203523] vc4-drm soc:gpu: bound 3f207000.pixelvalve (ops vc4_crtc_ops [vc4])
[    9.215032] vc4-drm soc:gpu: bound 3f807000.pixelvalve (ops vc4_crtc_ops [vc4])
[    9.274785] vc4-drm soc:gpu: bound 3fc00000.v3d (ops vc4_v3d_ops [vc4])
[    9.290246] [drm] Initialized vc4 0.0.0 20140616 for soc:gpu on minor 0
[    9.297464] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[    9.304600] [drm] Driver supports precise vblank timestamp query.
[    9.382856] vc4-drm soc:gpu: [drm:drm_fb_helper_fbdev_setup [drm_kms_helper]] *ERROR* Failed to set fbdev configuration
[   10.404937] Unable to handle kernel paging request at virtual address 00330a656369768a
[   10.441620] [00330a656369768a] address between user and kernel address ranges
[   10.449087] Internal error: Oops: 96000004 [wiktorek140#1] PREEMPT SMP
[   10.454762] Modules linked in: brcmfmac vc4 drm_kms_helper cfg80211 drm rfkill smsc95xx brcmutil usbnet drm_panel_orientation_quirks raspberrypi_hwmon bcm2835_dma crc32_ce pwm_bcm2835 bcm2835_rng virt_dma rng_core i2c_bcm2835 ip_tables x_tables ipv6
[   10.477296] CPU: 2 PID: 45 Comm: kworker/2:1 Not tainted 4.19.0-rc5 wiktorek140#3
[   10.483934] Hardware name: Raspberry Pi 3 Model B Rev 1.2 (DT)
[   10.489966] Workqueue: events output_poll_execute [drm_kms_helper]
[   10.596515] Process kworker/2:1 (pid: 45, stack limit = 0x000000007e8924dc)
[   10.603590] Call trace:
[   10.606259]  drm_client_dev_hotplug+0x5c/0xb0 [drm]
[   10.611303]  drm_kms_helper_hotplug_event+0x30/0x40 [drm_kms_helper]
[   10.617849]  output_poll_execute+0xc4/0x1e0 [drm_kms_helper]
[   10.623616]  process_one_work+0x1c8/0x318
[   10.627695]  worker_thread+0x48/0x428
[   10.631420]  kthread+0xf8/0x128
[   10.634615]  ret_from_fork+0x10/0x18
[   10.638255] Code: 54000220 f9401261 aa1303e0 b4000141 (f9400c21)
[   10.644456] ---[ end trace c75b4a4b0e141908 ]---
The reason for this is that drm_fbdev_cma_init() removes the drm_client
when fbdev registration fails, but it doesn't remove the client from the
drm_device client list. So the client list now has a pointer that points
into the unknown and we have a 'use after free' situation.

Split drm_client_new() into drm_client_init() and drm_client_add() to fix
removal in the error path.

Fixes: 894a677f4b3e ("drm/cma-helper: Use the generic fbdev emulation")
Reported-by: Sergey Suloev <ssuloev@orpaltech.com>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

as the existing changes are not present.]
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20181001194536.57756-1-noralf@tronnes.org
Patch-mainline: 20181001194536.57756-1-noralf@tronnes.org @ 01/10/18, 07:45 p.m.
[stalek@codeaurora.org: drop changes for drm_fb_helper.c and drm_fb_cma_helper.c].

Change-Id: I7cd0651e06a55095672ed6c9e7a885a4cfdba075
Signed-off-by: Shubham Talekar <stalek@codeaurora.org>
  • Loading branch information
@notro
notro authored and Gerrit - the friendly Code Review server committed Sep 27, 2021
1 parent 154661a commit a011ebd
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 10 deletions.
33 changes: 25 additions & 8 deletions drivers/gpu/drm/drm_client.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,19 +58,20 @@ static void drm_client_close(struct drm_client_dev *client)
EXPORT_SYMBOL(drm_client_close);

/**
* drm_client_new - Create a DRM client
* drm_client_init - Initialise a DRM client
* @dev: DRM device
* @client: DRM client
* @name: Client name
* @funcs: DRM client functions (optional)
*
* This initialises the client and opens a &drm_file. Use drm_client_add() to complete the process.
* The caller needs to hold a reference on @dev before calling this function.
* The client is freed when the &drm_device is unregistered. See drm_client_release().
*
* Returns:
* Zero on success or negative error code on failure.
*/
int drm_client_new(struct drm_device *dev, struct drm_client_dev *client,
int drm_client_init(struct drm_device *dev, struct drm_client_dev *client,
const char *name, const struct drm_client_funcs *funcs)
{
int ret;
Expand All @@ -90,10 +91,6 @@ int drm_client_new(struct drm_device *dev, struct drm_client_dev *client,
if (ret)
goto err_put_module;

mutex_lock(&dev->clientlist_mutex);
list_add(&client->list, &dev->clientlist);
mutex_unlock(&dev->clientlist_mutex);

drm_dev_ref(dev);

return 0;
Expand All @@ -104,13 +101,33 @@ int drm_client_new(struct drm_device *dev, struct drm_client_dev *client,

return ret;
}
EXPORT_SYMBOL(drm_client_new);
EXPORT_SYMBOL(drm_client_init);

/**
* drm_client_add - Add client to the device list
* @client: DRM client
*
* Add the client to the &drm_device client list to activate its callbacks.
* @client must be initialized by a call to drm_client_init(). After
* drm_client_add() it is no longer permissible to call drm_client_release()
* directly (outside the unregister callback), instead cleanup will happen
* automatically on driver unload.
*/
void drm_client_add(struct drm_client_dev *client)
{
struct drm_device *dev = client->dev;

mutex_lock(&dev->clientlist_mutex);
list_add(&client->list, &dev->clientlist);
mutex_unlock(&dev->clientlist_mutex);
}
EXPORT_SYMBOL(drm_client_add);

/**
* drm_client_release - Release DRM client resources
* @client: DRM client
*
* Releases resources by closing the &drm_file that was opened by drm_client_new().
* Releases resources by closing the &drm_file that was opened by drm_client_init().
* It is called automatically if the &drm_client_funcs.unregister callback is _not_ set.
*
* This function should only be called from the unregister callback. An exception
Expand Down
4 changes: 2 additions & 2 deletions include/drm/drm_client.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,9 @@ struct drm_client_dev {
struct drm_file *file;
};

int drm_client_new(struct drm_device *dev, struct drm_client_dev *client,
int drm_client_init(struct drm_device *dev, struct drm_client_dev *client,
const char *name, const struct drm_client_funcs *funcs);
void drm_client_release(struct drm_client_dev *client);
void drm_client_add(struct drm_client_dev *client);

void drm_client_dev_unregister(struct drm_device *dev);
void drm_client_dev_hotplug(struct drm_device *dev);
Expand Down

0 comments on commit a011ebd

Please sign in to comment.