linux-kernel - Re: [PATCH] drm/atomic: do not branch based on the value of current->comm[0]

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y3QyGtFcIj/LGPIw@zx2c4.com>
Date:   Wed, 16 Nov 2022 01:43:06 +0100
From:   "Jason A. Donenfeld" <Jason@...c4.com>
To:     dri-devel@...ts.freedesktop.org, linux-api@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     Daniel Vetter <daniel.vetter@...el.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ilia Mirkin <imirkin@...m.mit.edu>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Nicholas Kazlauskas <nicholas.kazlauskas@....com>,
        Christian Brauner <brauner@...nel.org>,
        Michel Dänzer <michel@...nzer.net>,
        Alex Deucher <alexdeucher@...il.com>,
        Adam Jackson <ajax@...hat.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Sean Paul <sean@...rly.run>, David Airlie <airlied@...hat.com>,
        Rob Clark <robdclark@...il.com>,
        Sultan Alsawaf <sultan@...neltoast.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH] drm/atomic: do not branch based on the value of
 current->comm[0]

Hey again,

On Wed, Nov 16, 2022 at 01:36:00AM +0100, Jason A. Donenfeld wrote:
> I'm a bit surprised that this patch was ignored. I had sort of assumed

Mystery solved: this message to you bounced from this linux.ie address I
somehow wound up with in the recipients list. Fixing now by using the
one in MAINTAINERS. Sorry about that.

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 <daniel.vetter@...el.com>
> > Cc: Peter Zijlstra <peterz@...radead.org>
> > Cc: Ilia Mirkin <imirkin@...m.mit.edu>
> > Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>
> > Cc: Nicholas Kazlauskas <nicholas.kazlauskas@....com>
> > Cc: Christian Brauner <brauner@...nel.org>
> > Cc: Michel Dänzer <michel@...nzer.net>
> > Cc: Alex Deucher <alexdeucher@...il.com>
> > Cc: Adam Jackson <ajax@...hat.com>
> > Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> > Cc: Sean Paul <sean@...rly.run>
> > Cc: David Airlie <airlied@...ux.ie>
> > Cc: Rob Clark <robdclark@...il.com>
> > Cc: Sultan Alsawaf <sultan@...neltoast.com>
> > Signed-off-by: Jason A. Donenfeld <Jason@...c4.com>
> > ---
> >  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