Received-SPF: pass (google.com: domain of linux-kernel+bounces-22812-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <ed9a5c2f-2a67-41c1-b72e-a97eaebda1a5@schaufler-ca.com>
Date: Wed, 10 Jan 2024 13:20:39 -0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [GIT PULL] lsm/lsm-pr-20240105
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>
References: <3f5a7bc467d221543444a268dd1a1fe0@paul-moore.com>
 <CAHk-=whb9anXdo4odpP-ybYbmQs0UaPp1fmDjsWFiGxV5aOmig@mail.gmail.com>
 <CAHC9VhRxG2jYayjpC=bLuBpfZsXnfYj_GoDBeK527sZWRe0ZrQ@mail.gmail.com>
 <CAHk-=winAVoX=u+uX1Cdf0ekmFHETumRr60rvC_z6jbno0ApPg@mail.gmail.com>
 <CAHC9VhQ6qcPZuL8jE0smNSeCfEbyk+6L0--t0iF4Awh7HHo1Jg@mail.gmail.com>
Content-Language: en-US
From: Casey Schaufler <casey@schaufler-ca.com>
In-Reply-To: <CAHC9VhQ6qcPZuL8jE0smNSeCfEbyk+6L0--t0iF4Awh7HHo1Jg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.