[Intel-gfx] [PATCH] drm/i915/gem: Add a check for object size for corner cases

Tue Feb 16 12:05:23 UTC 2021

Hi Chris,

-----Original Message-----
From: dri-devel <dri-devel-bounces at lists.freedesktop.org> On Behalf Of Chris Wilson
Sent: Monday, February 15, 2021 6:10 PM
To: Auld, Matthew <matthew.auld at intel.com>; Ram Moon, AnandX <anandx.ram.moon at intel.com>; Surendrakumar Upadhyay, TejaskumarX <tejaskumarx.surendrakumar.upadhyay at intel.com>; Ursulin, Tvrtko <tvrtko.ursulin at intel.com>; Jani Nikula <jani.nikula at linux.intel.com>; dri-devel at lists.freedesktop.org; intel-gfx at lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915/gem: Add a check for object size for corner cases

Quoting Ram Moon, AnandX (2021-02-15 12:29:17)
> Hi Chris,
> 
> -----Original Message-----
> From: dri-devel <dri-devel-bounces at lists.freedesktop.org> On Behalf Of 
> Chris Wilson
> Sent: Wednesday, February 10, 2021 4:15 PM
> To: Ram Moon, AnandX <anandx.ram.moon at intel.com>; Jani Nikula 
> <jani.nikula at linux.intel.com>; Auld, Matthew <matthew.auld at intel.com>; 
> Surendrakumar Upadhyay, TejaskumarX 
> <tejaskumarx.surendrakumar.upadhyay at intel.com>; Ursulin, Tvrtko 
> <tvrtko.ursulin at intel.com>; dri-devel at lists.freedesktop.org; 
> intel-gfx at lists.freedesktop.org
> Cc: Ram Moon, AnandX <anandx.ram.moon at intel.com>
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/gem: Add a check for object 
> size for corner cases
> 
> Quoting Anand Moon (2021-02-10 07:59:29)
> > Add check for object size to return appropriate error -E2BIG or 
> > -EINVAL to avoid WARM_ON and successful return for some testcase.
> 
> No. You miss the point of having those warnings. We need to inspect the code to remove the last remaining "int pagenum", and then we can remove the GEM_WARN_ON((sz) >> PAGE_SHIFT > INT_MAX). These are not emitted for users, only for us to motivate us into finally fixing the code.
> -Chris
> 
> Yes, I got your point these check are meant to catch up integer overflow.
> 
> I have check with the IGT testcase case  _gem_create_ and 
> _gem_userptr_blits_ which fails pass size *-1ull << 32*  left shift 
> and *0~* which leads to integer overflow ie  _negative_ size passed to create  i915_gem_create via ioctl  this leads to GM_WARN_ON.
> 
> Can we drop these testcase so that we don't break the kernel ?

The kernel rejects the ioctl with the expected errno. We leave a warning purely for the benefit of CI, only when compiled for debugging and not by default, so that we have a persistent reminder to do the code review.
What's broken?
-Chris

All though the testcase return with appropriate error we observe kernel taint see below.

Thanks
-Anand

IGT-Version: 1.25-g2982c998a (x86_64) (Linux: 5.11.0-rc7-CI-CI_DRM_9755+ x86_64)
Starting subtest: create-massive
Subtest create-massive: SUCCESS (0.001s)
Err	
Starting subtest: create-massive
Subtest create-massive: SUCCESS (0.001s)
Dmesg

Scroll to first warning
<6> [245.057395] Console: switching to colour dummy device 80x25
<6> [245.057684] [IGT] gem_create: executing
<6> [245.062015] [IGT] gem_create: starting subtest create-massive
<4> [245.062063] ------------[ cut here ]------------
<4> [245.062065] WARN_ON((size) >> 12 > ((int)(~0U >> 1)))
<4> [245.062089] WARNING: CPU: 1 PID: 1414 at drivers/gpu/drm/i915/gem/i915_gem_object.h:36 i915_gem_object_create_region+0x132/0x1b0 [i915]
<4> [245.062196] Modules linked in: vgem snd_hda_codec_hdmi i915 mei_hdcp x86_pkg_temp_thermal coretemp crct10dif_pclmul crc32_pclmul snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hwdep snd_hda_core ghash_clmulni_intel cdc_ether usbnet snd_pcm mii e1000e ptp mei_me pps_core mei intel_lpss_pci prime_numbers
<4> [245.062233] CPU: 1 PID: 1414 Comm: gem_create Tainted: G     U            5.11.0-rc7-CI-CI_DRM_9755+ #1
<4> [245.062236] Hardware name: Intel Corporation Tiger Lake Client Platform/TigerLake U DDR4 SODIMM RVP, BIOS TGLSFWI1.R00.3197.A00.2005110542 05/11/2020
<4> [245.062238] RIP: 0010:i915_gem_object_create_region+0x132/0x1b0 [i915]
<4> [245.062313] Code: 65 ff 0d 21 1c d5 5f 0f 85 79 ff ff ff e8 05 c7 d3 e0 e9 6f ff ff ff 48 c7 c6 70 6d 4e a0 48 c7 c7 0f fc 51 a0 e8 d7 4d 78 e1 <0f> 0b 49 c7 c4 f9 ff ff ff e9 65 ff ff ff 65 ff 05 e9 1b d5 5f 48
<4> [245.062315] RSP: 0018:ffffc9000230fd68 EFLAGS: 00010286
<4> [245.062319] RAX: 0000000000000000 RBX: ffffffff00000000 RCX: 0000000000000001
<4> [245.062320] RDX: 0000000080000001 RSI: ffffffff8235aaf7 RDI: 00000000ffffffff
<4> [245.062322] RBP: ffff88812922a800 R08: 0000000000000001 R09: 0000000000000001
<4> [245.062324] R10: 0000000000000001 R11: 0000000000000000 R12: ffff88811a8bb400
<4> [245.062325] R13: 0000000000000000 R14: ffffc9000230fe58 R15: ffffc9000230fe58
<4> [245.062327] FS:  00007f7fbd88c300(0000) GS:ffff8884a0280000(0000) knlGS:0000000000000000
<4> [245.062329] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
<4> [245.062331] CR2: 00007f7fbd262be0 CR3: 00000001240b2001 CR4: 0000000000770ee0
<4> [245.062332] PKRU: 55555554
<4> [245.062334] Call Trace:
<4> [245.062338]  i915_gem_create+0xc4/0x160 [i915]
<4> [245.062411]  ? i915_gem_dumb_create+0xc0/0xc0 [i915]
<4> [245.062591]  drm_ioctl_kernel+0xaa/0xf0
<4> [245.062600]  drm_ioctl+0x1e8/0x390
<4> [245.062604]  ? i915_gem_dumb_create+0xc0/0xc0 [i915]
_______________________________________________
dri-devel mailing list
dri-devel at lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel