[PATCH AUTOSEL 4.19 044/167] drm/amdgpu: validate user pitch alignment

[Michel Dänzer]

On 2019-09-03 10:16 p.m., Daniel Vetter wrote:
> On Tue, Sep 3, 2019 at 10:01 PM Sasha Levin <sashal at kernel.org> wrote:
>> On Tue, Sep 03, 2019 at 07:03:47PM +0200, Greg KH wrote:
>>> On Tue, Sep 03, 2019 at 06:40:43PM +0200, Michel Dänzer wrote:
>>>> On 2019-09-03 6:23 p.m., Sasha Levin wrote:
>>>>> From: Yu Zhao <yuzhao at google.com>
>>>>>
>>>>> [ Upstream commit 89f23b6efef554766177bf51aa754bce14c3e7da ]
>>>>
>>>> Hold your horses!
>>>>
>>>> This commit and c4a32b266da7bb702e60381ca0c35eaddbc89a6c had to be
>>>> reverted, as they caused regressions. See commits
>>>> 25ec429e86bb790e40387a550f0501d0ac55a47c &
>>>> 92b0730eaf2d549fdfb10ecc8b71f34b9f472c12 .
>>>>
>>>>
>>>> This isn't bolstering confidence in how these patches are selected...
>>>
>>> The patch _itself_ said to be backported to the stable trees from 4.2
>>> and newer.  Why wouldn't we be confident in doing this?
>>>
>>> If the patch doesn't want to be backported, then do not add the cc:
>>> stable line to it...
>>
>> This patch was picked because it has a stable tag, which you presumably
>> saw as your Reviewed-by tag is in the patch. This is why it was
>> backported; it doesn't take AI to backport patches tagged for stable...

The patches did point to gaps in validation of ioctl parameters passed
in by userspace. Unfortunately, they turned out to be too strict,
causing valid parameters to spuriously fail. If that wasn't the case,
and the patches didn't have stable tags, maybe we'd be having a
discussion about why they didn't have the tags now...


>> The revert of this patch, however:
>>
>>  1. Didn't have a stable tag.

I guess it didn't occur to me that was necessary, as the patches got
reverted within days.


>>  2. Didn't have a "Fixes:" tag.
>>  3. Didn't have the usual "the reverts commit ..." string added by git
>>  when one does a revert.

I suspect that's because there were no stable commit hashes to
reference, see below.


>> Which is why we still kick patches for review, even though they had a
>> stable tag, just so people could take a look and confirm we're not
>> missing anything - like we did here.
>>
>> I'm not sure what you expected me to do differently here.

Yeah, sorry, I didn't realize it's tricky for scripts to recognize that
the patches have been reverted in this case.


> Yeah this looks like fail on the revert side, they need to reference
> the reverted commit somehow ...
> 
> Alex, why got this dropped? Is this more fallout from the back&forth
> shuffling you're doing between your internal branches behind the
> firewall, and the public history?

I do suspect that was at least a contributing factor.


-- 
Earthling Michel Dänzer               |               https://redhat.com
Libre software enthusiast             |             Mesa and X developer