lkml.org 
[lkml]   [2020]   [Apr]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [GIT PULL] Please pull proc and exec work for 5.7-rc1
    Linus Torvalds <torvalds@linux-foundation.org> writes:

    > On Thu, Apr 2, 2020 at 2:00 PM Bernd Edlinger <bernd.edlinger@hotmail.de> wrote:
    >>
    >> There are two more patches, which might be of interest for you, just to
    >> make the picture complete.
    >> It is not clear if we go that way, or if Eric has a yet better idea.
    >>
    >> [PATCH v7 15/16] exec: Fix dead-lock in de_thread with ptrace_attach
    >> https://www.spinics.net/lists/kernel/msg3459067.html
    >
    > There is no way I would ever take that patch.
    >
    > The amount of confusion in that patch is not acceptable. Randomly
    > unlocking the new lock?
    >
    > That code makes everything worse, it's completely incomprehensible,
    > the locking rules make no sense ahwt-so-ever.
    >
    > I'm seriously starting to feel like I should not have pulled this
    > code, because the future looks _worse_ than what we used to have.
    >
    > No. No no no. Eric, this is not an acceptable direction.

    That is not the direction I intend to take either.

    I was hoping I could put off replying to this thread for a bit because
    I only managed to get 4 hours of sleep last night and I am not as alert
    to technical details as I would like to be.

    Long story short:

    The exec_update_mutex is to be a strict subset of today's
    cred_guard_mutex. I tried to copy cred_guard_mutex's unlock style so
    that was obvious and that turns out was messier than I intended.

    I thought the changes to the individual locking changes were
    sufficiently unsubtle that they did not need my personal attention.
    Especially as they are just a substitution of one lock for another
    with a slightly smaller scope.

    I started working on the the series of changes that reorganizes
    the changes in exec.

    It was reported that something had gone wrong with my introduction
    of exec_update_mutex and I pulled it from linux-next.

    By the time I was ready to start putting humpty dumpty back together
    again Bernd had collected everything up and had it working. I had seen
    that he had been given the feedback about better change descriptions.

    I had looked at the code of his patches earlier and the basic changes
    were trivial.

    Since I thought I already knew what was in the patches and the worst
    problem was the missing unlock of cred_guard_mutex, and I know Bernd's
    patches had been tested I applied them. I missed that Bernd had added
    the exec_mmap_called flag into my patch. I thought he had only added
    the missing unlock.

    I spotted the weirdness in unlocking exec_update_mutex, and because it
    does fix a real world deadlock with ptrace I did not back it out from my
    tree.

    I have been much laxer on the details than I like to be my apologies.

    The plan is:
    exec_udpate_mutex will cover just the subset of cred_guard_mutex
    after the point of no return, and after we do any actions that
    might block waiting for userspace to do anything.

    So exec_update_mutex will just cover those things that exec
    is updating, so if you want an atomic snapshot of them
    plus the appropriate struct cred you can grab exec_update_mutex.

    I added a new mutex instead of just fixing cred_guard_mutex so
    that we can update or revert the individual code paths if it
    is found that something is wrong.

    The cred_guard_mutex also prevents other tasks from starting
    to ptrace the task that is exec'ing, and other tasks from
    setting no_new_privs on the task that is exec'ing.

    My plan is to carefully refactor the code so it can perform
    both the ptrace and no_new_privs checks after the point of
    no return.

    I have uncovered all kinds of surprises while working in that direction
    and I realize it is going to take a very delicate and careful touch to
    achieve my goal.

    There are silly things like normal linux exec when you are ptraced and
    exec changes the credentials the ordinary code will continue with the
    old credentials, but the an LSM enabled your process is likely to be
    killed instead.

    There is the personal mind blowing scenario where selinux will increase
    your credentials upon exec but if a magic directive is supplied in it's
    rules will avoid setting AT_SECURE, so that userspace won't protect
    itself from hostile takeover from the pre credential change environment.
    Much to my surprise "noatsecure" is a known and documented feature of
    selinux. I am not certain but I think I even spotted it in use on
    production.

    I will catch up on my sleep before I allow any more changes, and I will
    see replacing the called_exec_mmap flag with something saner.

    Eric

    \
     
     \ /
      Last update: 2020-04-03 01:05    [W:4.363 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site