2023-10-24 17:08:24

by Rob Clark

[permalink] [raw]
Subject: [PATCH 1/2] drm/msm/gem: Demote userspace errors to DRM_UT_DRIVER

From: Rob Clark <[email protected]>

Error messages resulting from incorrect usage of the kernel uabi should
not spam dmesg by default. But it is useful to enable them to debug
userspace. So demote to DRM_UT_DRIVER.

Signed-off-by: Rob Clark <[email protected]>
---
drivers/gpu/drm/msm/msm_gem.c | 6 ++---
drivers/gpu/drm/msm/msm_gem_submit.c | 36 ++++++++++++++++------------
2 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index db1e748daa75..1113e6b2ec8e 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -226,9 +226,9 @@ static struct page **msm_gem_pin_pages_locked(struct drm_gem_object *obj,

msm_gem_assert_locked(obj);

- if (GEM_WARN_ON(msm_obj->madv > madv)) {
- DRM_DEV_ERROR(obj->dev->dev, "Invalid madv state: %u vs %u\n",
- msm_obj->madv, madv);
+ if (msm_obj->madv > madv) {
+ DRM_DEV_DEBUG_DRIVER(obj->dev->dev, "Invalid madv state: %u vs %u\n",
+ msm_obj->madv, madv);
return ERR_PTR(-EBUSY);
}

diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index a105ca1f2102..9f16af761bf0 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -17,6 +17,12 @@
#include "msm_gem.h"
#include "msm_gpu_trace.h"

+/* For userspace errors, use DRM_UT_DRIVER.. so that userspace can enable
+ * error msgs for debugging, but we don't spam dmesg by default
+ */
+#define SUBMIT_ERROR(submit, fmt, ...) \
+ DRM_DEV_DEBUG_DRIVER((submit)->dev->dev, fmt, ##__VA_ARGS__)
+
/*
* Cmdstream submission:
*/
@@ -136,7 +142,7 @@ static int submit_lookup_objects(struct msm_gem_submit *submit,

if ((submit_bo.flags & ~MSM_SUBMIT_BO_FLAGS) ||
!(submit_bo.flags & MANDATORY_FLAGS)) {
- DRM_ERROR("invalid flags: %x\n", submit_bo.flags);
+ SUBMIT_ERROR(submit, "invalid flags: %x\n", submit_bo.flags);
ret = -EINVAL;
i = 0;
goto out;
@@ -158,7 +164,7 @@ static int submit_lookup_objects(struct msm_gem_submit *submit,
*/
obj = idr_find(&file->object_idr, submit->bos[i].handle);
if (!obj) {
- DRM_ERROR("invalid handle %u at index %u\n", submit->bos[i].handle, i);
+ SUBMIT_ERROR(submit, "invalid handle %u at index %u\n", submit->bos[i].handle, i);
ret = -EINVAL;
goto out_unlock;
}
@@ -202,13 +208,13 @@ static int submit_lookup_cmds(struct msm_gem_submit *submit,
case MSM_SUBMIT_CMD_CTX_RESTORE_BUF:
break;
default:
- DRM_ERROR("invalid type: %08x\n", submit_cmd.type);
+ SUBMIT_ERROR(submit, "invalid type: %08x\n", submit_cmd.type);
return -EINVAL;
}

if (submit_cmd.size % 4) {
- DRM_ERROR("non-aligned cmdstream buffer size: %u\n",
- submit_cmd.size);
+ SUBMIT_ERROR(submit, "non-aligned cmdstream buffer size: %u\n",
+ submit_cmd.size);
ret = -EINVAL;
goto out;
}
@@ -306,8 +312,8 @@ static int submit_lock_objects(struct msm_gem_submit *submit)

fail:
if (ret == -EALREADY) {
- DRM_ERROR("handle %u at index %u already on submit list\n",
- submit->bos[i].handle, i);
+ SUBMIT_ERROR(submit, "handle %u at index %u already on submit list\n",
+ submit->bos[i].handle, i);
ret = -EINVAL;
}

@@ -448,8 +454,8 @@ static int submit_bo(struct msm_gem_submit *submit, uint32_t idx,
struct drm_gem_object **obj, uint64_t *iova, bool *valid)
{
if (idx >= submit->nr_bos) {
- DRM_ERROR("invalid buffer index: %u (out of %u)\n",
- idx, submit->nr_bos);
+ SUBMIT_ERROR(submit, "invalid buffer index: %u (out of %u)\n",
+ idx, submit->nr_bos);
return -EINVAL;
}

@@ -475,7 +481,7 @@ static int submit_reloc(struct msm_gem_submit *submit, struct drm_gem_object *ob
return 0;

if (offset % 4) {
- DRM_ERROR("non-aligned cmdstream buffer: %u\n", offset);
+ SUBMIT_ERROR(submit, "non-aligned cmdstream buffer: %u\n", offset);
return -EINVAL;
}

@@ -497,8 +503,8 @@ static int submit_reloc(struct msm_gem_submit *submit, struct drm_gem_object *ob
bool valid;

if (submit_reloc.submit_offset % 4) {
- DRM_ERROR("non-aligned reloc offset: %u\n",
- submit_reloc.submit_offset);
+ SUBMIT_ERROR(submit, "non-aligned reloc offset: %u\n",
+ submit_reloc.submit_offset);
ret = -EINVAL;
goto out;
}
@@ -508,7 +514,7 @@ static int submit_reloc(struct msm_gem_submit *submit, struct drm_gem_object *ob

if ((off >= (obj->size / 4)) ||
(off < last_offset)) {
- DRM_ERROR("invalid offset %u at reloc %u\n", off, i);
+ SUBMIT_ERROR(submit, "invalid offset %u at reloc %u\n", off, i);
ret = -EINVAL;
goto out;
}
@@ -881,7 +887,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
if (!submit->cmd[i].size ||
((submit->cmd[i].size + submit->cmd[i].offset) >
obj->size / 4)) {
- DRM_ERROR("invalid cmdstream size: %u\n", submit->cmd[i].size * 4);
+ SUBMIT_ERROR(submit, "invalid cmdstream size: %u\n", submit->cmd[i].size * 4);
ret = -EINVAL;
goto out;
}
@@ -893,7 +899,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,

if (!gpu->allow_relocs) {
if (submit->cmd[i].nr_relocs) {
- DRM_ERROR("relocs not allowed\n");
+ SUBMIT_ERROR(submit, "relocs not allowed\n");
ret = -EINVAL;
goto out;
}
--
2.41.0


2023-10-24 17:08:55

by Rob Clark

[permalink] [raw]
Subject: [PATCH 2/2] drm/msm/gem: Demote allocations to __GFP_NOWARN

From: Rob Clark <[email protected]>

For allocations with userspace controlled size, we should not warn on
allocation failure. Fixes KASAN splat:

WARNING: CPU: 6 PID: 29557 at mm/page_alloc.c:5398 __alloc_pages+0x160c/0x2204
Modules linked in: bridge stp llc hci_vhci tun veth xt_cgroup uinput xt_MASQUERADE rfcomm ip6table_nat fuse 8021q r8153_ecm cdc_ether usbnet r8152 mii venus_enc venus_dec uvcvideo algif_hash algif_skcipher af_alg qcom_spmi_adc_tm5 qcom_spmi_adc5 qcom_vadc_common qcom_spmi_temp_alarm cros_ec_typec typec hci_uart btqca qcom_stats snd_soc_sc7180 venus_core ath10k_snoc ath10k_core ath coresight_tmc coresight_replicator coresight_etm4x coresight_funnel snd_soc_lpass_sc7180 mac80211 coresight bluetooth ecdh_generic ecc cfg80211 cros_ec_sensorhub lzo_rle lzo_compress zram joydev
CPU: 6 PID: 29557 Comm: syz-executor Not tainted 5.15.110-lockdep-19320-g89d010b0a9df #1 45bdd400697a78353f2927c116615abba810e5dd
Hardware name: Google Kingoftown (DT)
pstate: 20400009 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : __alloc_pages+0x160c/0x2204
lr : __alloc_pages+0x58/0x2204
sp : ffffffc0214176c0
x29: ffffffc0214178a0 x28: ffffff801f7b4000 x27: 0000000000000000
x26: ffffff808a4fa000 x25: 1ffffff011290781 x24: ffffff808a59c000
x23: 0000000000000010 x22: ffffffc0080e6980 x21: 0000000000000010
x20: 0000000000000000 x19: 00000000080001f8 x18: 0000000000000000
x17: 0000000000000000 x16: 0000000000000000 x15: 0000000020000500
x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000001
x11: 0000000000000000 x10: 1ffffff804282f06 x9 : 0000000000000000
x8 : ffffffc021417848 x7 : 0000000000000000 x6 : ffffffc0082ac788
x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000010
x2 : 0000000000000008 x1 : 0000000000000000 x0 : ffffffc021417830
Call trace:
__alloc_pages+0x160c/0x2204
kmalloc_order+0x50/0xf4
kmalloc_order_trace+0x38/0x18c
__kmalloc+0x300/0x45c
msm_ioctl_gem_submit+0x284/0x5988
drm_ioctl_kernel+0x270/0x418
drm_ioctl+0x5e0/0xbf8
__arm64_sys_ioctl+0x154/0x1d0
invoke_syscall+0x98/0x278
el0_svc_common+0x214/0x274
do_el0_svc+0x9c/0x19c
el0_svc+0x5c/0xc0
el0t_64_sync_handler+0x78/0x108
el0t_64_sync+0x1a4/0x1a8

Signed-off-by: Rob Clark <[email protected]>
---
drivers/gpu/drm/msm/msm_gem_submit.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index 9f16af761bf0..6d8ec1337e8b 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -43,7 +43,7 @@ static struct msm_gem_submit *submit_create(struct drm_device *dev,
if (sz > SIZE_MAX)
return ERR_PTR(-ENOMEM);

- submit = kzalloc(sz, GFP_KERNEL);
+ submit = kzalloc(sz, GFP_KERNEL | __GFP_NOWARN);
if (!submit)
return ERR_PTR(-ENOMEM);

@@ -234,7 +234,7 @@ static int submit_lookup_cmds(struct msm_gem_submit *submit,
ret = -ENOMEM;
goto out;
}
- submit->cmd[i].relocs = kmalloc(sz, GFP_KERNEL);
+ submit->cmd[i].relocs = kmalloc(sz, GFP_KERNEL | __GFP_NOWARN);
if (!submit->cmd[i].relocs) {
ret = -ENOMEM;
goto out;
--
2.41.0