* [PATCH] kexec: Support purgatories with .text.hot sections
@ 2023-03-21 11:49 Ricardo Ribalda
2023-03-22 14:34 ` Steven Rostedt
0 siblings, 1 reply; 7+ messages in thread
From: Ricardo Ribalda @ 2023-03-21 11:49 UTC (permalink / raw)
To: Eric Biederman; +Cc: Philipp Rudo, Ricardo Ribalda, linux-kernel, kexec
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>
---
kexec: Support purgatories with .text.hot sections
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
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) {
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>
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] kexec: Support purgatories with .text.hot sections
2023-03-21 11:49 [PATCH] kexec: Support purgatories with .text.hot sections Ricardo Ribalda
@ 2023-03-22 14:34 ` Steven Rostedt
2023-03-22 14:42 ` Ricardo Ribalda
2023-03-22 20:13 ` Ross Zwisler
0 siblings, 2 replies; 7+ messages in thread
From: Steven Rostedt @ 2023-03-22 14:34 UTC (permalink / raw)
To: Ricardo Ribalda; +Cc: Eric Biederman, Philipp Rudo, linux-kernel, kexec
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) {
?
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>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] kexec: Support purgatories with .text.hot sections
2023-03-22 14:34 ` Steven Rostedt
@ 2023-03-22 14:42 ` Ricardo Ribalda
2023-03-22 14:52 ` Baoquan He
2023-03-22 20:13 ` Ross Zwisler
1 sibling, 1 reply; 7+ messages in thread
From: Ricardo Ribalda @ 2023-03-22 14:42 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Eric Biederman, Philipp Rudo, linux-kernel, kexec
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] kexec: Support purgatories with .text.hot sections
2023-03-22 14:42 ` Ricardo Ribalda
@ 2023-03-22 14:52 ` Baoquan He
2023-03-22 15:00 ` Steven Rostedt
0 siblings, 1 reply; 7+ messages in thread
From: Baoquan He @ 2023-03-22 14:52 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: Steven Rostedt, Eric Biederman, Philipp Rudo, linux-kernel, kexec
On 03/22/23 at 03:42pm, Ricardo Ribalda wrote:
> 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
When you resne patch, please fix Philipp's mail adress as
'Philipp Rudo <prudo@redhat.com>' if he should know this. He has joined
Redhat.
Thanks
Baoquan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] kexec: Support purgatories with .text.hot sections
2023-03-22 14:52 ` Baoquan He
@ 2023-03-22 15:00 ` Steven Rostedt
2023-03-23 0:17 ` Baoquan He
0 siblings, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2023-03-22 15:00 UTC (permalink / raw)
To: Baoquan He
Cc: Ricardo Ribalda, Eric Biederman, Philipp Rudo, linux-kernel,
kexec, Philipp Rudo
On Wed, 22 Mar 2023 22:52:04 +0800
Baoquan He <bhe@redhat.com> wrote:
> When you resne patch, please fix Philipp's mail adress as
> 'Philipp Rudo <prudo@redhat.com>' if he should know this. He has joined
> Redhat.
But I thought redhat *was* IBM? ;-)
-- Steve
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] kexec: Support purgatories with .text.hot sections
2023-03-22 14:34 ` Steven Rostedt
2023-03-22 14:42 ` Ricardo Ribalda
@ 2023-03-22 20:13 ` Ross Zwisler
1 sibling, 0 replies; 7+ messages in thread
From: Ross Zwisler @ 2023-03-22 20:13 UTC (permalink / raw)
To: Steven Rostedt
Cc: Ricardo Ribalda, Eric Biederman, Philipp Rudo, linux-kernel, kexec
On Wed, Mar 22, 2023 at 10:34:46AM -0400, Steven Rostedt 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) {
I think we should also be comparing against the initial value (set ~20 lines
above) of pi->ehdr->e_entry, not pi->ehdr->e_shnum.
This patch works correctly for me:
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index f7a4fd4d243f4..967826a42cdd7 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -913,7 +913,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_entry) {
kbuf->image->start -= sechdrs[i].sh_addr;
kbuf->image->start += kbuf->mem + offset;
}
Great find. With those 2 quick changes, you can add:
Reviewed-by: Ross Zwisler <zwisler@google.com>
> Shouldn't this be: kbuf->image->start == pi->ehdr->e_shnum) {
>
> ?
>
> 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>
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] kexec: Support purgatories with .text.hot sections
2023-03-22 15:00 ` Steven Rostedt
@ 2023-03-23 0:17 ` Baoquan He
0 siblings, 0 replies; 7+ messages in thread
From: Baoquan He @ 2023-03-23 0:17 UTC (permalink / raw)
To: Steven Rostedt
Cc: Ricardo Ribalda, Eric Biederman, Philipp Rudo, linux-kernel,
kexec, Philipp Rudo
On 03/22/23 at 11:00am, Steven Rostedt wrote:
> On Wed, 22 Mar 2023 22:52:04 +0800
> Baoquan He <bhe@redhat.com> wrote:
>
> > When you resne patch, please fix Philipp's mail adress as
> > 'Philipp Rudo <prudo@redhat.com>' if he should know this. He has joined
> > Redhat.
>
> But I thought redhat *was* IBM? ;-)
That's interesting questioin, maybe yes on capital operation, but no
from company operation and management, in an engineer's eyes.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-03-23 0:18 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-21 11:49 [PATCH] kexec: Support purgatories with .text.hot sections Ricardo Ribalda
2023-03-22 14:34 ` Steven Rostedt
2023-03-22 14:42 ` Ricardo Ribalda
2023-03-22 14:52 ` Baoquan He
2023-03-22 15:00 ` Steven Rostedt
2023-03-23 0:17 ` Baoquan He
2023-03-22 20:13 ` Ross Zwisler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).