From: Ricardo Ribalda <ribalda@chromium.org> To: Steven Rostedt <rostedt@goodmis.org> Cc: Eric Biederman <ebiederm@xmission.com>, Philipp Rudo <prudo@linux.vnet.ibm.com>, linux-kernel@vger.kernel.org, kexec@lists.infradead.org Subject: Re: [PATCH] kexec: Support purgatories with .text.hot sections Date: Wed, 22 Mar 2023 15:42:14 +0100 [thread overview] Message-ID: <CANiDSCt_wvaHBq-Yss0QaKTtefBhWwtahFO8_jw6CPSvBwbbMg@mail.gmail.com> (raw) In-Reply-To: <ZBsSBr87al9ccG96@home.goodmis.org> Hi Steven On Wed, 22 Mar 2023 at 15:34, Steven Rostedt <rostedt@goodmis.org> wrote: > > On Tue, Mar 21, 2023 at 12:49:08PM +0100, Ricardo Ribalda wrote: > > Clang16 links the purgatory text in two sections: > > > > [ 1] .text PROGBITS 0000000000000000 00000040 > > 00000000000011a1 0000000000000000 AX 0 0 16 > > [ 2] .rela.text RELA 0000000000000000 00003498 > > 0000000000000648 0000000000000018 I 24 1 8 > > ... > > [17] .text.hot. PROGBITS 0000000000000000 00003220 > > 000000000000020b 0000000000000000 AX 0 0 1 > > [18] .rela.text.hot. RELA 0000000000000000 00004428 > > 0000000000000078 0000000000000018 I 24 17 8 > > > > And both of them have their range [sh_addr ... sh_addr+sh_size] on the > > area pointed by `e_entry`. > > > > This causes that image->start is calculated twice, once for .text and > > another time for .text.hot. The second calculation leaves image->start > > in a random location. > > > > Because of this, the system crashes inmediatly after: > > > > kexec_core: Starting new kernel > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > To: Eric Biederman <ebiederm@xmission.com> > > Cc: Philipp Rudo <prudo@linux.vnet.ibm.com> > > Cc: kexec@lists.infradead.org > > Cc: linux-kernel@vger.kernel.org > > --- > > kernel/kexec_file.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c > > index f1a0e4e3fb5c..b1a25d97d5e2 100644 > > --- a/kernel/kexec_file.c > > +++ b/kernel/kexec_file.c > > @@ -904,7 +904,8 @@ static int kexec_purgatory_setup_sechdrs(struct purgatory_info *pi, > > if (sechdrs[i].sh_flags & SHF_EXECINSTR && > > pi->ehdr->e_entry >= sechdrs[i].sh_addr && > > pi->ehdr->e_entry < (sechdrs[i].sh_addr > > - + sechdrs[i].sh_size)) { > > + + sechdrs[i].sh_size) && > > + kbuf->image->start != pi->ehdr->e_shnum) { > > Shouldn't this be: kbuf->image->start == pi->ehdr->e_shnum) { You are absolutely right, I screwed up when I ported the code from my test tree to the tree that I use for upstreaming. Instead of removing all the printks I wrote code. :S Will resend the patch > > ? > > As you want to only do this update when it's not equal to the initial value. > If this did work, then you may want to make sure that was the initial value. > > Also, please add a comment about why you are doing this check. > > Thanks! > > -- Steve > > > kbuf->image->start -= sechdrs[i].sh_addr; > > kbuf->image->start += kbuf->mem + offset; > > } > > > > --- > > base-commit: 17214b70a159c6547df9ae204a6275d983146f6b > > change-id: 20230321-kexec_clang16-4510c23d129c > > > > Best regards, > > -- > > Ricardo Ribalda <ribalda@chromium.org> -- Ricardo Ribalda _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
WARNING: multiple messages have this Message-ID (diff)
From: Ricardo Ribalda <ribalda@chromium.org> To: Steven Rostedt <rostedt@goodmis.org> Cc: Eric Biederman <ebiederm@xmission.com>, Philipp Rudo <prudo@linux.vnet.ibm.com>, linux-kernel@vger.kernel.org, kexec@lists.infradead.org Subject: Re: [PATCH] kexec: Support purgatories with .text.hot sections Date: Wed, 22 Mar 2023 15:42:14 +0100 [thread overview] Message-ID: <CANiDSCt_wvaHBq-Yss0QaKTtefBhWwtahFO8_jw6CPSvBwbbMg@mail.gmail.com> (raw) In-Reply-To: <ZBsSBr87al9ccG96@home.goodmis.org> Hi Steven On Wed, 22 Mar 2023 at 15:34, Steven Rostedt <rostedt@goodmis.org> wrote: > > On Tue, Mar 21, 2023 at 12:49:08PM +0100, Ricardo Ribalda wrote: > > Clang16 links the purgatory text in two sections: > > > > [ 1] .text PROGBITS 0000000000000000 00000040 > > 00000000000011a1 0000000000000000 AX 0 0 16 > > [ 2] .rela.text RELA 0000000000000000 00003498 > > 0000000000000648 0000000000000018 I 24 1 8 > > ... > > [17] .text.hot. PROGBITS 0000000000000000 00003220 > > 000000000000020b 0000000000000000 AX 0 0 1 > > [18] .rela.text.hot. RELA 0000000000000000 00004428 > > 0000000000000078 0000000000000018 I 24 17 8 > > > > And both of them have their range [sh_addr ... sh_addr+sh_size] on the > > area pointed by `e_entry`. > > > > This causes that image->start is calculated twice, once for .text and > > another time for .text.hot. The second calculation leaves image->start > > in a random location. > > > > Because of this, the system crashes inmediatly after: > > > > kexec_core: Starting new kernel > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > To: Eric Biederman <ebiederm@xmission.com> > > Cc: Philipp Rudo <prudo@linux.vnet.ibm.com> > > Cc: kexec@lists.infradead.org > > Cc: linux-kernel@vger.kernel.org > > --- > > kernel/kexec_file.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c > > index f1a0e4e3fb5c..b1a25d97d5e2 100644 > > --- a/kernel/kexec_file.c > > +++ b/kernel/kexec_file.c > > @@ -904,7 +904,8 @@ static int kexec_purgatory_setup_sechdrs(struct purgatory_info *pi, > > if (sechdrs[i].sh_flags & SHF_EXECINSTR && > > pi->ehdr->e_entry >= sechdrs[i].sh_addr && > > pi->ehdr->e_entry < (sechdrs[i].sh_addr > > - + sechdrs[i].sh_size)) { > > + + sechdrs[i].sh_size) && > > + kbuf->image->start != pi->ehdr->e_shnum) { > > Shouldn't this be: kbuf->image->start == pi->ehdr->e_shnum) { You are absolutely right, I screwed up when I ported the code from my test tree to the tree that I use for upstreaming. Instead of removing all the printks I wrote code. :S Will resend the patch > > ? > > As you want to only do this update when it's not equal to the initial value. > If this did work, then you may want to make sure that was the initial value. > > Also, please add a comment about why you are doing this check. > > Thanks! > > -- Steve > > > kbuf->image->start -= sechdrs[i].sh_addr; > > kbuf->image->start += kbuf->mem + offset; > > } > > > > --- > > base-commit: 17214b70a159c6547df9ae204a6275d983146f6b > > change-id: 20230321-kexec_clang16-4510c23d129c > > > > Best regards, > > -- > > Ricardo Ribalda <ribalda@chromium.org> -- Ricardo Ribalda
next prev parent reply other threads:[~2023-03-22 14:42 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-03-21 11:49 [PATCH] kexec: Support purgatories with .text.hot sections Ricardo Ribalda 2023-03-21 11:49 ` Ricardo Ribalda 2023-03-22 14:34 ` Steven Rostedt 2023-03-22 14:34 ` Steven Rostedt 2023-03-22 14:42 ` Ricardo Ribalda [this message] 2023-03-22 14:42 ` Ricardo Ribalda 2023-03-22 14:52 ` Baoquan He 2023-03-22 14:52 ` Baoquan He 2023-03-22 15:00 ` Steven Rostedt 2023-03-22 15:00 ` Steven Rostedt 2023-03-23 0:17 ` Baoquan He 2023-03-23 0:17 ` Baoquan He 2023-03-22 20:13 ` Ross Zwisler 2023-03-22 20:13 ` Ross Zwisler
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CANiDSCt_wvaHBq-Yss0QaKTtefBhWwtahFO8_jw6CPSvBwbbMg@mail.gmail.com \ --to=ribalda@chromium.org \ --cc=ebiederm@xmission.com \ --cc=kexec@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=prudo@linux.vnet.ibm.com \ --cc=rostedt@goodmis.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.