Quoting Christian König (2020-05-20 13:54:55)
Am 20.05.20 um 14:49 schrieb Chris Wilson:
Quoting Christian König (2020-05-20 13:19:42)
Am 20.05.20 um 14:14 schrieb Nirmoy Das:
drm_gem_fb_destroy() calls drm_gem_object_put() with NULL obj causing: [ 11.584209] BUG: kernel NULL pointer dereference, address: 0000000000000000 [ 11.584213] #PF: supervisor write access in kernel mode [ 11.584215] #PF: error_code(0x0002) - not-present page [ 11.584216] PGD 0 P4D 0 [ 11.584220] Oops: 0002 [#1] SMP NOPTI [ 11.584223] CPU: 7 PID: 1571 Comm: gnome-shell Tainted: G E 5.7.0-rc1-1-default+ #27 [ 11.584225] Hardware name: Micro-Star International Co., Ltd. MS-7A31/X370 XPOWER GAMING TITANIUM (MS-7A31), BIOS 1.MR 12/03/2019 [ 11.584237] RIP: 0010:drm_gem_fb_destroy+0x28/0x70 [drm_kms_helper]
<snip> [ 11.584256] Call Trace: [ 11.584279] drm_mode_rmfb+0x189/0x1c0 [drm] [ 11.584299] ? drm_mode_rmfb+0x1c0/0x1c0 [drm] [ 11.584314] drm_ioctl_kernel+0xaa/0xf0 [drm] [ 11.584329] drm_ioctl+0x1ff/0x3b0 [drm] [ 11.584347] ? drm_mode_rmfb+0x1c0/0x1c0 [drm] [ 11.584421] amdgpu_drm_ioctl+0x49/0x80 [amdgpu] [ 11.584427] ksys_ioctl+0x87/0xc0 [ 11.584430] __x64_sys_ioctl+0x16/0x20 [ 11.584434] do_syscall_64+0x5f/0x240 [ 11.584438] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [ 11.584440] RIP: 0033:0x7f0ef80f7227
Signed-off-by: Nirmoy Das nirmoy.das@amd.com
Fixes: .... missing here. Apart from that Reviewed-by: Christian König christian.koenig@amd.com.
Please resend with the tag added, then I'm going to push this to drm-misc-next asap.
Christian.
include/drm/drm_gem.h | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 52173abdf500..a13510346a9b 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -372,6 +372,9 @@ static inline void drm_gem_object_get(struct drm_gem_object *obj) static inline void drm_gem_object_put(struct drm_gem_object *obj) {
if (!obj)return;This adds several thousand NULL checks where there were previously none. I doubt the compiler eliminates them all.
I'd suggest reverting b5d250744ccc ("drm/gem: fold drm_gem_object_put_unlocked and __drm_gem_object_put()")
I didn't looked into this patch in detail, but allowing to call *_put() functions with NULL and nothing bad happens is rather common practice.
On the other hand I agree that this might cause a performance problem. How many sites do we have which could call drm_gem_object_put() with NULL?
Just to put this in a tiny bit of perspective, for i915.ko
add/remove: 0/0 grow/shrink: 141/20 up/down: 2211/-525 (1686)
We've had flame wars for less. (Because it's easy to argue over little changes.) Now this is just from this patch and not the revert... The revert has no effect on code size. -Chris