public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Some bug fixes and cleanups related to kexec
@ 2023-12-15  8:09 Yuntao Wang
  2023-12-15  8:09 ` [PATCH 1/3] kexec_file: fix incorrect end value passed to kimage_is_destination_range() Yuntao Wang
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Yuntao Wang @ 2023-12-15  8:09 UTC (permalink / raw)
  To: linux-kernel, kexec, x86
  Cc: Andrew Morton, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, H. Peter Anvin, Jonathan Corbet, Eric Biederman,
	Yuntao Wang

This series includes some bug fixes and cleanups for kexec.

Yuntao Wang (3):
  kexec_file: fix incorrect end value passed to
    kimage_is_destination_range()
  kexec_file: fix incorrect temp_start value in
    locate_mem_hole_top_down()
  x86/kexec: use pr_err() instead of pr_debug() when an error occurs

 arch/x86/kernel/kexec-bzimage64.c |  2 +-
 kernel/kexec_file.c               | 11 +++++------
 mm/highmem.c                      |  2 --
 3 files changed, 6 insertions(+), 9 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3] kexec_file: fix incorrect end value passed to kimage_is_destination_range()
  2023-12-15  8:09 [PATCH 0/3] Some bug fixes and cleanups related to kexec Yuntao Wang
@ 2023-12-15  8:09 ` Yuntao Wang
  2023-12-15 17:46   ` Eric W. Biederman
  2023-12-15  8:09 ` [PATCH 2/3] kexec_file: fix incorrect temp_start value in locate_mem_hole_top_down() Yuntao Wang
  2023-12-15  8:09 ` [PATCH 3/3] x86/kexec: use pr_err() instead of pr_debug() when an error occurs Yuntao Wang
  2 siblings, 1 reply; 11+ messages in thread
From: Yuntao Wang @ 2023-12-15  8:09 UTC (permalink / raw)
  To: linux-kernel, kexec, x86
  Cc: Andrew Morton, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, H. Peter Anvin, Jonathan Corbet, Eric Biederman,
	Yuntao Wang

The end parameter received by kimage_is_destination_range() should be the
last valid byte address of the target memory segment plus 1. However, in
the locate_mem_hole_bottom_up() and locate_mem_hole_top_down() functions,
the corresponding value passed to kimage_is_destination_range() is the last
valid byte address of the target memory segment, which is 1 less. Fix it.

Signed-off-by: Yuntao Wang <ytcoode@gmail.com>
---
 kernel/kexec_file.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index f9a419cd22d4..26be070d3bdd 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -435,13 +435,12 @@ static int locate_mem_hole_top_down(unsigned long start, unsigned long end,
 		if (temp_start < start || temp_start < kbuf->buf_min)
 			return 0;
 
-		temp_end = temp_start + kbuf->memsz - 1;
-
 		/*
 		 * Make sure this does not conflict with any of existing
 		 * segments
 		 */
-		if (kimage_is_destination_range(image, temp_start, temp_end)) {
+		if (kimage_is_destination_range(image, temp_start,
+						temp_start + kbuf->memsz)) {
 			temp_start = temp_start - PAGE_SIZE;
 			continue;
 		}
@@ -475,7 +474,7 @@ static int locate_mem_hole_bottom_up(unsigned long start, unsigned long end,
 		 * Make sure this does not conflict with any of existing
 		 * segments
 		 */
-		if (kimage_is_destination_range(image, temp_start, temp_end)) {
+		if (kimage_is_destination_range(image, temp_start, temp_end + 1)) {
 			temp_start = temp_start + PAGE_SIZE;
 			continue;
 		}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] kexec_file: fix incorrect temp_start value in locate_mem_hole_top_down()
  2023-12-15  8:09 [PATCH 0/3] Some bug fixes and cleanups related to kexec Yuntao Wang
  2023-12-15  8:09 ` [PATCH 1/3] kexec_file: fix incorrect end value passed to kimage_is_destination_range() Yuntao Wang
@ 2023-12-15  8:09 ` Yuntao Wang
  2023-12-15  8:09 ` [PATCH 3/3] x86/kexec: use pr_err() instead of pr_debug() when an error occurs Yuntao Wang
  2 siblings, 0 replies; 11+ messages in thread
From: Yuntao Wang @ 2023-12-15  8:09 UTC (permalink / raw)
  To: linux-kernel, kexec, x86
  Cc: Andrew Morton, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, H. Peter Anvin, Jonathan Corbet, Eric Biederman,
	Yuntao Wang

temp_end represents the address of the last available byte. Therefore, the
starting address of the memory segment with temp_end as its last available
byte and a size of `kbuf->memsz`, that is, the value of temp_start, should
be `temp_end - kbuf->memsz + 1` instead of `temp_end - kbuf->memsz`.

Additionally, use the ALIGN_DOWN macro instead of open-coding it directly
in locate_mem_hole_top_down() to improve code readability.

Signed-off-by: Yuntao Wang <ytcoode@gmail.com>
---
 kernel/kexec_file.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 26be070d3bdd..c3bf52a9a8ad 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -426,11 +426,11 @@ static int locate_mem_hole_top_down(unsigned long start, unsigned long end,
 	unsigned long temp_start, temp_end;
 
 	temp_end = min(end, kbuf->buf_max);
-	temp_start = temp_end - kbuf->memsz;
+	temp_start = temp_end - kbuf->memsz + 1;
 
 	do {
 		/* align down start */
-		temp_start = temp_start & (~(kbuf->buf_align - 1));
+		temp_start = ALIGN_DOWN(temp_start, kbuf->buf_align);
 
 		if (temp_start < start || temp_start < kbuf->buf_min)
 			return 0;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] x86/kexec: use pr_err() instead of pr_debug() when an error occurs
  2023-12-15  8:09 [PATCH 0/3] Some bug fixes and cleanups related to kexec Yuntao Wang
  2023-12-15  8:09 ` [PATCH 1/3] kexec_file: fix incorrect end value passed to kimage_is_destination_range() Yuntao Wang
  2023-12-15  8:09 ` [PATCH 2/3] kexec_file: fix incorrect temp_start value in locate_mem_hole_top_down() Yuntao Wang
@ 2023-12-15  8:09 ` Yuntao Wang
  2 siblings, 0 replies; 11+ messages in thread
From: Yuntao Wang @ 2023-12-15  8:09 UTC (permalink / raw)
  To: linux-kernel, kexec, x86
  Cc: Andrew Morton, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, H. Peter Anvin, Jonathan Corbet, Eric Biederman,
	Yuntao Wang

When an error is detected, use pr_err() instead of pr_debug() to output
log message.

In addition, remove the unnecessary return from set_page_address().

Signed-off-by: Yuntao Wang <ytcoode@gmail.com>
---
 arch/x86/kernel/kexec-bzimage64.c | 2 +-
 mm/highmem.c                      | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index a61c12c01270..472a45dbc79a 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -424,7 +424,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
 	 * command line. Make sure it does not overflow
 	 */
 	if (cmdline_len + MAX_ELFCOREHDR_STR_LEN > header->cmdline_size) {
-		pr_debug("Appending elfcorehdr=<addr> to command line exceeds maximum allowed length\n");
+		pr_err("Appending elfcorehdr=<addr> to command line exceeds maximum allowed length\n");
 		return ERR_PTR(-EINVAL);
 	}
 
diff --git a/mm/highmem.c b/mm/highmem.c
index e19269093a93..bd48ba445dd4 100644
--- a/mm/highmem.c
+++ b/mm/highmem.c
@@ -799,8 +799,6 @@ void set_page_address(struct page *page, void *virtual)
 		}
 		spin_unlock_irqrestore(&pas->lock, flags);
 	}
-
-	return;
 }
 
 void __init page_address_init(void)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] kexec_file: fix incorrect end value passed to kimage_is_destination_range()
  2023-12-15  8:09 ` [PATCH 1/3] kexec_file: fix incorrect end value passed to kimage_is_destination_range() Yuntao Wang
@ 2023-12-15 17:46   ` Eric W. Biederman
  2023-12-16  2:21     ` Baoquan He
  0 siblings, 1 reply; 11+ messages in thread
From: Eric W. Biederman @ 2023-12-15 17:46 UTC (permalink / raw)
  To: Yuntao Wang
  Cc: linux-kernel, kexec, x86, Andrew Morton, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin,
	Jonathan Corbet

Yuntao Wang <ytcoode@gmail.com> writes:

> The end parameter received by kimage_is_destination_range() should be the
> last valid byte address of the target memory segment plus 1. However, in
> the locate_mem_hole_bottom_up() and locate_mem_hole_top_down() functions,
> the corresponding value passed to kimage_is_destination_range() is the last
> valid byte address of the target memory segment, which is 1 less. Fix
> it.

If that is true we I think we should rather fix
kimage_is_destination_range.

Otherwise we run the risk of having areas whose end is not
representable, epecially on 32bit.


Eric


> Signed-off-by: Yuntao Wang <ytcoode@gmail.com>
> ---
>  kernel/kexec_file.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index f9a419cd22d4..26be070d3bdd 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -435,13 +435,12 @@ static int locate_mem_hole_top_down(unsigned long start, unsigned long end,
>  		if (temp_start < start || temp_start < kbuf->buf_min)
>  			return 0;
>  
> -		temp_end = temp_start + kbuf->memsz - 1;
> -
>  		/*
>  		 * Make sure this does not conflict with any of existing
>  		 * segments
>  		 */
> -		if (kimage_is_destination_range(image, temp_start, temp_end)) {
> +		if (kimage_is_destination_range(image, temp_start,
> +						temp_start + kbuf->memsz)) {
>  			temp_start = temp_start - PAGE_SIZE;
>  			continue;
>  		}
> @@ -475,7 +474,7 @@ static int locate_mem_hole_bottom_up(unsigned long start, unsigned long end,
>  		 * Make sure this does not conflict with any of existing
>  		 * segments
>  		 */
> -		if (kimage_is_destination_range(image, temp_start, temp_end)) {
> +		if (kimage_is_destination_range(image, temp_start, temp_end + 1)) {
>  			temp_start = temp_start + PAGE_SIZE;
>  			continue;
>  		}

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] kexec_file: fix incorrect end value passed to kimage_is_destination_range()
  2023-12-15 17:46   ` Eric W. Biederman
@ 2023-12-16  2:21     ` Baoquan He
  2023-12-16  4:18       ` [PATCH 1/3 v2] kexec: modify the meaning of the end parameter in kimage_is_destination_range() Yuntao Wang
  0 siblings, 1 reply; 11+ messages in thread
From: Baoquan He @ 2023-12-16  2:21 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Yuntao Wang, linux-kernel, kexec, x86, Andrew Morton,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H. Peter Anvin, Jonathan Corbet

On 12/15/23 at 11:46am, Eric W. Biederman wrote:
> Yuntao Wang <ytcoode@gmail.com> writes:
> 
> > The end parameter received by kimage_is_destination_range() should be the
> > last valid byte address of the target memory segment plus 1. However, in
> > the locate_mem_hole_bottom_up() and locate_mem_hole_top_down() functions,
> > the corresponding value passed to kimage_is_destination_range() is the last
> > valid byte address of the target memory segment, which is 1 less. Fix
> > it.
> 
> If that is true we I think we should rather fix
> kimage_is_destination_range.

It's true wit the current implementation of
kimage_is_destination_range(). Its callers pass the start/end+1
pair. Agree we should fix kimage_is_destination_range() instead and
adjust callers, such as kimage_alloc_normal_control_pages(), and
kimage_alloc_page().


> 
> Otherwise we run the risk of having areas whose end is not
> representable, epecially on 32bit.
> 
> 
> Eric
> 
> 
> > Signed-off-by: Yuntao Wang <ytcoode@gmail.com>
> > ---
> >  kernel/kexec_file.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> > index f9a419cd22d4..26be070d3bdd 100644
> > --- a/kernel/kexec_file.c
> > +++ b/kernel/kexec_file.c
> > @@ -435,13 +435,12 @@ static int locate_mem_hole_top_down(unsigned long start, unsigned long end,
> >  		if (temp_start < start || temp_start < kbuf->buf_min)
> >  			return 0;
> >  
> > -		temp_end = temp_start + kbuf->memsz - 1;
> > -
> >  		/*
> >  		 * Make sure this does not conflict with any of existing
> >  		 * segments
> >  		 */
> > -		if (kimage_is_destination_range(image, temp_start, temp_end)) {
> > +		if (kimage_is_destination_range(image, temp_start,
> > +						temp_start + kbuf->memsz)) {
> >  			temp_start = temp_start - PAGE_SIZE;
> >  			continue;
> >  		}
> > @@ -475,7 +474,7 @@ static int locate_mem_hole_bottom_up(unsigned long start, unsigned long end,
> >  		 * Make sure this does not conflict with any of existing
> >  		 * segments
> >  		 */
> > -		if (kimage_is_destination_range(image, temp_start, temp_end)) {
> > +		if (kimage_is_destination_range(image, temp_start, temp_end + 1)) {
> >  			temp_start = temp_start + PAGE_SIZE;
> >  			continue;
> >  		}
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3 v2] kexec: modify the meaning of the end parameter in kimage_is_destination_range()
  2023-12-16  2:21     ` Baoquan He
@ 2023-12-16  4:18       ` Yuntao Wang
  2023-12-16  9:29         ` Baoquan He
  0 siblings, 1 reply; 11+ messages in thread
From: Yuntao Wang @ 2023-12-16  4:18 UTC (permalink / raw)
  To: bhe
  Cc: akpm, bp, corbet, dave.hansen, ebiederm, hpa, kexec, linux-kernel,
	mingo, tglx, x86, ytcoode

The end parameter received by kimage_is_destination_range() should be the
last valid byte address of the target memory segment plus 1. However, in
the locate_mem_hole_bottom_up() and locate_mem_hole_top_down() functions,
the corresponding value passed to kimage_is_destination_range() is the last
valid byte address of the target memory segment, which is 1 less.

There are two ways to fix this bug. We can either correct the logic of the
locate_mem_hole_bottom_up() and locate_mem_hole_top_down() functions, or we
can fix kimage_is_destination_range() by making the end parameter represent
the last valid byte address of the target memory segment. Here, we choose
the second approach.

Due to the modification to kimage_is_destination_range(), we also need to
adjust its callers, such as kimage_alloc_normal_control_pages() and
kimage_alloc_page().

Signed-off-by: Yuntao Wang <ytcoode@gmail.com>
---
v1->v2:
  Fix this issue using the approach suggested by Eric and Baoquan.

  As this patch is independent of the other patches in this series, I sent
  out the v2 patch separately. If it's inconvenient for anyone, I can
  resend the entire series again.

 kernel/kexec_core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index be5642a4ec49..5991b3ae072c 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -276,8 +276,8 @@ int kimage_is_destination_range(struct kimage *image,
 		unsigned long mstart, mend;
 
 		mstart = image->segment[i].mem;
-		mend = mstart + image->segment[i].memsz;
-		if ((end > mstart) && (start < mend))
+		mend = mstart + image->segment[i].memsz - 1;
+		if ((end >= mstart) && (start <= mend))
 			return 1;
 	}
 
@@ -372,7 +372,7 @@ static struct page *kimage_alloc_normal_control_pages(struct kimage *image,
 		addr  = pfn << PAGE_SHIFT;
 		eaddr = epfn << PAGE_SHIFT;
 		if ((epfn >= (KEXEC_CONTROL_MEMORY_LIMIT >> PAGE_SHIFT)) ||
-			      kimage_is_destination_range(image, addr, eaddr)) {
+			      kimage_is_destination_range(image, addr, eaddr - 1)) {
 			list_add(&pages->lru, &extra_pages);
 			pages = NULL;
 		}
@@ -716,7 +716,7 @@ static struct page *kimage_alloc_page(struct kimage *image,
 
 		/* If the page is not a destination page use it */
 		if (!kimage_is_destination_range(image, addr,
-						  addr + PAGE_SIZE))
+						  addr + PAGE_SIZE - 1))
 			break;
 
 		/*
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3 v2] kexec: modify the meaning of the end parameter in kimage_is_destination_range()
  2023-12-16  4:18       ` [PATCH 1/3 v2] kexec: modify the meaning of the end parameter in kimage_is_destination_range() Yuntao Wang
@ 2023-12-16  9:29         ` Baoquan He
  2023-12-16 11:23           ` [PATCH 1/3 v3] " Yuntao Wang
  2023-12-16 12:05           ` [PATCH 1/3 v4] " Yuntao Wang
  0 siblings, 2 replies; 11+ messages in thread
From: Baoquan He @ 2023-12-16  9:29 UTC (permalink / raw)
  To: Yuntao Wang
  Cc: akpm, bp, corbet, dave.hansen, ebiederm, hpa, kexec, linux-kernel,
	mingo, tglx, x86

On 12/16/23 at 12:18pm, Yuntao Wang wrote:
> The end parameter received by kimage_is_destination_range() should be the
> last valid byte address of the target memory segment plus 1. However, in
> the locate_mem_hole_bottom_up() and locate_mem_hole_top_down() functions,
> the corresponding value passed to kimage_is_destination_range() is the last
> valid byte address of the target memory segment, which is 1 less.
> 
> There are two ways to fix this bug. We can either correct the logic of the
> locate_mem_hole_bottom_up() and locate_mem_hole_top_down() functions, or we
> can fix kimage_is_destination_range() by making the end parameter represent
> the last valid byte address of the target memory segment. Here, we choose
> the second approach.
> 
> Due to the modification to kimage_is_destination_range(), we also need to
> adjust its callers, such as kimage_alloc_normal_control_pages() and
> kimage_alloc_page().
> 
> Signed-off-by: Yuntao Wang <ytcoode@gmail.com>
> ---
> v1->v2:
>   Fix this issue using the approach suggested by Eric and Baoquan.
> 
>   As this patch is independent of the other patches in this series, I sent
>   out the v2 patch separately. If it's inconvenient for anyone, I can
>   resend the entire series again.
> 
>  kernel/kexec_core.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index be5642a4ec49..5991b3ae072c 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -276,8 +276,8 @@ int kimage_is_destination_range(struct kimage *image,
>  		unsigned long mstart, mend;
>  
>  		mstart = image->segment[i].mem;
> -		mend = mstart + image->segment[i].memsz;
> -		if ((end > mstart) && (start < mend))
> +		mend = mstart + image->segment[i].memsz - 1;
> +		if ((end >= mstart) && (start <= mend))
>  			return 1;
>  	}
>  
> @@ -372,7 +372,7 @@ static struct page *kimage_alloc_normal_control_pages(struct kimage *image,
>  		addr  = pfn << PAGE_SHIFT;
>  		eaddr = epfn << PAGE_SHIFT;

  		eaddr = epfn << PAGE_SHIFT - 1;

The overall looks good to me, one tiny concern is I would like to
have 'eaddr' changed as above if a specific local variable has been
defined.

>  		if ((epfn >= (KEXEC_CONTROL_MEMORY_LIMIT >> PAGE_SHIFT)) ||
> -			      kimage_is_destination_range(image, addr, eaddr)) {
> +			      kimage_is_destination_range(image, addr, eaddr - 1)) {

Then we have:

 +			      kimage_is_destination_range(image, addr, eaddr)) {


>  			list_add(&pages->lru, &extra_pages);
>  			pages = NULL;
>  		}
> @@ -716,7 +716,7 @@ static struct page *kimage_alloc_page(struct kimage *image,
>  
>  		/* If the page is not a destination page use it */
>  		if (!kimage_is_destination_range(image, addr,
> -						  addr + PAGE_SIZE))
> +						  addr + PAGE_SIZE - 1))
>  			break;
>  
>  		/*
> -- 
> 2.43.0
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3 v3] kexec: modify the meaning of the end parameter in kimage_is_destination_range()
  2023-12-16  9:29         ` Baoquan He
@ 2023-12-16 11:23           ` Yuntao Wang
  2023-12-16 12:05           ` [PATCH 1/3 v4] " Yuntao Wang
  1 sibling, 0 replies; 11+ messages in thread
From: Yuntao Wang @ 2023-12-16 11:23 UTC (permalink / raw)
  To: bhe
  Cc: akpm, bp, corbet, dave.hansen, ebiederm, hpa, kexec, linux-kernel,
	mingo, tglx, x86, ytcoode

The end parameter received by kimage_is_destination_range() should be the
last valid byte address of the target memory segment plus 1. However, in
the locate_mem_hole_bottom_up() and locate_mem_hole_top_down() functions,
the corresponding value passed to kimage_is_destination_range() is the last
valid byte address of the target memory segment, which is 1 less.

There are two ways to fix this bug. We can either correct the logic of the
locate_mem_hole_bottom_up() and locate_mem_hole_top_down() functions, or we
can fix kimage_is_destination_range() by making the end parameter represent
the last valid byte address of the target memory segment. Here, we choose
the second approach.

Due to the modification to kimage_is_destination_range(), we also need to
adjust its callers, such as kimage_alloc_normal_control_pages() and
kimage_alloc_page().

Signed-off-by: Yuntao Wang <ytcoode@gmail.com>
---
v1->v2:
  Fix this issue using the approach suggested by Eric and Baoquan.

  As this patch is independent of the other patches in this series, I sent
  out the v2 patch separately. If it's inconvenient for anyone, I can
  resend the entire series again.

v2->v3:
  Modify the assignment of eaddr as suggested by Baoquan.

 kernel/kexec_core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index be5642a4ec49..e65e8f186eff 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -276,8 +276,8 @@ int kimage_is_destination_range(struct kimage *image,
 		unsigned long mstart, mend;
 
 		mstart = image->segment[i].mem;
-		mend = mstart + image->segment[i].memsz;
-		if ((end > mstart) && (start < mend))
+		mend = mstart + image->segment[i].memsz - 1;
+		if ((end >= mstart) && (start <= mend))
 			return 1;
 	}
 
@@ -370,7 +370,7 @@ static struct page *kimage_alloc_normal_control_pages(struct kimage *image,
 		pfn   = page_to_boot_pfn(pages);
 		epfn  = pfn + count;
 		addr  = pfn << PAGE_SHIFT;
-		eaddr = epfn << PAGE_SHIFT;
+		eaddr = epfn << PAGE_SHIFT - 1;
 		if ((epfn >= (KEXEC_CONTROL_MEMORY_LIMIT >> PAGE_SHIFT)) ||
 			      kimage_is_destination_range(image, addr, eaddr)) {
 			list_add(&pages->lru, &extra_pages);
@@ -716,7 +716,7 @@ static struct page *kimage_alloc_page(struct kimage *image,
 
 		/* If the page is not a destination page use it */
 		if (!kimage_is_destination_range(image, addr,
-						  addr + PAGE_SIZE))
+						  addr + PAGE_SIZE - 1))
 			break;
 
 		/*
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 1/3 v4] kexec: modify the meaning of the end parameter in kimage_is_destination_range()
  2023-12-16  9:29         ` Baoquan He
  2023-12-16 11:23           ` [PATCH 1/3 v3] " Yuntao Wang
@ 2023-12-16 12:05           ` Yuntao Wang
  2023-12-17  1:02             ` Baoquan He
  1 sibling, 1 reply; 11+ messages in thread
From: Yuntao Wang @ 2023-12-16 12:05 UTC (permalink / raw)
  To: bhe
  Cc: akpm, bp, corbet, dave.hansen, ebiederm, hpa, kexec, linux-kernel,
	mingo, tglx, x86, ytcoode

The end parameter received by kimage_is_destination_range() should be the
last valid byte address of the target memory segment plus 1. However, in
the locate_mem_hole_bottom_up() and locate_mem_hole_top_down() functions,
the corresponding value passed to kimage_is_destination_range() is the last
valid byte address of the target memory segment, which is 1 less.

There are two ways to fix this bug. We can either correct the logic of the
locate_mem_hole_bottom_up() and locate_mem_hole_top_down() functions, or we
can fix kimage_is_destination_range() by making the end parameter represent
the last valid byte address of the target memory segment. Here, we choose
the second approach.

Due to the modification to kimage_is_destination_range(), we also need to
adjust its callers, such as kimage_alloc_normal_control_pages() and
kimage_alloc_page().

Signed-off-by: Yuntao Wang <ytcoode@gmail.com>
---
v1->v2:
  Fix this issue using the approach suggested by Eric and Baoquan.

  As this patch is independent of the other patches in this series, I sent
  out the v2 patch separately. If it's inconvenient for anyone, I can
  resend the entire series again.

v2->v3:
  Modify the assignment of eaddr as suggested by Baoquan.

v3->v4:
  `eaddr = epfn << PAGE_SHIFT - 1` causes a compilation warning, fix it.

 kernel/kexec_core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index be5642a4ec49..e3b1a699f087 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -276,8 +276,8 @@ int kimage_is_destination_range(struct kimage *image,
 		unsigned long mstart, mend;
 
 		mstart = image->segment[i].mem;
-		mend = mstart + image->segment[i].memsz;
-		if ((end > mstart) && (start < mend))
+		mend = mstart + image->segment[i].memsz - 1;
+		if ((end >= mstart) && (start <= mend))
 			return 1;
 	}
 
@@ -370,7 +370,7 @@ static struct page *kimage_alloc_normal_control_pages(struct kimage *image,
 		pfn   = page_to_boot_pfn(pages);
 		epfn  = pfn + count;
 		addr  = pfn << PAGE_SHIFT;
-		eaddr = epfn << PAGE_SHIFT;
+		eaddr = (epfn << PAGE_SHIFT) - 1;
 		if ((epfn >= (KEXEC_CONTROL_MEMORY_LIMIT >> PAGE_SHIFT)) ||
 			      kimage_is_destination_range(image, addr, eaddr)) {
 			list_add(&pages->lru, &extra_pages);
@@ -716,7 +716,7 @@ static struct page *kimage_alloc_page(struct kimage *image,
 
 		/* If the page is not a destination page use it */
 		if (!kimage_is_destination_range(image, addr,
-						  addr + PAGE_SIZE))
+						  addr + PAGE_SIZE - 1))
 			break;
 
 		/*
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3 v4] kexec: modify the meaning of the end parameter in kimage_is_destination_range()
  2023-12-16 12:05           ` [PATCH 1/3 v4] " Yuntao Wang
@ 2023-12-17  1:02             ` Baoquan He
  0 siblings, 0 replies; 11+ messages in thread
From: Baoquan He @ 2023-12-17  1:02 UTC (permalink / raw)
  To: Yuntao Wang
  Cc: akpm, bp, corbet, dave.hansen, ebiederm, hpa, kexec, linux-kernel,
	mingo, tglx, x86

On 12/16/23 at 08:05pm, Yuntao Wang wrote:
> The end parameter received by kimage_is_destination_range() should be the
> last valid byte address of the target memory segment plus 1. However, in
> the locate_mem_hole_bottom_up() and locate_mem_hole_top_down() functions,
> the corresponding value passed to kimage_is_destination_range() is the last
> valid byte address of the target memory segment, which is 1 less.
> 
> There are two ways to fix this bug. We can either correct the logic of the
> locate_mem_hole_bottom_up() and locate_mem_hole_top_down() functions, or we
> can fix kimage_is_destination_range() by making the end parameter represent
> the last valid byte address of the target memory segment. Here, we choose
> the second approach.
> 
> Due to the modification to kimage_is_destination_range(), we also need to
> adjust its callers, such as kimage_alloc_normal_control_pages() and
> kimage_alloc_page().
> 
> Signed-off-by: Yuntao Wang <ytcoode@gmail.com>
> ---
> v1->v2:
>   Fix this issue using the approach suggested by Eric and Baoquan.
> 
>   As this patch is independent of the other patches in this series, I sent
>   out the v2 patch separately. If it's inconvenient for anyone, I can
>   resend the entire series again.
> 
> v2->v3:
>   Modify the assignment of eaddr as suggested by Baoquan.
> 
> v3->v4:
>   `eaddr = epfn << PAGE_SHIFT - 1` causes a compilation warning, fix it.
> 
>  kernel/kexec_core.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

The whole series looks good to me with this v4 of patch 1, you may need
to reorganize them and repost.

Acked-by: Baoquan He <bhe@redhat.com>

> 
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index be5642a4ec49..e3b1a699f087 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -276,8 +276,8 @@ int kimage_is_destination_range(struct kimage *image,
>  		unsigned long mstart, mend;
>  
>  		mstart = image->segment[i].mem;
> -		mend = mstart + image->segment[i].memsz;
> -		if ((end > mstart) && (start < mend))
> +		mend = mstart + image->segment[i].memsz - 1;
> +		if ((end >= mstart) && (start <= mend))
>  			return 1;
>  	}
>  
> @@ -370,7 +370,7 @@ static struct page *kimage_alloc_normal_control_pages(struct kimage *image,
>  		pfn   = page_to_boot_pfn(pages);
>  		epfn  = pfn + count;
>  		addr  = pfn << PAGE_SHIFT;
> -		eaddr = epfn << PAGE_SHIFT;
> +		eaddr = (epfn << PAGE_SHIFT) - 1;
>  		if ((epfn >= (KEXEC_CONTROL_MEMORY_LIMIT >> PAGE_SHIFT)) ||
>  			      kimage_is_destination_range(image, addr, eaddr)) {
>  			list_add(&pages->lru, &extra_pages);
> @@ -716,7 +716,7 @@ static struct page *kimage_alloc_page(struct kimage *image,
>  
>  		/* If the page is not a destination page use it */
>  		if (!kimage_is_destination_range(image, addr,
> -						  addr + PAGE_SIZE))
> +						  addr + PAGE_SIZE - 1))
>  			break;
>  
>  		/*
> -- 
> 2.43.0
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-12-17  1:02 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-15  8:09 [PATCH 0/3] Some bug fixes and cleanups related to kexec Yuntao Wang
2023-12-15  8:09 ` [PATCH 1/3] kexec_file: fix incorrect end value passed to kimage_is_destination_range() Yuntao Wang
2023-12-15 17:46   ` Eric W. Biederman
2023-12-16  2:21     ` Baoquan He
2023-12-16  4:18       ` [PATCH 1/3 v2] kexec: modify the meaning of the end parameter in kimage_is_destination_range() Yuntao Wang
2023-12-16  9:29         ` Baoquan He
2023-12-16 11:23           ` [PATCH 1/3 v3] " Yuntao Wang
2023-12-16 12:05           ` [PATCH 1/3 v4] " Yuntao Wang
2023-12-17  1:02             ` Baoquan He
2023-12-15  8:09 ` [PATCH 2/3] kexec_file: fix incorrect temp_start value in locate_mem_hole_top_down() Yuntao Wang
2023-12-15  8:09 ` [PATCH 3/3] x86/kexec: use pr_err() instead of pr_debug() when an error occurs Yuntao Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox