Re: [lvc-project] [PATCH v2] drm/amd/pm: check return value of amdgpu_irq_add_id()
On Wed, 02. Oct 16:01, Igor Artemiev wrote:
> amdgpu_irq_ad_id() may fail and the irq handlers will not be registered.
> This patch adds error code check.
>
> Found by Linux Verification Center (linuxtesting.org) with static
> analysis tool SVACE.
>
> Signed-off-by: Igor Artemiev
> ---
> v2: Remove the cast to struct amdgpu_device as Christian König
> suggested.
>
> .../drm/amd/pm/powerplay/hwmgr/smu_helper.c | 19 ---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu_helper.c
> b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu_helper.c
> index 79a566f3564a..50a3085c00aa 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu_helper.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu_helper.c
> @@ -647,28 +647,41 @@ int smu9_register_irq_handlers(struct pp_hwmgr *hwmgr)
> {
> struct amdgpu_irq_src *source =
> kzalloc(sizeof(struct amdgpu_irq_src), GFP_KERNEL);
> + int ret;
>
> if (!source)
> return -ENOMEM;
>
> source->funcs = &smu9_irq_funcs;
>
> - amdgpu_irq_add_id((struct amdgpu_device *)(hwmgr->adev),
> + ret = amdgpu_irq_add_id(hwmgr->adev,
> SOC15_IH_CLIENTID_THM,
> THM_9_0__SRCID__THM_DIG_THERM_L2H,
> source);
> - amdgpu_irq_add_id((struct amdgpu_device *)(hwmgr->adev),
> + if (ret)
> + goto err;
> +
> + ret = amdgpu_irq_add_id(hwmgr->adev,
> SOC15_IH_CLIENTID_THM,
> THM_9_0__SRCID__THM_DIG_THERM_H2L,
> source);
> + if (ret)
> + goto err;
>
> /* Register CTF(GPIO_19) interrupt */
> - amdgpu_irq_add_id((struct amdgpu_device *)(hwmgr->adev),
> + ret = amdgpu_irq_add_id(hwmgr->adev,
> SOC15_IH_CLIENTID_ROM_SMUIO,
> SMUIO_9_0__SRCID__SMUIO_GPIO19,
> source);
> + if (ret)
> + goto err;
>
> return 0;
> +
> +err:
> + kfree(source);
Oh, the calltrace looks like:
hwmgr_sw_init()
phm_register_irq_handlers()
->register_irq_handlers()
smu9_register_irq_handlers()
And the return value of phm_register_irq_handlers() is not processed and
the error is not reported anywhere, so I guess there is a risk of
use-after-free: the source pointer may have been already registered by
some of amdgpu_irq_add_id() calls before the error occured.
The similar code exists in smu7_register_irq_handlers(), maybe should be
fixed as well.
Alex, is https://gitlab.freedesktop.org/agd5f/linux a public repo this
patch should go in? I'd suggest to drop the patch and ask Igor to do a
complete fix or, if dropping is not possible now, fix it by another patch.
For the latter one I can do this myself but it would be nice to refer to
the current patch via a git hash (it's probably not published yet in your
repo).
> +
> + return ret;
> }
>
> void *smu_atom_get_data_table(void *dev, uint32_t table, uint16_t *size,
> --
> 2.39.2
[PATCH v2] dma-buf: handle testing kthreads creation failure
kthread creation may possibly fail inside race_signal_callback(). In
such a case stop the already started threads, put the already taken
references to them and return with error code.
Found by Linux Verification Center (linuxtesting.org).
Fixes: 2989f6451084 ("dma-buf: Add selftests for dma-fence")
Cc: sta...@vger.kernel.org
Signed-off-by: Fedor Pchelkin
---
v2: use kthread_stop_put() to actually put the last reference as
T.J. Mercier noticed;
link to v1:
https://lore.kernel.org/lkml/20240522122326.696928-1-pchel...@ispras.ru/
drivers/dma-buf/st-dma-fence.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/dma-buf/st-dma-fence.c b/drivers/dma-buf/st-dma-fence.c
index b7c6f7ea9e0c..6a1bfcd0cc21 100644
--- a/drivers/dma-buf/st-dma-fence.c
+++ b/drivers/dma-buf/st-dma-fence.c
@@ -540,6 +540,12 @@ static int race_signal_callback(void *arg)
t[i].before = pass;
t[i].task = kthread_run(thread_signal_callback, &t[i],
"dma-fence:%d", i);
+ if (IS_ERR(t[i].task)) {
+ ret = PTR_ERR(t[i].task);
+ while (--i >= 0)
+ kthread_stop_put(t[i].task);
+ return ret;
+ }
get_task_struct(t[i].task);
}
--
2.39.2
[PATCH] dma-buf: handle testing kthreads creation failure
kthread creation may possibly fail inside race_signal_callback(). In
such case stop the already started threads and return with error code.
Found by Linux Verification Center (linuxtesting.org).
Fixes: 2989f6451084 ("dma-buf: Add selftests for dma-fence")
Cc: sta...@vger.kernel.org
Signed-off-by: Fedor Pchelkin
---
drivers/dma-buf/st-dma-fence.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/dma-buf/st-dma-fence.c b/drivers/dma-buf/st-dma-fence.c
index b7c6f7ea9e0c..ab1ec4631578 100644
--- a/drivers/dma-buf/st-dma-fence.c
+++ b/drivers/dma-buf/st-dma-fence.c
@@ -540,6 +540,12 @@ static int race_signal_callback(void *arg)
t[i].before = pass;
t[i].task = kthread_run(thread_signal_callback, &t[i],
"dma-fence:%d", i);
+ if (IS_ERR(t[i].task)) {
+ ret = PTR_ERR(t[i].task);
+ while (--i >= 0)
+ kthread_stop(t[i].task);
+ return ret;
+ }
get_task_struct(t[i].task);
}
--
2.39.2
Re: [lvc-project] [PATCH] [RFC] dma-buf: fix race condition between poll and close
On Fri, 03. May 14:08, Dmitry Antipov wrote:
> On 5/3/24 11:18 AM, Christian König wrote:
>
> > Attached is a compile only tested patch, please verify if it fixes your
> > problem.
>
> LGTM, and this is similar to get_file() in __pollwait() and fput() in
> free_poll_entry() used in implementation of poll(). Please resubmit to
> linux-fsdevel@ including the following:
>
> Reported-by: syzbot+5d4cb6b4409edfd18...@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=5d4cb6b4409edfd18646
> Tested-by: Dmitry Antipov
I guess the problem is addressed by commit 4efaa5acf0a1 ("epoll: be better
about file lifetimes") which was pushed upstream just before v6.9-rc7.
Link: https://lore.kernel.org/lkml/2d631f0615918...@google.com/
[PATCH] drm/ttm: fix ttm pool initialization for no-dma-device drivers
QXL driver doesn't use any device for DMA mappings or allocations so
dev_to_node() will panic inside ttm_device_init() on NUMA systems:
general protection fault, probably for non-canonical address
0xdc7a: [#1] PREEMPT SMP KASAN NOPTI
KASAN: null-ptr-deref in range [0x03d0-0x03d7]
CPU: 1 PID: 1 Comm: swapper/0 Not tainted 6.7.0+ #9
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
rel-1.16.2-3-gd478f380-rebuilt.opensuse.org 04/01/2014
RIP: 0010:ttm_device_init+0x10e/0x340
Call Trace:
qxl_ttm_init+0xaa/0x310
qxl_device_init+0x1071/0x2000
qxl_pci_probe+0x167/0x3f0
local_pci_probe+0xe1/0x1b0
pci_device_probe+0x29d/0x790
really_probe+0x251/0x910
__driver_probe_device+0x1ea/0x390
driver_probe_device+0x4e/0x2e0
__driver_attach+0x1e3/0x600
bus_for_each_dev+0x12d/0x1c0
bus_add_driver+0x25a/0x590
driver_register+0x15c/0x4b0
qxl_pci_driver_init+0x67/0x80
do_one_initcall+0xf5/0x5d0
kernel_init_freeable+0x637/0xb10
kernel_init+0x1c/0x2e0
ret_from_fork+0x48/0x80
ret_from_fork_asm+0x1b/0x30
Modules linked in:
---[ end trace ]---
RIP: 0010:ttm_device_init+0x10e/0x340
Fall back to NUMA_NO_NODE if there is no device for DMA.
Found by Linux Verification Center (linuxtesting.org).
Fixes: b0a7ce53d494 ("drm/ttm: Schedule delayed_delete worker closer")
Signed-off-by: Fedor Pchelkin
---
drivers/gpu/drm/ttm/ttm_device.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index f5187b384ae9..4130945052ed 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -195,7 +195,7 @@ int ttm_device_init(struct ttm_device *bdev, const struct
ttm_device_funcs *func
bool use_dma_alloc, bool use_dma32)
{
struct ttm_global *glob = &ttm_glob;
- int ret;
+ int ret, nid;
if (WARN_ON(vma_manager == NULL))
return -EINVAL;
@@ -215,7 +215,12 @@ int ttm_device_init(struct ttm_device *bdev, const struct
ttm_device_funcs *func
ttm_sys_man_init(bdev);
- ttm_pool_init(&bdev->pool, dev, dev_to_node(dev), use_dma_alloc,
use_dma32);
+ if (dev)
+ nid = dev_to_node(dev);
+ else
+ nid = NUMA_NO_NODE;
+
+ ttm_pool_init(&bdev->pool, dev, nid, use_dma_alloc, use_dma32);
bdev->vma_manager = vma_manager;
spin_lock_init(&bdev->lru_lock);
--
2.43.0
[PATCH 5.10 1/1] drm/qxl: fix UAF on handle creation
ode_create_dumb linux/drivers/gpu/drm/drm_dumb_buffers.c:96
drm_mode_create_dumb_ioctl+0x1f5/0x2d0
linux/drivers/gpu/drm/drm_dumb_buffers.c:102
drm_ioctl_kernel+0x21d/0x430 linux/drivers/gpu/drm/drm_ioctl.c:788
drm_ioctl+0x56f/0xcc0 linux/drivers/gpu/drm/drm_ioctl.c:891
vfs_ioctl linux/fs/ioctl.c:51
__do_sys_ioctl linux/fs/ioctl.c:870
__se_sys_ioctl linux/fs/ioctl.c:856
__x64_sys_ioctl+0x13d/0x1c0 linux/fs/ioctl.c:856
do_syscall_x64 linux/arch/x86/entry/common.c:50
do_syscall_64+0x5b/0x90 linux/arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x72/0xdc linux/arch/x86/entry/entry_64.S:120
Freed by task 515:
kasan_save_stack+0x38/0x70 linux/mm/kasan/common.c:45
kasan_set_track+0x25/0x40 linux/mm/kasan/common.c:52
kasan_save_free_info+0x2e/0x60 linux/mm/kasan/generic.c:521
kasan_slab_free linux/mm/kasan/common.c:236
kasan_slab_free+0x180/0x1f0 linux/mm/kasan/common.c:200
__kasan_slab_free+0x12/0x30 linux/mm/kasan/common.c:244
kasan_slab_free linux/./include/linux/kasan.h:162
slab_free_hook linux/mm/slub.c:1781
slab_free_freelist_hook+0xd2/0x1a0 linux/mm/slub.c:1807
slab_free linux/mm/slub.c:3787
__kmem_cache_free+0x196/0x2d0 linux/mm/slub.c:3800
kfree+0x78/0x120 linux/mm/slab_common.c:1019
qxl_ttm_bo_destroy+0x140/0x1a0 linux/drivers/gpu/drm/qxl/qxl_object.c:49
ttm_bo_release+0x678/0xa30 linux/drivers/gpu/drm/ttm/ttm_bo.c:381
kref_put linux/./include/linux/kref.h:65
ttm_bo_put+0x50/0x80 linux/drivers/gpu/drm/ttm/ttm_bo.c:393
qxl_gem_object_free+0x3e/0x60 linux/drivers/gpu/drm/qxl/qxl_gem.c:42
drm_gem_object_free+0x5c/0x90 linux/drivers/gpu/drm/drm_gem.c:974
kref_put linux/./include/linux/kref.h:65
__drm_gem_object_put linux/./include/drm/drm_gem.h:431
drm_gem_object_put linux/./include/drm/drm_gem.h:444
qxl_gem_object_create_with_handle+0x151/0x180
linux/drivers/gpu/drm/qxl/qxl_gem.c:100
qxl_mode_dumb_create+0x1cd/0x400 linux/drivers/gpu/drm/qxl/qxl_dumb.c:63
drm_mode_create_dumb linux/drivers/gpu/drm/drm_dumb_buffers.c:96
drm_mode_create_dumb_ioctl+0x1f5/0x2d0
linux/drivers/gpu/drm/drm_dumb_buffers.c:102
drm_ioctl_kernel+0x21d/0x430 linux/drivers/gpu/drm/drm_ioctl.c:788
drm_ioctl+0x56f/0xcc0 linux/drivers/gpu/drm/drm_ioctl.c:891
vfs_ioctl linux/fs/ioctl.c:51
__do_sys_ioctl linux/fs/ioctl.c:870
__se_sys_ioctl linux/fs/ioctl.c:856
__x64_sys_ioctl+0x13d/0x1c0 linux/fs/ioctl.c:856
do_syscall_x64 linux/arch/x86/entry/common.c:50
do_syscall_64+0x5b/0x90 linux/arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x72/0xdc linux/arch/x86/entry/entry_64.S:120
The buggy address belongs to the object at 88801136c000
which belongs to the cache kmalloc-1k of size 1024
The buggy address is located 576 bytes inside of
freed 1024-byte region [88801136c000, 88801136c400)
The buggy address belongs to the physical page:
page:89fc329b refcount:1 mapcount:0 mapping: index:0x0
pfn:0x11368
head:89fc329b order:3 entire_mapcount:0 nr_pages_mapped:0 pincount:0
flags: 0xfc0010200(slab|head|node=0|zone=1|lastcpupid=0x1f)
raw: 000fc0010200 888007841dc0 dead0122
raw: 80100010 0001
page dumped because: kasan: bad access detected
Memory state around the buggy address:
88801136c100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
88801136c180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>88801136c200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
88801136c280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
88801136c300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==
Disabling lock debugging due to kernel taint
Instead of returning a weak reference to the qxl_bo object, return the
created drm_gem_object and let the caller decrement the reference count
when it no longer needs it. As a convenience, if the caller is not
interested in the gobj object, it can pass NULL to the parameter and the
reference counting is descremented internally.
The bug and the reproducer were originally found by the Zero Day Initiative
project (ZDI-CAN-20940).
Link: https://www.zerodayinitiative.com/
Signed-off-by: Wander Lairson Costa
Cc: sta...@vger.kernel.org
Reviewed-by: Dave Airlie
Signed-off-by: Dave Airlie
Link:
https://patchwork.freedesktop.org/patch/msgid/20230814165119.90847-1-wan...@redhat.com
[pchelkin: The problem can be reproduced on 5.10 stable. It lacks commit
f4a84e165e6d ("drm/qxl: allocate dumb buffers in ram"). Adjust a small
conflict regarding that commit: it affects only where the buffers are
placed.]
Signed-off-by: Fedor Pchelkin
---
drivers/gpu/drm/qxl/qxl_drv.h | 2 +-
drivers/gpu/drm/qxl/qxl_dumb.c | 5 -
drivers/gpu/drm/qxl/qxl_gem.c | 25 +
drivers/gpu/drm/qxl/qxl_ioctl.c | 6 ++
4 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index aae90a9ee1
[PATCH 5.10 0/1] drm/qxl: fix UAF on handle creation
The bug `KASAN: slab-use-after-free in qxl_mode_dumb_create` is reproduced on 5.10 stable branch. The problem has been fixed by the following patch which can be cleanly applied to 5.10. The fix is already included in all stable branches starting from 5.15. Link to the "failed to apply to 5.10" report [1]. [1]: https://lore.kernel.org/stable/2023082121-mumps-residency-9108@gregkh/
[PATCH] drm/exynos: gsc: minor fix for loop iteration in gsc_runtime_resume
Do not forget to call clk_disable_unprepare() on the first element of
ctx->clocks array.
Found by Linux Verification Center (linuxtesting.org).
Fixes: 8b7d3ec83aba ("drm/exynos: gsc: Convert driver to IPP v2 core API")
Signed-off-by: Fedor Pchelkin
---
drivers/gpu/drm/exynos/exynos_drm_gsc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
index 34cdabc30b4f..5302bebbe38c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
@@ -1342,7 +1342,7 @@ static int __maybe_unused gsc_runtime_resume(struct
device *dev)
for (i = 0; i < ctx->num_clocks; i++) {
ret = clk_prepare_enable(ctx->clocks[i]);
if (ret) {
- while (--i > 0)
+ while (--i >= 0)
clk_disable_unprepare(ctx->clocks[i]);
return ret;
}
--
2.43.0
[PATCH] drm/tegra: put drm_gem_object ref on error in tegra_fb_create
Inside tegra_fb_create(), drm_gem_object_lookup() increments ref count of
the found object. But if the following size check fails then the last
found object's ref count should be put there as the unreferencing loop
can't detect this situation.
Found by Linux Verification Center (linuxtesting.org).
Fixes: de2ba664c30f ("gpu: host1x: drm: Add memory manager and fb")
Signed-off-by: Fedor Pchelkin
---
drivers/gpu/drm/tegra/fb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
index a719af1dc9a5..46170753699d 100644
--- a/drivers/gpu/drm/tegra/fb.c
+++ b/drivers/gpu/drm/tegra/fb.c
@@ -159,6 +159,7 @@ struct drm_framebuffer *tegra_fb_create(struct drm_device
*drm,
if (gem->size < size) {
err = -EINVAL;
+ drm_gem_object_put(gem);
goto unreference;
}
--
2.43.0
Re: [PATCH] drm/crtc: do not release uninitialized connector reference
On 23/07/21 01:15PM, Fedor Pchelkin wrote:
> Inside drm_mode_setcrtc() connector_set is allocated using kmalloc_array()
> so its values are uninitialized. When filling this array with actual
> pointers to drm connector objects, an error caused with invalid ioctl
> request data may occur leading us to put references to already taken
> objects. However, the last elements of the array are left uninitialized
> which makes drm_connector_put() to be called with an invalid argument.
>
> We can obviously just initialize the array with kcalloc() but the current
> fix chose a slightly different way.
>
> The index of failing array element is known so just put references to the
> array members with lower indices.
>
> The temporary 'connector' pointer seems to be redundant as we can directly
> fill the connector_set elements and thus avoid unnecessary NULL
> assignments and checks.
>
> Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
>
> Fixes: b164d31f50b2 ("drm/modes: add connector reference counting. (v2)")
> Signed-off-by: Fedor Pchelkin
I'm sorry for bothering everyone with this issue, but status of the patch
here [1] is still 'New', and I have no means to deduce whether the
subsystem maintainers didn't have time to review (it is totally
understandable as the amount of patches is enormous) or the patch was
missed somehow.
[1]:
https://patchwork.kernel.org/project/dri-devel/patch/20230721101600.4392-1-pchel...@ispras.ru/
[PATCH] drm/crtc: do not release uninitialized connector reference
Inside drm_mode_setcrtc() connector_set is allocated using kmalloc_array()
so its values are uninitialized. When filling this array with actual
pointers to drm connector objects, an error caused with invalid ioctl
request data may occur leading us to put references to already taken
objects. However, the last elements of the array are left uninitialized
which makes drm_connector_put() to be called with an invalid argument.
We can obviously just initialize the array with kcalloc() but the current
fix chose a slightly different way.
The index of failing array element is known so just put references to the
array members with lower indices.
The temporary 'connector' pointer seems to be redundant as we can directly
fill the connector_set elements and thus avoid unnecessary NULL
assignments and checks.
Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
Fixes: b164d31f50b2 ("drm/modes: add connector reference counting. (v2)")
Signed-off-by: Fedor Pchelkin
---
drivers/gpu/drm/drm_crtc.c | 22 --
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index df9bf3c9206e..2a29c3cf12de 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -709,7 +709,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
struct drm_mode_crtc *crtc_req = data;
struct drm_crtc *crtc;
struct drm_plane *plane;
- struct drm_connector **connector_set = NULL, *connector;
+ struct drm_connector **connector_set = NULL;
struct drm_framebuffer *fb = NULL;
struct drm_display_mode *mode = NULL;
struct drm_mode_set set;
@@ -852,25 +852,22 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
}
for (i = 0; i < crtc_req->count_connectors; i++) {
- connector_set[i] = NULL;
set_connectors_ptr = (uint32_t __user *)(unsigned
long)crtc_req->set_connectors_ptr;
if (get_user(out_id, &set_connectors_ptr[i])) {
ret = -EFAULT;
goto out;
}
- connector = drm_connector_lookup(dev, file_priv,
out_id);
- if (!connector) {
+ connector_set[i] = drm_connector_lookup(dev, file_priv,
out_id);
+ if (!connector_set[i]) {
DRM_DEBUG_KMS("Connector id %d unknown\n",
out_id);
ret = -ENOENT;
goto out;
}
DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
- connector->base.id,
- connector->name);
-
- connector_set[i] = connector;
+ connector_set[i]->base.id,
+ connector_set[i]->name);
}
}
@@ -891,12 +888,9 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
if (fb)
drm_framebuffer_put(fb);
- if (connector_set) {
- for (i = 0; i < crtc_req->count_connectors; i++) {
- if (connector_set[i])
- drm_connector_put(connector_set[i]);
- }
- }
+ if (connector_set)
+ while (--i >= 0)
+ drm_connector_put(connector_set[i]);
kfree(connector_set);
drm_mode_destroy(dev, mode);
--
2.41.0
[PATCH 5.4/5.10 1/1] drm/atomic: Don't pollute crtc_state->mode_blob with error pointers
From: Ville Syrjälä
commit 439cf34c8e0a8a33d8c15a31be1b7423426bc765 upstream.
Make sure we don't assign an error pointer to crtc_state->mode_blob
as that will break all kinds of places that assume either NULL or a
valid pointer (eg. drm_property_blob_put()).
Cc: sta...@vger.kernel.org
Reported-by: fuyufan
Signed-off-by: Ville Syrjälä
Link:
https://patchwork.freedesktop.org/patch/msgid/20220209091928.14766-1-ville.syrj...@linux.intel.com
Acked-by: Maxime Ripard
Signed-off-by: Fedor Pchelkin
---
drivers/gpu/drm/drm_atomic_uapi.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index 25c269bc4681..b6062833370f 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -75,15 +75,17 @@ int drm_atomic_set_mode_for_crtc(struct drm_crtc_state
*state,
state->mode_blob = NULL;
if (mode) {
+ struct drm_property_blob *blob;
+
drm_mode_convert_to_umode(&umode, mode);
- state->mode_blob =
- drm_property_create_blob(state->crtc->dev,
-sizeof(umode),
-&umode);
- if (IS_ERR(state->mode_blob))
- return PTR_ERR(state->mode_blob);
+ blob = drm_property_create_blob(crtc->dev,
+ sizeof(umode), &umode);
+ if (IS_ERR(blob))
+ return PTR_ERR(blob);
drm_mode_copy(&state->mode, mode);
+
+ state->mode_blob = blob;
state->enable = true;
DRM_DEBUG_ATOMIC("Set [MODE:%s] for [CRTC:%d:%s] state %p\n",
mode->name, crtc->base.id, crtc->name, state);
--
2.34.1
[PATCH 5.4/5.10 0/1] drm/atomic: Don't pollute crtc_state->mode_blob with error pointers
general protection fault in drm_mode_object_put() is hit on 5.4/5.10 if drm_property_create_blob() fails for some reason and state->mode_blob is assigned an error pointer which is not treated correctly in some places as mentioned in patch description. The following patch fixes the issue and can be cleanly applied to 5.4/5.10 stable branches. Seems the patch could not be initially backported due to DRM_DEBUG_ATOMIC -> drm_dbg_atomic() change.
