Skip to content

Commit 3cbdc8d

Browse files
Akhil P Oommenrobclark
authored andcommitted
drm/msm: Fix a null pointer access in msm_gem_shrinker_count()
Adding an msm_gem_object object to the inactive_list before completing its initialization is a bad idea because shrinker may pick it up from the inactive_list. Fix this by making sure that the initialization is complete before moving the msm_obj object to the inactive list. This patch fixes the below error: [10027.553044] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000068 [10027.573305] Mem abort info: [10027.590160] ESR = 0x96000006 [10027.597905] EC = 0x25: DABT (current EL), IL = 32 bits [10027.614430] SET = 0, FnV = 0 [10027.624427] EA = 0, S1PTW = 0 [10027.632722] Data abort info: [10027.638039] ISV = 0, ISS = 0x00000006 [10027.647459] CM = 0, WnR = 0 [10027.654345] user pgtable: 4k pages, 39-bit VAs, pgdp=00000001e3a6a000 [10027.672681] [0000000000000068] pgd=0000000198c31003, pud=0000000198c31003, pmd=0000000000000000 [10027.693900] Internal error: Oops: 96000006 [#1] PREEMPT SMP [10027.738261] CPU: 3 PID: 214 Comm: kswapd0 Tainted: G S 5.4.40 #1 [10027.745766] Hardware name: Qualcomm Technologies, Inc. SC7180 IDP (DT) [10027.752472] pstate: 80c00009 (Nzcv daif +PAN +UAO) [10027.757409] pc : mutex_is_locked+0x14/0x2c [10027.761626] lr : msm_gem_shrinker_count+0x70/0xec [10027.766454] sp : ffffffc011323ad0 [10027.769867] x29: ffffffc011323ad0 x28: ffffffe677e4b878 [10027.775324] x27: 0000000000000cc0 x26: 0000000000000000 [10027.780783] x25: ffffff817114a708 x24: 0000000000000008 [10027.786242] x23: ffffff8023ab7170 x22: 0000000000000001 [10027.791701] x21: ffffff817114a080 x20: 0000000000000119 [10027.797160] x19: 0000000000000068 x18: 00000000000003bc [10027.802621] x17: 0000000004a34210 x16: 00000000000000c0 [10027.808083] x15: 0000000000000000 x14: 0000000000000000 [10027.813542] x13: ffffffe677e0a3c0 x12: 0000000000000000 [10027.819000] x11: 0000000000000000 x10: ffffff8174b94340 [10027.824461] x9 : 0000000000000000 x8 : 0000000000000000 [10027.829919] x7 : 00000000000001fc x6 : ffffffc011323c88 [10027.835373] x5 : 0000000000000001 x4 : ffffffc011323d80 [10027.840832] x3 : ffffffff0477b348 x2 : 0000000000000000 [10027.846290] x1 : ffffffc011323b68 x0 : 0000000000000068 [10027.851748] Call trace: [10027.854264] mutex_is_locked+0x14/0x2c [10027.858121] msm_gem_shrinker_count+0x70/0xec [10027.862603] shrink_slab+0xc0/0x4b4 [10027.866187] shrink_node+0x4a8/0x818 [10027.869860] kswapd+0x624/0x890 [10027.873097] kthread+0x11c/0x12c [10027.876424] ret_from_fork+0x10/0x18 [10027.880102] Code: f9000bf3 910003fd aa0003f3 d503201f (f9400268) [10027.886362] ---[ end trace df5849a1a3543251 ]--- [10027.891518] Kernel panic - not syncing: Fatal exception Signed-off-by: Akhil P Oommen <[email protected]> Signed-off-by: Rob Clark <[email protected]>
1 parent 3c12863 commit 3cbdc8d

File tree

1 file changed

+21
-15
lines changed

1 file changed

+21
-15
lines changed

drivers/gpu/drm/msm/msm_gem.c

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -996,10 +996,8 @@ int msm_gem_new_handle(struct drm_device *dev, struct drm_file *file,
996996

997997
static int msm_gem_new_impl(struct drm_device *dev,
998998
uint32_t size, uint32_t flags,
999-
struct drm_gem_object **obj,
1000-
bool struct_mutex_locked)
999+
struct drm_gem_object **obj)
10011000
{
1002-
struct msm_drm_private *priv = dev->dev_private;
10031001
struct msm_gem_object *msm_obj;
10041002

10051003
switch (flags & MSM_BO_CACHE_MASK) {
@@ -1025,15 +1023,6 @@ static int msm_gem_new_impl(struct drm_device *dev,
10251023
INIT_LIST_HEAD(&msm_obj->submit_entry);
10261024
INIT_LIST_HEAD(&msm_obj->vmas);
10271025

1028-
if (struct_mutex_locked) {
1029-
WARN_ON(!mutex_is_locked(&dev->struct_mutex));
1030-
list_add_tail(&msm_obj->mm_list, &priv->inactive_list);
1031-
} else {
1032-
mutex_lock(&dev->struct_mutex);
1033-
list_add_tail(&msm_obj->mm_list, &priv->inactive_list);
1034-
mutex_unlock(&dev->struct_mutex);
1035-
}
1036-
10371026
*obj = &msm_obj->base;
10381027

10391028
return 0;
@@ -1043,6 +1032,7 @@ static struct drm_gem_object *_msm_gem_new(struct drm_device *dev,
10431032
uint32_t size, uint32_t flags, bool struct_mutex_locked)
10441033
{
10451034
struct msm_drm_private *priv = dev->dev_private;
1035+
struct msm_gem_object *msm_obj;
10461036
struct drm_gem_object *obj = NULL;
10471037
bool use_vram = false;
10481038
int ret;
@@ -1063,14 +1053,15 @@ static struct drm_gem_object *_msm_gem_new(struct drm_device *dev,
10631053
if (size == 0)
10641054
return ERR_PTR(-EINVAL);
10651055

1066-
ret = msm_gem_new_impl(dev, size, flags, &obj, struct_mutex_locked);
1056+
ret = msm_gem_new_impl(dev, size, flags, &obj);
10671057
if (ret)
10681058
goto fail;
10691059

1060+
msm_obj = to_msm_bo(obj);
1061+
10701062
if (use_vram) {
10711063
struct msm_gem_vma *vma;
10721064
struct page **pages;
1073-
struct msm_gem_object *msm_obj = to_msm_bo(obj);
10741065

10751066
mutex_lock(&msm_obj->lock);
10761067

@@ -1105,6 +1096,15 @@ static struct drm_gem_object *_msm_gem_new(struct drm_device *dev,
11051096
mapping_set_gfp_mask(obj->filp->f_mapping, GFP_HIGHUSER);
11061097
}
11071098

1099+
if (struct_mutex_locked) {
1100+
WARN_ON(!mutex_is_locked(&dev->struct_mutex));
1101+
list_add_tail(&msm_obj->mm_list, &priv->inactive_list);
1102+
} else {
1103+
mutex_lock(&dev->struct_mutex);
1104+
list_add_tail(&msm_obj->mm_list, &priv->inactive_list);
1105+
mutex_unlock(&dev->struct_mutex);
1106+
}
1107+
11081108
return obj;
11091109

11101110
fail:
@@ -1127,6 +1127,7 @@ struct drm_gem_object *msm_gem_new(struct drm_device *dev,
11271127
struct drm_gem_object *msm_gem_import(struct drm_device *dev,
11281128
struct dma_buf *dmabuf, struct sg_table *sgt)
11291129
{
1130+
struct msm_drm_private *priv = dev->dev_private;
11301131
struct msm_gem_object *msm_obj;
11311132
struct drm_gem_object *obj;
11321133
uint32_t size;
@@ -1140,7 +1141,7 @@ struct drm_gem_object *msm_gem_import(struct drm_device *dev,
11401141

11411142
size = PAGE_ALIGN(dmabuf->size);
11421143

1143-
ret = msm_gem_new_impl(dev, size, MSM_BO_WC, &obj, false);
1144+
ret = msm_gem_new_impl(dev, size, MSM_BO_WC, &obj);
11441145
if (ret)
11451146
goto fail;
11461147

@@ -1165,6 +1166,11 @@ struct drm_gem_object *msm_gem_import(struct drm_device *dev,
11651166
}
11661167

11671168
mutex_unlock(&msm_obj->lock);
1169+
1170+
mutex_lock(&dev->struct_mutex);
1171+
list_add_tail(&msm_obj->mm_list, &priv->inactive_list);
1172+
mutex_unlock(&dev->struct_mutex);
1173+
11681174
return obj;
11691175

11701176
fail:

0 commit comments

Comments
 (0)