From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com ([192.55.52.115]:15116 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751526AbbKQOlb convert rfc822-to-8bit (ORCPT ); Tue, 17 Nov 2015 09:41:31 -0500 From: Jani Nikula To: Ville =?utf-8?B?U3lyasOkbMOk?= , Daniel Vetter Cc: Daniel Vetter , intel-gfx@lists.freedesktop.org, stable@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915: Don't clobber the addfb2 ioctl params In-Reply-To: <20151117130045.GS4437@intel.com> References: <1447261890-3960-1-git-send-email-ville.syrjala@linux.intel.com> <20151117094706.GH16848@phenom.ffwll.local> <20151117110421.GQ4437@intel.com> <20151117130045.GS4437@intel.com> Date: Tue, 17 Nov 2015 16:45:31 +0200 Message-ID: <87egfoelbo.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Sender: stable-owner@vger.kernel.org List-ID: On Tue, 17 Nov 2015, Ville Syrjälä wrote: > On Tue, Nov 17, 2015 at 01:04:21PM +0200, Ville Syrjälä wrote: >> On Tue, Nov 17, 2015 at 10:47:06AM +0100, Daniel Vetter wrote: >> > On Wed, Nov 11, 2015 at 07:11:28PM +0200, ville.syrjala@linux.intel.com wrote: >> > > From: Ville Syrjälä >> > > >> > > We try to convert the old way of of specifying fb tiling (obj->tiling) >> > > into the new fb modifiers. We store the result in the passed in mode_cmd >> > > structure. But that structure comes directly from the addfb2 ioctl, and >> > > gets copied back out to userspace, which means we're clobbering the >> > > modifiers that the user provided (all 0 since the DRM_MODE_FB_MODIFIERS >> > > flag wasn't even set by the user). Hence if the user reuses the struct >> > > for another addfb2, the ioctl will be rejected since it's now asking for >> > > some modifiers w/o the flag set. >> > > >> > > Fix the problem by making a copy of the user provided structure. We can >> > > play any games we want with the copy. >> > > >> > > Cc: stable@vger.kernel.org >> > > Cc: Daniel Vetter >> > > Cc: Tvrtko Ursulin >> > > Fixes: 2a80eada326f ("drm/i915: Add fb format modifier support") >> > > Testcase: igt/kms_addfb_basic/clobbered-modifier >> > > Signed-off-by: Ville Syrjälä >> > >> > Out of curiosity: Where does this blow up? That should be added to the >> > commit message (so that people affected can match it up with this fix). >> >> I don't know that it affects any actual users. The way I caught this was >> running kms_addfb_basic. One of the subtests failed when I ran the full >> test, but if I ran only the specific subtest it was fine. So the >> modifiers got clobbered by the previous subtest. I already forgot which >> subtest it was, but it's easy enough to track it down again. > > # ./tests/kms_addfb_basic > IGT-Version: 1.12-git (x86_64) (Linux: 4.4.0-rc1-stereo+ x86_64) > ... > Subtest basic-X-tiled: SUCCESS (0.001s) > Test assertion failure function pitch_tests, file kms_addfb_basic.c:167: > Failed assertion: drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f) == 0 > Last errno: 22, Invalid argument > Stack trace: > #0 [__igt_fail_assert+0x101] > #1 [pitch_tests+0x619] > #2 [__real_main426+0x2f] > #3 [main+0x23] > #4 [__libc_start_main+0xf0] > #5 [_start+0x29] > #6 [+0x29] > Subtest framebuffer-vs-set-tiling failed. > **** DEBUG **** > Test assertion failure function pitch_tests, file kms_addfb_basic.c:167: > Failed assertion: drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f) == 0 > Last errno: 22, Invalid argument > **** END **** > Subtest framebuffer-vs-set-tiling: FAIL (0.003s) > ... > > # ./tests/kms_addfb_basic --r framebuffer-vs-set-tiling > IGT-Version: 1.12-git (x86_64) (Linux: 4.4.0-rc1-stereo+ x86_64) > Subtest framebuffer-vs-set-tiling: SUCCESS (0.000s) > >> >> > With that: >> > >> > Reviewed-by: Daniel Vetter Pushed to drm-intel-fixes, thanks for the patch and review. I had to do a trivial rebase, and resolve conflicts for nightly. Ville, please check the commit in -fixes and yell if it's not right. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center