Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Message-ID: <5438c132-e127-4456-9551-42c76fb521dd@amd.com>
Date:   Thu, 2 Nov 2023 11:48:25 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 2/3] drm/scheduler: Fix UAF in
 drm_sched_fence_get_timeline_name
Content-Language: en-US
To:     Daniel Vetter <daniel@ffwll.ch>, Dave Airlie <airlied@gmail.com>
Cc:     Asahi Lina <lina@asahilina.net>, alyssa@rosenzweig.io,
        Sumit Semwal <sumit.semwal@linaro.org>,
        Faith Ekstrand <faith.ekstrand@collabora.com>,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        linux-media@vger.kernel.org, asahi@lists.linux.dev,
        Luben Tuikov <ltuikov89@gmail.com>
References: <9c0cff84-45b1-268f-bdad-38c16316dbc3@amd.com>
 <20230714-drm-sched-fixes-v1-0-c567249709f7@asahilina.net>
 <20230714-drm-sched-fixes-v1-2-c567249709f7@asahilina.net>
 <bef7ef62-3cd9-6ceb-5eb4-5ae0c0236778@amd.com>
 <de502b41-2864-db1e-16a0-8a5d5e0e4ad3@asahilina.net>
 <d9dc2fd5-d054-dbf3-72b7-fe9deaa46350@amd.com>
 <236422117088ca854a6717114de73d99b2b9ba2f@rosenzweig.io>
 <a42bd218-6eb5-6ddb-bbb4-d25118c59f40@amd.com>
 <7b564e55-a9b7-0585-3cf1-d1f132f9a918@asahilina.net>
 <daf48d76-ceee-c82d-a63a-e8e7770a9d83@amd.com>
 <f5de10fa-57d6-a3d0-1cf9-084491aa6025@asahilina.net>
 <200e9d74-7191-b1ed-e5f3-775827550853@amd.com>
 <CAPM=9txcC9+ZePA5onJxtQr+nBe8UcA3_Kp5Da3zjKL7ZB4JPQ@mail.gmail.com>
 <CAKMK7uG0G02ierkgAmJE1gfLto08LK5twGUEX1qN+qk9-AavYA@mail.gmail.com>
From:   =?UTF-8?Q?Christian_K=C3=B6nig?= <christian.koenig@amd.com>
In-Reply-To: <CAKMK7uG0G02ierkgAmJE1gfLto08LK5twGUEX1qN+qk9-AavYA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UjFsci96QVlOa3YwYXVUNjQ1ZUFuWURqWVJuVmN3NXZBbS9UVTUrSU1ZY2Qv?=
 =?utf-8?B?NHA3R1FjblNFWkdaRThUNWtlVmR2NXRiRGRCY1NoTjNMSU5nUzE5RVI3bVhB?=
 =?utf-8?B?UUlUNjRONTErQTMyYjhQSG1Uc1phR0tBUlZrV0J4Z3hSQ2htY1V0Yy9NMGFF?=
 =?utf-8?B?WHFKSkZibEZzSC9wbWhmekhvS1VNUFhjN3FYQ0Q5bzR0OG5SRi9tWllsa2tT?=
 =?utf-8?B?SDltN1cvaUl4UjFMWFdNY3kxc1BoWDJaVVRMc2NnME1CYmNSUnZLODVpd2Fy?=
 =?utf-8?B?eS9DdXhiRXJMRm5UTnREZE96b1RZV0xRV2ZGNHpBN1lXTy9vZHFsdk9VaWtt?=
 =?utf-8?B?d0drQzVFSGljY0FUU3Bpdmxhc242Z1BwUFNxSENSNTltUmtqdWpCejY5THRW?=
 =?utf-8?B?bHFjeVR3SDNaNlVDaTh1ek5vUEFZY0JTeitGSmwyVmlyQmxSM3M4alNNdURU?=
 =?utf-8?B?SWEraTMwVHpESDFXZUxzeXZuRnY5QjNaMEFMdVlDeXBLTjB4bzdNbGJ0amZD?=
 =?utf-8?B?K2cydldlTEhkSjByL2xPNVBvTWU5NHVFRHUzRmpJT3NBb0FaYXYrZ1hRaEly?=
 =?utf-8?B?TzJ6UktPTUdicDg3b1JxWnVVU0FEWENTWHRYWHR6K1NtSVU3clBSZVhaR1Vl?=
 =?utf-8?B?YUx4Q3JsdDNnWTVTdGVuaDZXaVdZQnRwUmM0clgrbmpQUWlxaXZJQ2ZMVjJF?=
 =?utf-8?B?Tjhua0VNR3pudEJsWjdwaEpUNUJuTlMzUmZtSEgyYmpnNkRjK0NKQURZSEZI?=
 =?utf-8?B?SFh6cmJHd1E1cllac2pER3lZMjJCeWsyMDl5bVlMcHllblVhNTExNUgzSHlG?=
 =?utf-8?B?RHFsM3UvS25JMjZoTGlNWW1sMUluZkxUZmhXb3VPT0tvZ3NSWGs4RXN0NE41?=
 =?utf-8?B?SlBWeDlQeVZnV1VkaHA3M1ExWG50ZzUwUXE5dU9UdlZXdzB0TGVlTWFLVUNR?=
 =?utf-8?B?NkN3Q1M2RXovdGNwaXNaVURuY2VVclVnL3FJUkd3T3czcVY4SHp6VWhQVk04?=
 =?utf-8?B?NngyOWZMZXh0U3ZZeXdmSk1hSDJTSzRsRnZyVzBNalpobVFpUzBqYmFkS2I2?=
 =?utf-8?B?dnZaUnEyWFdDc2k2ZmxieHlkYVBQalFXN0pZNTRTaFU3R1VSL3BQR3RMelAv?=
 =?utf-8?B?Q2swdVpSeXVoYnZueGp5K0Y0bWVLSVJqbDJFMVFkU3RIbndEQ3I5U3hoYjNH?=
 =?utf-8?B?aUN2OThMY0pqUit6cmhKLzZrWUt6TzhrUmRYUmh5ZHg2N1FEQXg0amhCUzBo?=
 =?utf-8?B?NXlsRWNjNnBwTytydmQ4WDZjbGR6czRReFRvenJ3ZUJjazIwaFRlVWhhWCtP?=
 =?utf-8?B?R1plSWptK0xTaVhBckR1MUpGT0xkTDdIc3VmSzg4K1BZY2V2c2tqZjhUVGpD?=
 =?utf-8?B?UThkWnBsV2ErSGhBcWdFcXcrSkljRVRXNnVlSlowejEzZlJOY0huc2JwbDlL?=
 =?utf-8?B?WXdRa01CSTcwYkFtNkZQeHpPME9QbFdiNVBySjd6V0Vld3VYS1lUbkp1eGN5?=
 =?utf-8?B?b0lPK0kzenR1MHJ2YTJKVStFQ1cyVU4veUMzVVJoZHN2ZHBPRUp4cFo5MWEy?=
 =?utf-8?B?OXV4b2JMbExhdEQrTW9LTXFKTW1nMzkwSmNFTEVxRVpmRytPamFscFNNL1Jx?=
 =?utf-8?B?ZjNJK05wWWtraDExNzUwMlE3K0ZtdC9aeGd6dHBURkd3bkNISm0ySklLSERT?=
 =?utf-8?B?d2NMc3AwaWs0U0JoL2lyQVY1bXNXeE5SZzRTdDI2Q1RvU1BaL3hWb05wdURD?=
 =?utf-8?B?alNjTzZmcXBzUm15bHEwSFFuZ0pvb2psRGpjMmpXOXlXLzlHbXpJSkgzQ09j?=
 =?utf-8?B?dHMwU1pMeFlucFQ4b2VwbWlQOXFaaTFnTFpZWjRUZWdTbUttUHNpaXpXU3NO?=
 =?utf-8?B?bEYzdzRjd1U1RXZEZmtsQklCVCtJbHI0cG5Vd2NVM2gzNktyS1BvY1RqQVVE?=
 =?utf-8?B?Nkh5OUthSHgwUFZzeXkrd2haYm9yanNUMlZmejV4b3NZMUJ0K0NFRklhdGlz?=
 =?utf-8?B?ZlJwWWhHTHNXS3hBSzVYRWpKT3hXeHVWMUtzMmp3Q1pCT2pUc1EwZ2U0WDZ1?=
 =?utf-8?B?cGlPQ0FJNnU2S3MycDlXTXBuWGRhZldjTjZWUEtNR1lvb2l1d25OZ1lmRnk5?=
 =?utf-8?Q?KClQ=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2e5ed2f6-d874-44d2-55ec-08dbdb914057
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Nov 2023 10:48:30.1472
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 9EQzujH40G5tLMO51+Sj4PNh4MaqFyf0BHNaz/5RcNz/x5+vcWDOVsuAA6BHQEtr
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR12MB4225
X-Spam-Status: No, score=-1.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=no autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Thu, 02 Nov 2023 03:48:42 -0700 (PDT)

Am 01.11.23 um 09:13 schrieb Daniel Vetter:
> On Wed, 1 Nov 2023 at 07:59, Dave Airlie <airlied@gmail.com> wrote:
>>> Well, to make it clear once more: Signaling a dma_fence from the
>>> destructor of a reference counted object is very problematic! This will
>>> be rejected no matter if you do that in C or in Rust.
>>>
>>> What we can do is to make it safe in the sense that you don't access
>>> freed up memory by using the scheduler fences even more as wrapper
>>> around the hardware fence as we do now. But this quite a change and
>>> requires a bit more than just hacking around
>>> drm_sched_fence_get_timeline_name().
>> I really think this needs to be documented if nothing else out of this thread.
>>
>> Clearly nobody is going to get it right and hidden here in this
>> thread, this info isn't useful.
>>
>> Can we have some sort of design document for the dma-fence/scheduler
>> interactions written and we can try and refine it with solutions on
>> the list, because I'm tired of people proposing things and NAK's
>> getting thrown around without anything to point people at.
>>
>> The next NAK I see on the list will mean I block all patches from the
>> sender until they write a documentation patch, because seriously this
>> stuff is too hard for someone to just keep it in their head and expect
>> everyone else to understand from reading the code.
> I very much like the idea that NAK replies are counted as "you've just
> volunteered yourself for some documentation patches so that next time
> around you can reply with a link to the docs instead of just a NAK".

Yeah, that sounds like a great idea to me as well :)

Especially when I can use it to convince managers that we need to have 
more work force on writing documentation.

> I don't think we'll get out of these discussions otherwise, since
> currently we have undocumented, but very tricky semantics of the
> drm/sched codebase for ringbuffer scheduling which is extended to fw
> scheduling in also very tricky ways, with not entirely clear impacts
> on semantics of all the drm/sched things. And as a result we just pile
> up enormous amounts of threads where I think the only thing assured is
> that people talk past each another.

The scheduler is certainly the ugliest part, but it's unfortunately 
still only the tip of the iceberg.

I have seen at least halve a dozen approach in the last two years where 
people tried to signal a dma_fence from userspace or similar.

Fortunately it was mostly prototyping and I could jump in early enough 
to stop that, but basically this is a fight against windmills.

I was considering to change the dma_fence semantics so that 
dma_fence_signal() could only be called from the interrupt contexts of 
devices and then put a big fat WARN_ON(!in_interrupt()) in there.

It's a sledgehammer, but as far as I can see the only thing which might 
help. Opinions?

Thanks,
Christian.

>
> Converting NAKs into doc patches should at least eventually get rid of
> the worst confusions we're dealing with here.
>
> Cheers, Sima