Messages in this thread | | | Subject | Re: ac0e958a00: Kernel_panic-not_syncing:stack-protector:Kernel_stack_is_corrupted_in:run_init_process | From | Rob Landley <> | Date | Thu, 12 Nov 2020 10:02:38 -0600 |
| |
On 11/12/20 7:49 AM, David Laight wrote: > From: Rob Landley >> Sent: 12 November 2020 12:46 >> >> On 11/12/20 1:11 AM, kernel test robot wrote: >>> >>> Greeting, >>> >>> FYI, we noticed the following commit (built with gcc-9): >> >> Blah, switched from strlcpy to sprintf due to the lack of spaces and didn't >> adjust the size. >> >> (And yes, the compiler's lifetime analysis should free the stack space before >> the tail call, and I'd assume exec restarts the stack anyway.)
This is why I didn't put anything like that in the first submission. (I knew better, and did it anyway...)
>> Second-attempt-by: Rob Landley <rob@landley.net> >> --- >> >> init/main.c | 15 ++++++++------- >> 1 file changed, 8 insertions(+), 7 deletions(-) >> >> diff --git a/init/main.c b/init/main.c >> index 130376ec10ba..e92320816ef8 100644 >> --- a/init/main.c >> +++ b/init/main.c >> @@ -1328,15 +1328,16 @@ static void __init do_pre_smp_initcalls(void) >> static int run_init_process(const char *init_filename) >> { >> const char *const *p; >> + char buf[512], *s = buf; >> >> argv_init[0] = init_filename; >> - pr_info("Run %s as init process\n", init_filename); >> - pr_debug(" with arguments:\n"); >> - for (p = argv_init; *p; p++) >> - pr_debug(" %s\n", *p); >> - pr_debug(" with environment:\n"); >> - for (p = envp_init; *p; p++) >> - pr_debug(" %s\n", *p); >> + >> + for (p = (void *)envp_init; *p; p++) >> + s += sprintf(s, "%.*s ", (int)(sizeof(buf)-(s-buf)-2), *p); >> + for (p = (void *)argv_init; *p; p++) >> + s += sprintf(s, "%.*s ", (int)(sizeof(buf)-(s-buf)-2), *p); >> + pr_info("Run init: %s\n", buf); >> + > > Why not use scnprintf() as: > len += scnprintf(buf + len, 256 - len, " %s", *p);
Because what I did worked for me?
The buffer size isn't 256, sizeof() means if the buffer size changes the code automatically adjusts and the -2 gets constant folded at compile time anyway, you've proposed switching from a posix function to a kernel-specific function for no obvious benefit, it's the same number of arguments and other than 2 bytes in a string constant you've just swapped s-buf for buf+len, I didn't want to dig into whether passing rdinit= could have an empty environment so the "skip a space" has no space to skip and thus you skip the null terminator so I just left one harmlessly trailing on the end, if I wanted to get FANCY I'd measure and allocate space then free it after printing...
I could go on, but that's about as much bikeshedding as I have the stomach for right now on two for loops calling print statements, thanks.
> or even: > s = buf + sizeof buf; > len = sizeof buf; > ... > len -= scnprintf(s - len, len, " %s", *p); > > and remove the " " before the %s in the final pr_info().
Feel free to submit your own patch...?
I don't really expect this to get merged. It's not like I cc'd any humans. My latest thingied-by tag is not an approved entry in Documentation/process/submitting-patches.rst and I didn't go through all 27 steps in Documentation/process/submit-checklist.rst because I'm not part of the fulltime kernel political clique and I don't bother to fight them anymore. It's just a small thing that annoyed me and I mostly posted it here so when some clown sues us for shipping a modified kernel I can cost them more money by pointing their lawyers at the patch on the list's web archive. (I could do so on a website I maintain, but then I'd have to track it and dowanna.)
I was only ever involved here as a hobbyist. The Linux Foundation is currently holding a "conference dedicated to driving collaboration and innovation in financial services" with "featured speakers" including the Managing Director of Goldman Sachs, the Founder of the Alliance for Innovative Regulation, the former CIO of Deutsche Bank, Red Hat's Director of Financial Services Strategy, and whatever "Open Source Wonk, Azure Office of the CTO, Microsoft" means.
No, I did not make that up, they spammed me about it as part of their perpetual fundraising strategy to sell for-profit conference tickets:
https://events.linuxfoundation.org/open-source-strategy-forum/
Rob
| |