From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout-p-103.mailbox.org (mout-p-103.mailbox.org [80.241.56.161]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E15CA183CC3 for ; Tue, 3 Mar 2026 18:53:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=80.241.56.161 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772564029; cv=none; b=CnwOAiDyVEb8yeEEZtzpqb3Gn+lhKrNh3GUdRpH/QrubVcGrMTXQNxWrPcbweqgZZD6jfnfgtcg661svacf7yae4sm8Xrbw0gH1wyv7UmKJw7rCllYAGzRibhOHXjIzUQNA3Aq7jmYzuA+6xKyalTquDs8FveshRHeCpMSXbE8A= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772564029; c=relaxed/simple; bh=Rth2xwMsBFi1U8Ab8zRnBBtj8JGE2EmxuyQQt5TuXZQ=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=jX0o8S0HfoAKktzJElax8WQy4Ll5vvswa3EptObRwLQChXVQzrg3lGvu/EkOWLYJOGdeYaXo1x+0o9FpFTeXJ5Lis/EVNsi6VbTJSgw86IqDZBeud4gEiQ22SzdNYE1USl13II66znp2Mp1/4gRqXQl7QxyHxvl1rKp+07PglyQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=mailbox.org; spf=pass smtp.mailfrom=mailbox.org; dkim=pass (2048-bit key) header.d=mailbox.org header.i=@mailbox.org header.b=kDjBQnct; arc=none smtp.client-ip=80.241.56.161 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=mailbox.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=mailbox.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=mailbox.org header.i=@mailbox.org header.b="kDjBQnct" Received: from smtp2.mailbox.org (smtp2.mailbox.org [IPv6:2001:67c:2050:b231:465::2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-103.mailbox.org (Postfix) with ESMTPS id 4fQQ1g6Ykyz9ssd; Tue, 3 Mar 2026 19:53:43 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mailbox.org; s=mail20150812; t=1772564024; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=jg5nfly4d8nYPWpoWnRACsuMW4dTM5oPdA6hG9Z9y44=; b=kDjBQnct0k/r80CZYdDX40k2269WIhSgTOpv0I7PGWU8KW9sW1R6n7m5OMfiFt7vsgupP2 7XGOxSQ0iouvCk/p8H+1xFscfe31bWLJcyIwqm3NFTjQZPyAvHs6xP8RJlKi8tJKnIX+G0 LD9p/qyVCCmFu1gCJrTwgGpPaf09Rykkd8Qymgm2au/a85FPEAKz4kKam9yTyqOZvAClL1 5xMc9NvN+SUmBbnHNzaPMTmJ0ZgR3Umz83yyEQSSA1u2xPpWtT43QpNEO9rOxo88nyfWKR HIQ7XVDZs+4RdPNPMyvlABccQQnGEItOMKekm+A218JQgFy4658dd7bWdeiOOw== Message-ID: <5464a6fa-5a49-42f5-a1ea-ce9f18a6955e@mailbox.org> Date: Tue, 3 Mar 2026 19:53:39 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Subject: Re: [PATCH] drm/syncobj: Fix handle <-> fd ioctls with dirty stack To: Maarten Lankhorst , Julian Orth Cc: =?UTF-8?Q?Christian_K=C3=B6nig?= , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Dmitry Osipenko , Rob Clark , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org References: <20260301-point-v1-1-21fc5fd98614@gmail.com> <3c969254-ed38-4b13-84b3-5afa365b04cb@amd.com> <2b75199f-b78a-4915-8e75-5d186f63f7c5@mailbox.org> From: =?UTF-8?Q?Michel_D=C3=A4nzer?= Content-Language: en-CA In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-MBO-RS-META: 39tb5ez316yko7oj9hebttk77qtt71o8 X-MBO-RS-ID: 2ec16498483368e0d37 On 3/3/26 18:44, Maarten Lankhorst wrote: > Den 2026-03-03 kl. 18:30, skrev Julian Orth: >> On Tue, Mar 3, 2026 at 6:18 PM Michel Dänzer wrote: >>> >>> I wrote in my first post in this thread that I don't object to your patch, so you can relax and stop trying to convince me not to object to it. :) >>> >>> I'm just pointing out that this is working around broken user-space code, and that there are other similar cases where that kind of broken users-space code couldn't be worked around in the kernel, so it's better to also fix the user-space code anyway. >> >> At this point I think we're arguing about "how can ioctls be extended" >> and "does userspace have to use memset" in general, not just about >> this particular ioctl. You've made the argument that ioctls are not >> extensible in general unless userspace uses memset. However, I'm not >> yet convinced of this. As you've also said above, drm_ioctl happily >> truncates or zero-extends ioctl arguments without returning an error >> due to size mismatch. Therefore, the only way for userspace to detect >> if the kernel supports the "extended" ioctl is to add a flag so that >> the kernel can return an error if it doesn't know the flag. And then >> that flag could also be used by the kernel to detect which fields of >> the argument are potentially uninitialized. >> >> That is why I asked above if you knew of any other examples where an >> ioctl was extended and where memset(0) became effectively required due >> to the extension. Since it's always been effectively required for ioctl structs, "become" doesn't apply. In terms of documentation, the "(How to avoid) Botching up ioctls" page says under Basics: * Check all unused fields and flags and all the padding for whether it’s 0, and reject the ioctl if that’s not the case. Which is what the code you're modifying here did: The code after the args->point checks doesn't use the point field, so it's checking that user space initialized the field to 0 per above. This contradicts the arguments in the commit log, so I'm leaning toward objecting to this patch now. > You don't even need to use memset, this would work too: > > struct drm_syncobj_handle args = { > .flags = 0 > }; TL;DR: This method isn't 100% safe either. It won't initialize any padding which isn't covered by any struct field. We try to avoid that and have explicit padding fields instead, mistakes may happen though, and in theory such padding could later be used for a new field. FWIW, libdrm uses memset for all ioctl structs. -- Earthling Michel Dänzer \ GNOME / Xwayland / Mesa developer https://redhat.com \ Libre software enthusiast