linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Paul Moore <paul@paul-moore.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [GIT PULL] lsm/lsm-pr-20240105
Date: Wed, 10 Jan 2024 15:58:35 -0500	[thread overview]
Message-ID: <CAHC9VhQ6qcPZuL8jE0smNSeCfEbyk+6L0--t0iF4Awh7HHo1Jg@mail.gmail.com> (raw)
In-Reply-To: <CAHk-=winAVoX=u+uX1Cdf0ekmFHETumRr60rvC_z6jbno0ApPg@mail.gmail.com>

On Wed, Jan 10, 2024 at 3:22 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Wed, 10 Jan 2024 at 11:54, Paul Moore <paul@paul-moore.com> wrote:
> >
> > Thanks for pulling the changes, I'm sorry the syscall table entries
> > for the LSM syscalls were not how you want to see them, but I'm more
> > than a little confused as to what exactly we did wrong here.
>
> Look at commit 5f42375904b0 ("LSM: wireup Linux Security Module
> syscalls") and notice for example this:
>
>   --- a/arch/x86/entry/syscalls/syscall_64.tbl
>   +++ b/arch/x86/entry/syscalls/syscall_64.tbl
>   @@ -378,6 +378,9 @@
>    454    common  futex_wake              sys_futex_wake
>    455    common  futex_wait              sys_futex_wait
>    456    common  futex_requeue           sys_futex_requeue
>   +457    common  lsm_get_self_attr       sys_lsm_get_self_attr
>   +458    common  lsm_set_self_attr       sys_lsm_set_self_attr
>   +459    common  lsm_list_modules        sys_lsm_list_modules
>
> Ok, fine - you added your new system calls to the end of the table.
> Sure, I ended up having to fix them up because the "end of the table"
> was different by the time I merged your tree, but that wasn't the
> problem.
>
> The problem is here - in the same commit:
>
>   --- a/tools/perf/arch/x86/entry/syscalls/syscall_64.tbl
>   +++ b/tools/perf/arch/x86/entry/syscalls/syscall_64.tbl
>   @@ -375,6 +375,9 @@
>    451    common  cachestat               sys_cachestat
>    452    common  fchmodat2               sys_fchmodat2
>    453    64      map_shadow_stack        sys_map_shadow_stack
>   +454    common  lsm_get_self_attr       sys_lsm_get_self_attr
>   +455    common  lsm_set_self_attr       sys_lsm_set_self_attr
>   +456    common  lsm_list_modules        sys_lsm_list_modules
>
> note how you updated the tools copy WITH THE WRONG NUMBERS!
>
> You just added them at the end of the table again, and just
> incremented the numbers, but that was complete nonsense, because the
> numbers didn't actually match the real system call numbers, because
> that tools table hadn't been updated for new system calls - because it
> hadn't needed them.
>
> Yeah, our tooling header duplication is annoying, but the old
> situation where the tooling just used various kernel headers directly
> and would randomly break when kernel changes were made was even worse.
>
> End result: avoid touching the tooling headers - and if you have to,
> you need to *think* about it.

Thanks for the explanation, when I read your comment about "tools" I
was thinking of whatever tooling transforms the arch/**/*.tbl file and
not the tools/perf directory.  I should have caught the tools/perf
mismatch when reviewing the patches from Casey, but I didn't, I'm
sorry.  My guess is that my mind was just in the "use the next three
numbers" due to the lack of syscall number sync across architectures,
but who knows.  My mistake, I'll make sure it doesn't happen again.

-- 
paul-moore.com

  reply	other threads:[~2024-01-10 20:58 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-05 23:21 [GIT PULL] lsm/lsm-pr-20240105 Paul Moore
2024-01-09 21:07 ` Linus Torvalds
2024-01-10 19:54   ` Paul Moore
2024-01-10 20:22     ` Linus Torvalds
2024-01-10 20:58       ` Paul Moore [this message]
2024-01-10 21:20         ` Casey Schaufler
2024-01-09 21:40 ` pr-tracker-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHC9VhQ6qcPZuL8jE0smNSeCfEbyk+6L0--t0iF4Awh7HHo1Jg@mail.gmail.com \
    --to=paul@paul-moore.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).