From: Casey Schaufler <casey@schaufler-ca.com>
To: Paul Moore <paul@paul-moore.com>,
Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-security-module@vger.kernel.org,
linux-kernel@vger.kernel.org,
Casey Schaufler <casey@schaufler-ca.com>
Subject: Re: [GIT PULL] lsm/lsm-pr-20240105
Date: Wed, 10 Jan 2024 13:20:39 -0800 [thread overview]
Message-ID: <ed9a5c2f-2a67-41c1-b72e-a97eaebda1a5@schaufler-ca.com> (raw)
In-Reply-To: <CAHC9VhQ6qcPZuL8jE0smNSeCfEbyk+6L0--t0iF4Awh7HHo1Jg@mail.gmail.com>
On 1/10/2024 12:58 PM, Paul Moore wrote:
> 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.
No, It's my mistake. I could have asked for help when I found my head
spinning from the syscall numbering scheme and its various implications.
I will have a look at how it might be improved. Sorry to have mucked it
up, and thank you for the explanation.
next prev parent reply other threads:[~2024-01-10 21:20 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
2024-01-10 21:20 ` Casey Schaufler [this message]
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=ed9a5c2f-2a67-41c1-b72e-a97eaebda1a5@schaufler-ca.com \
--to=casey@schaufler-ca.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=paul@paul-moore.com \
--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).