linux-kernel - Re: [PATCH 4.19 32/32] drm/atomic: Take the atomic toys away from X

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]

Date:   Fri, 8 May 2020 22:30:21 +0200
From:   Pavel Machek <pavel@...x.de>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Ilia Mirkin <imirkin@...m.mit.edu>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Michel Dänzer <michel@...nzer.net>,
        Alex Deucher <alexdeucher@...il.com>,
        Adam Jackson <ajax@...hat.com>, Sean Paul <sean@...rly.run>,
        David Airlie <airlied@...ux.ie>,
        Rob Clark <robdclark@...il.com>,
        Daniel Vetter <daniel.vetter@...el.com>,
        Yves-Alexis Perez <corsac@...ian.org>,
        Nicholas Kazlauskas <nicholas.kazlauskas@....com>
Subject: Re: [PATCH 4.19 32/32] drm/atomic: Take the atomic toys away from X

Hi!

> From: Daniel Vetter <daniel.vetter@...ll.ch>
> 
> commit 26b1d3b527e7bf3e24b814d617866ac5199ce68d upstream.
> 
> The -modesetting ddx has a totally broken idea of how atomic works:
> - doesn't disable old connectors, assuming they get auto-disable like
>   with the legacy setcrtc
> - assumes ASYNC_FLIP is wired through for the atomic ioctl
> - not a single call to TEST_ONLY
> 
> Iow the implementation is a 1:1 translation of legacy ioctls to
> atomic, which is a) broken b) pointless.
> 
> We already have bugs in both i915 and amdgpu-DC where this prevents us
> from enabling neat features.
> 
> If anyone ever cares about atomic in X we can easily add a new atomic
> level (req->value == 2) for X to get back the shiny toys.
> 
> Since these broken versions of -modesetting have been shipping,
> there's really no other way to get out of this bind.

This is quite crazy. You really should not fight with X like
that. Will it break someone's setup?

> @@ -321,7 +321,12 @@ drm_setclientcap(struct drm_device *dev,
>  	case DRM_CLIENT_CAP_ATOMIC:
>  		if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
>  			return -EINVAL;
> -		if (req->value > 1)
> +		/* 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)

Really? Checking first letter of command name? Is there no other way
to do it? Should it at least check full command name, so my
XtremeWindowingSystem can continue working?

Is this justified? If this is not an regression, you should simply ask
people to update their X server, not add crazy hack in kernel for
that.

Does it even work? Will not comm[0] be "/" in many cases?

root     13628  1.6  2.5 914196 150524 tty7    Ssl+ Apr19 482:32 /usr/lib/xorg/Xorg :0 -seat seat0 -auth /var/run/lightdm/root/:0 -nolisten tcp vt7 -novtswitch

Best regards,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Download attachment "signature.asc" of type "application/pgp-signature" (196 bytes)