[LTP] [PATCH] kallsyms: skip user-space mapped addresses

[Jan Stancek]

On Tue, Jul 9, 2024 at 2:35 PM Li Wang <liwan@redhat.com> wrote:
>
>
>
> On Tue, Jul 9, 2024 at 8:18 PM Jan Stancek <jstancek@redhat.com> wrote:
>>
>> RHEL9 s390x with KPTI enabled has been observed to sporadically
>> use same address ranges for user-space libraries as the ones
>> occupied by kallsyms while in kernel-space.
>
>
> Thanks for the proposal fix, do you think we need to add
> a check to see if KPTI is explicitly enabled before doing that
> is_address_mapped?
> (or, check the configuration for CONFIG_PAGE_TABLE_ISOLATION)

It shouldn't be needed. It's a small optimization, but the test run looks
fast enough (at least on s390x I tried).

>
>
>>
>> The test currently (wrongly) assumes, that a write to kallsym
>> address should fail and doesn't take into account that same
>> memory ranges could be mapped differently in user vs. kernel-space.
>>
>> Add a check to see if a kallsym address is already mapped by user-space,
>> and if it is, skip it to avoid false positives.
>>
>>
>> Before:
>>     13  kallsyms.c:115: TFAIL: Successfully accessed kernel addr 0x3ff8060787d (r) (__kstrtabns_nfs3_set_ds_client)
>>     14  kallsyms.c:115: TFAIL: Successfully accessed kernel addr 0x3ff806007a0 (t) (nfs_init_server_aclclient)
>>     15  kallsyms.c:115: TFAIL: Successfully accessed kernel addr 0x3ff8060a058 (d) (nfsacl_version)
>>     16  kallsyms.c:115: TFAIL: Successfully accessed kernel addr 0x3ff8060a078 (d) (nfsacl_rpcstat)
>>     ...
>>     kallsyms.c:130: TFAIL: Caught 103102 SIGSEGV but expected 153738
>>
>> After:
>>     kallsyms.c:175: TDEBUG: Skipping userspace mapped address 0x3ff80220024
>>     kallsyms.c:175: TDEBUG: Skipping userspace mapped address 0x3ff8022003c
>>     kallsyms.c:175: TDEBUG: Skipping userspace mapped address 0x3ff80213b20
>>     ...
>>     kallsyms.c:184: TPASS: Caught 103473 SIGSEGV in access ksymbols addr, skipped 27234
>>
>> Signed-off-by: Jan Stancek <jstancek@redhat.com>
>> ---
>>  testcases/kernel/security/kallsyms/kallsyms.c | 68 +++++++++++++++++--
>>  1 file changed, 63 insertions(+), 5 deletions(-)
>>
>> diff --git a/testcases/kernel/security/kallsyms/kallsyms.c b/testcases/kernel/security/kallsyms/kallsyms.c
>> index 2a5cbaa325ea..b07cb9cda268 100644
>> --- a/testcases/kernel/security/kallsyms/kallsyms.c
>> +++ b/testcases/kernel/security/kallsyms/kallsyms.c
>> @@ -54,6 +54,8 @@ static struct kallsym *sym_table;
>>  static unsigned int nr_symbols;
>>  static sigjmp_buf jmpbuf;
>>  volatile sig_atomic_t segv_caught;
>> +static unsigned long *ranges;
>> +static int ranges_size, ranges_len;
>>
>>  static void segv_handler(int sig)
>>  {
>> @@ -89,6 +91,49 @@ static unsigned int read_kallsyms(struct kallsym *table, unsigned int table_size
>>         return nr_syms;
>>  }
>>
>> +static void read_proc_self_maps(void)
>> +{
>> +       FILE *fp;
>> +
>> +       ranges_len = 0;
>> +       fp = fopen("/proc/self/maps", "r");
>> +       if (fp == NULL)
>> +               tst_brk(TBROK | TERRNO, "Failed to open /proc/self/maps.");
>> +
>> +       while (!feof(fp)) {
>> +               unsigned long start, end;
>> +               int ret;
>> +
>> +               ret = fscanf(fp, "%lx-%lx %*[^\n]\n", &start, &end);
>> +               if (ret != 2) {
>> +                       fclose(fp);
>> +                       tst_brk(TBROK | TERRNO, "Couldn't parse /proc/self/maps line.");
>> +               }
>> +
>> +               if (ranges_size < ranges_len + 1) {
>> +                       ranges_size += 128;
>> +                       ranges = realloc(ranges, 2*ranges_size*sizeof(unsigned long));
>> +               }
>> +               ranges[ranges_len*2] = start;
>> +               ranges[ranges_len*2 + 1] = end;
>> +
>> +               ranges_len++;
>> +       }
>> +
>> +       fclose(fp);
>> +}
>> +
>> +static int is_address_mapped(unsigned long addr)
>> +{
>> +       int i;
>> +
>> +       for (i = 0; i < ranges_len; i++) {
>> +               if (ranges[i*2] <= addr && addr < ranges[i*2 + 1])
>> +                       return 1;
>> +       }
>> +       return 0;
>> +}
>> +
>>  static void setup(void)
>>  {
>>         struct sigaction sa;
>> @@ -117,17 +162,30 @@ static void access_ksymbols_address(struct kallsym *table)
>>         }
>>  }
>>
>> +
>>  static void test_access_kernel_address(void)
>>  {
>> -       segv_caught = 0;
>> +       int skipped = 0;
>>
>> -       for (unsigned int i = 0; i < nr_symbols; i++)
>> +       segv_caught = 0;
>> +       read_proc_self_maps();
>> +
>> +       for (unsigned int i = 0; i < nr_symbols; i++) {
>> +               if (is_address_mapped(sym_table[i].addr)) {
>> +                       tst_res(TDEBUG, "Skipping userspace mapped address 0x%lx",
>> +                               sym_table[i].addr);
>> +                       skipped++;
>> +                       continue;
>> +               }
>>                 access_ksymbols_address(&sym_table[i]);
>> +       }
>>
>> -       if (segv_caught == (sig_atomic_t)nr_symbols)
>> -               tst_res(TPASS, "Caught %d SIGSEGV in access ksymbols addr", segv_caught);
>> +       if (segv_caught == (sig_atomic_t)nr_symbols - skipped)
>> +               tst_res(TPASS, "Caught %d SIGSEGV in access ksymbols addr, skipped %d",
>> +                       segv_caught, skipped);
>>         else
>> -               tst_res(TFAIL, "Caught %d SIGSEGV but expected %d", segv_caught, nr_symbols);
>> +               tst_res(TFAIL, "Caught %d SIGSEGV but expected %d, skipped %d",
>> +                       segv_caught, nr_symbols-skipped, skipped);
>>  }
>>
>>  static void cleanup(void)
>> --
>> 2.39.3
>>