Re: [REGRESSION][BISECTED] Cannot boot Lichee Pi 4A with FORTIFY_SOURCE enabled

[Kees Cook]

On September 24, 2024 8:58:57 AM PDT, Jason Montleon <jmontleo at redhat.com> wrote:
>On Sun, Sep 22, 2024 at 6:38 PM Kees Cook <kees at kernel.org> wrote:
>> Can you try this patch? It should avoid using the "WARN" infrastructure
>> (if that is the source of blocking boot), but should still provide some
>> detail about what tripped it up (via the "regular" pr_*() logging). And
>> if it boots, can you look at the log to find what it reports for the
>> "Wanted to write ..." line(s)?
>>
>> Thanks!
>>
>> -Kees
>>
>>
>> diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
>> index 0d99bf11d260..469a3439959d 100644
>> --- a/include/linux/fortify-string.h
>> +++ b/include/linux/fortify-string.h
>> @@ -549,7 +549,8 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
>>                                          const size_t q_size,
>>                                          const size_t p_size_field,
>>                                          const size_t q_size_field,
>> -                                        const u8 func)
>> +                                        const u8 func,
>> +                                        const char *field)
>>  {
>>         if (__builtin_constant_p(size)) {
>>                 /*
>> @@ -610,8 +611,13 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
>>          * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101832
>>          */
>>         if (p_size_field != SIZE_MAX &&
>> -           p_size != p_size_field && p_size_field < size)
>> -               return true;
>> +           p_size != p_size_field && p_size_field < size) {
>> +               if (p_size_field == 0)
>> +                       pr_warn("Wanted to write %zu to a 0-sized destination: %s\n",
>> +                               size, field);
>> +               else
>> +                       return true;
>> +       }
>>
>>         return false;
>>  }
>> @@ -625,7 +631,8 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
>>         const size_t __q_size_field = (q_size_field);                   \
>>         fortify_warn_once(fortify_memcpy_chk(__fortify_size, __p_size,  \
>>                                      __q_size, __p_size_field,          \
>> -                                    __q_size_field, FORTIFY_FUNC_ ##op), \
>> +                                    __q_size_field, FORTIFY_FUNC_ ##op,\
>> +                                    #p " at " FILE_LINE), \
>>                   #op ": detected field-spanning write (size %zu) of single %s (size %zu)\n", \
>>                   __fortify_size,                                       \
>>                   "field \"" #p "\" at " FILE_LINE,                     \
>>
>>
>> --
>> Kees Cook
>>
>
>Hi Kees,
>Sorry for the delay in responding. Your patch also caused the system
>to hang booting, but I took it as inspiration to try some things.
>TL;DR I see these two print several times:
>
>Wanted to write 8 to a 0-sized destination: oldptr at
>arch/riscv/errata/thead/errata.c:185
>Wanted to write 28 to a 0-sized destination: oldptr at
>arch/riscv/errata/thead/errata.c:185
>
>Since I suspected CONFIG_ERRATA_THEAD_MAE which relies on
>CONFIG_RISCV_ALTERNATIVE_EARLY I suspected it is just too early for
>pr_*, etc. I tried trace_printk but that didn't produce any traces,
>and then I found a comment which makes me think we're too early for
>that too.
>https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/riscv/kernel/alternative.c?h=v6.11#n214
>
>I didn't want to give up so I wrote a simple module to figure out how
>to use sbi_ecall and then incorporated that idea into your patch,
>although even that was not straightforward:
>https://gist.github.com/jmontleon/f3536ad70dc92bed992139ad5b266d5c

Oh nice work! Probably something riscv-specific needs to be wired up to the printk routines to do what you have there until printk is fully available  But that's a separate issue, I guess.

>Full output: https://gist.github.com/jmontleon/823facff38e28b717f153e38adcfd7fe

Does the system boot with your patch?

I would guess that this line at arch/riscv/errata/thead/errata.c:168:

void *oldptr, *altptr;

Should be:

u8 *oldptr, *altptr;

But maybe __ALT_PTR needs adjustment too? Hmm. I would have expected void * to have a size of SIZE_MAX here, though, so something else might be confusing the check.


-- 
Kees Cook