From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A2552C4332F for ; Wed, 16 Nov 2022 00:36:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229537AbiKPAgL (ORCPT ); Tue, 15 Nov 2022 19:36:11 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59418 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229478AbiKPAgK (ORCPT ); Tue, 15 Nov 2022 19:36:10 -0500 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 56AFA2A73F; Tue, 15 Nov 2022 16:36:09 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id E53426176B; Wed, 16 Nov 2022 00:36:08 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 32545C433C1; Wed, 16 Nov 2022 00:36:06 +0000 (UTC) Authentication-Results: smtp.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=zx2c4.com header.i=@zx2c4.com header.b="WadNeQqZ" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=zx2c4.com; s=20210105; t=1668558963; 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=xxwhZ1JbZLEFT7v8XmPjo2GDV7ahuoimNewbxe4SHR4=; b=WadNeQqZfpaaG7pOGoOZegG56N+tU7vmniNRsbAnAmvOFql3UgnmZlGF3jkwAg0KYUuJnl vPO1x3/+Yd1mV8BLHiLJBtr7DWgWQX74GhaPD5R6gjrXeuQpkaV6S8JqLBhF8HcPiQAIeF giEdXmUZGn2q93G/kA+FleaT4qgoh6w= Received: by mail.zx2c4.com (ZX2C4 Mail Server) with ESMTPSA id 151f81bc (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 16 Nov 2022 00:36:03 +0000 (UTC) Date: Wed, 16 Nov 2022 01:36:00 +0100 From: "Jason A. Donenfeld" To: dri-devel@lists.freedesktop.org, linux-api@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Daniel Vetter , Peter Zijlstra , Ilia Mirkin , Maarten Lankhorst , Nicholas Kazlauskas , Christian Brauner , Michel =?utf-8?Q?D=C3=A4nzer?= , Alex Deucher , Adam Jackson , Greg Kroah-Hartman , Sean Paul , David Airlie , Rob Clark , Sultan Alsawaf , Linus Torvalds , Andrew Morton , Kees Cook Subject: Re: [PATCH] drm/atomic: do not branch based on the value of current->comm[0] Message-ID: References: <20221105222012.4226-1-Jason@zx2c4.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20221105222012.4226-1-Jason@zx2c4.com> Precedence: bulk List-ID: X-Mailing-List: linux-api@vger.kernel.org Hi David, I'm a bit surprised that this patch was ignored. I had sort of assumed the response would be, "good god, I can't believe we're doing this. Applied to drm-next!" rather than crickets, but maybe it just got buried under a lot of other patches. So I thought I'd poke about this again. The original message is reproduced in full below. Regards, Jason On Sat, Nov 05, 2022 at 11:20:12PM +0100, Jason A. Donenfeld wrote: > This reverts 26b1d3b527e7 ("drm/atomic: Take the atomic toys away from > X"), a rootkit-like kludge that has no business being inside of a > general purpose kernel. It's the type of debugging hack I'll use > momentarily but never commit, or a sort of babbies-first-process-hider > malware trick. > > The backstory is that some userspace code -- xorg-server -- has a > modesetting DDX that isn't really coded right. With nobody wanting to > maintain X11 anymore, rather than fixing the buggy code, the kernel was > adjusted to avoid having to touch X11. A bummer, but fair enough: if the > kernel doesn't want to support some userspace API any more, the right > thing to do is to arrange for a graceful fallback where userspace thinks > it's not available in a manageable way. > > However, the *way* it goes about doing that is just to check > `current->comm[0] == 'X'`, and disable it for only that case. So that > means it's *not* simply a matter of the kernel not wanting to support a > particular userspace API anymore, but rather it's the kernel not wanting > to support xorg-server, in theory, but actually, it turns out, that's > all processes that begin with 'X'. > > Playing games with current->comm like this is obviously wrong, and it's > pretty shocking that this ever got committed. > > Fortunately, since this was committed, somebody did actually disable > the userspace side by default in X11: > https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/180 and > this was three years ago. So userspace is mostly fine now for ordinary > default usage. And people who opt into this -- since it does actually > work fine for many use cases on i915 -- ostensibly know what they're > getting themselves into (my case). > > So let's just revert this `comm[0] == 'X'` business entirely, but still > allow for `value == 2`, in case anybody actually started working on that > part elsewhere. > > Fixes: 26b1d3b527e7 ("drm/atomic: Take the atomic toys away from X") > Cc: Daniel Vetter > Cc: Peter Zijlstra > Cc: Ilia Mirkin > Cc: Maarten Lankhorst > Cc: Nicholas Kazlauskas > Cc: Christian Brauner > Cc: Michel Dänzer > Cc: Alex Deucher > Cc: Adam Jackson > Cc: Greg Kroah-Hartman > Cc: Sean Paul > Cc: David Airlie > Cc: Rob Clark > Cc: Sultan Alsawaf > Signed-off-by: Jason A. Donenfeld > --- > drivers/gpu/drm/drm_ioctl.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > index ca2a6e6101dc..017f31e67179 100644 > --- a/drivers/gpu/drm/drm_ioctl.c > +++ b/drivers/gpu/drm/drm_ioctl.c > @@ -336,11 +336,6 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv) > case DRM_CLIENT_CAP_ATOMIC: > if (!drm_core_check_feature(dev, DRIVER_ATOMIC)) > return -EOPNOTSUPP; > - /* The modesetting DDX has a totally broken idea of atomic. */ > - if (current->comm[0] == 'X' && req->value == 1) { > - pr_info("broken atomic modeset userspace detected, disabling atomic\n"); > - return -EOPNOTSUPP; > - } > if (req->value > 2) > return -EINVAL; > file_priv->atomic = req->value; > -- > 2.38.1 >