linux-kernel - Re: [GIT PULL] KVM/rseq changes for Linux 5.15-rc3

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: <5b351abc-3bb7-abce-7a5c-2d16f6fe9b34@redhat.com>
Date:   Thu, 23 Sep 2021 20:44:17 +0200
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Oleg Nesterov <oleg@...hat.com>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Al Viro <viro@...iv.linux.org.uk>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        KVM list <kvm@...r.kernel.org>
Subject: Re: [GIT PULL] KVM/rseq changes for Linux 5.15-rc3

On 23/09/21 20:35, Linus Torvalds wrote:
> But I'm adding a few people to the cc for a completely different
> reason: the cleanup to move all the notify_resume stuff to
> tracehook_notify_resume() is good, but it does make me go - once again
> - "Hmm, that naming is really really bad".

Absolutely, it's even in the commit message:

---
Note, tracehook_notify_resume() is horribly named and arguably does not
belong in tracehook.h as literally every line of code in it has nothing
to do with tracing.  But, that's been true since commit a42c6ded827d
("move key_repace_session_keyring() into tracehook_notify_resume()")
first usurped tracehook_notify_resume() back in 2012.  Punt cleaning that
mess up to future patches.
---

As you point out, it's really all of the header and not just that one
function.

Paolo

> The <linux/tracehook.h> code was literally meant for tracing. It's
> where the name comes from, and it's the original intent: having a
> place that you can hook into for tracing that doesn't depend on how
> the core kernel code ends up changing.
> 
> But that's not how it actually acts right now. That header file is now
> some very core functionality, and little of it is actually related to
> tracing any more. It's more core process state handling for the user
> space return path.
> 
> So I don't object to the patches, and they are merged, but I'm cc'ing people to
> 
>   (a) let them know about this (see commit a68de80f61f6: "entry: rseq:
> Call rseq_handle_notify_resume() in tracehook_notify_resume()" in the
> current -git tree)
> 
>   (b) possibly prod some people into perhaps moving/renaming some of
> that code to actual core kernel C files, instead of a misnamed header
> file..
> 
> Hmm?
> 
>           Linus
>