On Tue, Sep 24, 2024 at 1:36 PM Kees Cook <[email protected]> wrote: > > > > On September 24, 2024 8:58:57 AM PDT, Jason Montleon <[email protected]> > wrote: > >On Sun, Sep 22, 2024 at 6:38 PM Kees Cook <[email protected]> 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?
Yes. It boots fully. It also booted with trace_printk, it just didn't produce a trace, which again I think is expected at that stage. It seems as long as there are no 'normal' print attempts, pr_()*, etc. it works. > I would guess that this line at arch/riscv/errata/thead/errata.c:168: > > void *oldptr, *altptr; > > Should be: > > u8 *oldptr, *altptr; I tried this, but it didn't appear to make a difference, I will try some more today, though I admit to being out of my element. > 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 >
