lkml.org 
[lkml]   [2022]   [Nov]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] drm/atomic: do not branch based on the value of current->comm[0]
    On Wed, Nov 16, 2022 at 01:49:43PM +1000, Dave Airlie wrote:
    > On Sun, 6 Nov 2022 at 08:21, Jason A. Donenfeld <Jason@zx2c4.com> 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.
    >
    > I've been ignoring this because I don't really want to reintroduce a
    > regression for deployed X servers. I don't see the value.
    >
    > You use a lot of what I'd call overly not backed up language. Why is
    > it obviously wrong though? Is it "playing games" or is it accessing
    > the comm to see if the process starts with X.
    >
    > Do we have lots of userspace processes starting with X that access
    > this specific piece of the drm modesetting API. I suppose we might and
    > if we have complaints about that I'd say let's try to fix it better.
    >
    > Sometimes engineering is hard, It was a big enough problem that a big
    > enough hammer was used.
    >
    > I'd hope @Daniel Vetter can comment as well since the original patch was his.

    Frankly I refrained from replying when I've seen the patch originally
    because I didn't manage to come up with a nice&constructive reply like you
    did here. The only thing novel here is the amount of backhanded insults
    folded into the commit message.

    I very much welcome constructive contributions that actually solve the
    problem here, or at least move it forward a bit. This patch is neither.

    What might be an option is a tainting module option that disables this
    check, since the amount of people willing&able to fix up Xorg is still
    zero. But that would need to come with a proper commit message and all
    that, and ideally a pile of acks from people who insist they really want
    this and need it.
    -Daniel
    --
    Daniel Vetter
    Software Engineer, Intel Corporation
    http://blog.ffwll.ch

    \
     
     \ /
      Last update: 2022-11-16 10:40    [W:4.548 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site