[PATCH v2] s390/sclp: Add check for get_zeroed_page()

[PATCH v2] s390/sclp: Add check for get_zeroed_page()

[Haoxiang Li]

Add check for the return value of get_zeroed_page() in
sclp_console_init() to prevent null pointer dereference.
Furthermore, to solve the memory leak caused by the loop
allocation, add a free helper to do the free job.

Fixes: 4c8f4794b61e ("[S390] sclp console: convert from bootmem to slab")
Cc: stable@vger.kernel.org
Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
---
Changes in v2:
- Add a free helper to solve the memory leak caused by loop allocation.
- Thanks Heiko! I realized that v1 patch overlooked a potential memory leak.
After consideration, I choose to do the full exercise. I noticed a similar
handling in [1], following that handling I submit this v2 patch. Thanks again!

Reference link:
[1]https://github.com/torvalds/linux/blob/master/drivers/s390/char/sclp_vt220.c#L699
---
 drivers/s390/char/sclp_con.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/s390/char/sclp_con.c b/drivers/s390/char/sclp_con.c
index e5d947c763ea..c87b0c204718 100644
--- a/drivers/s390/char/sclp_con.c
+++ b/drivers/s390/char/sclp_con.c
@@ -263,6 +263,19 @@ static struct console sclp_console =
 	.index = 0 /* ttyS0 */
 };
 
+/*
+ *  Release allocated pages.
+ */
+static void __init __sclp_console_free_pages(void)
+{
+	struct list_head *page, *p;
+
+	list_for_each_safe(page, p, &sclp_con_pages) {
+		list_del(page);
+		free_page((unsigned long) page);
+	}
+}
+
 /*
  * called by console_init() in drivers/char/tty_io.c at boot-time.
  */
@@ -282,6 +295,10 @@ sclp_console_init(void)
 	/* Allocate pages for output buffering */
 	for (i = 0; i < sclp_console_pages; i++) {
 		page = (void *) get_zeroed_page(GFP_KERNEL | GFP_DMA);
+		if (!page) {
+			__sclp_console_free_pages();
+			return -ENOMEM;
+		}
 		list_add_tail(page, &sclp_con_pages);
 	}
 	sclp_conbuf = NULL;
-- 
2.25.1

Re: [PATCH v2] s390/sclp: Add check for get_zeroed_page()

[Heiko Carstens]

On Tue, Feb 18, 2025 at 10:52:16AM +0800, Haoxiang Li wrote:
> Add check for the return value of get_zeroed_page() in
> sclp_console_init() to prevent null pointer dereference.
> Furthermore, to solve the memory leak caused by the loop
> allocation, add a free helper to do the free job.
> 
> Fixes: 4c8f4794b61e ("[S390] sclp console: convert from bootmem to slab")
> Cc: stable@vger.kernel.org
> Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
> ---
> Changes in v2:
> - Add a free helper to solve the memory leak caused by loop allocation.
> - Thanks Heiko! I realized that v1 patch overlooked a potential memory leak.
> After consideration, I choose to do the full exercise. I noticed a similar
> handling in [1], following that handling I submit this v2 patch. Thanks again!
> 
> Reference link:
> [1]https://github.com/torvalds/linux/blob/master/drivers/s390/char/sclp_vt220.c#L699
> ---
>  drivers/s390/char/sclp_con.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)

Ok, but this should come without Fixes and Cc stable, since in real life this
code will never be executed. It is just to make the code look saner, and to
avoid that more people look into this in the future.

Acked-by: Heiko Carstens <hca@linux.ibm.com>

Re: [PATCH v2] s390/sclp: Add check for get_zeroed_page()

[Vasily Gorbik]

On Fri, Feb 21, 2025 at 04:11:57PM +0100, Heiko Carstens wrote:
> On Tue, Feb 18, 2025 at 10:52:16AM +0800, Haoxiang Li wrote:
> > Add check for the return value of get_zeroed_page() in
> > sclp_console_init() to prevent null pointer dereference.
> > Furthermore, to solve the memory leak caused by the loop
> > allocation, add a free helper to do the free job.
> > 
> > Fixes: 4c8f4794b61e ("[S390] sclp console: convert from bootmem to slab")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
> > ---
> > Changes in v2:
> > - Add a free helper to solve the memory leak caused by loop allocation.
> > - Thanks Heiko! I realized that v1 patch overlooked a potential memory leak.
> > After consideration, I choose to do the full exercise. I noticed a similar
> > handling in [1], following that handling I submit this v2 patch. Thanks again!
> > 
> > Reference link:
> > [1]https://github.com/torvalds/linux/blob/master/drivers/s390/char/sclp_vt220.c#L699
> > ---
> >  drivers/s390/char/sclp_con.c | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> 
> Ok, but this should come without Fixes and Cc stable, since in real life this
> code will never be executed. It is just to make the code look saner, and to
> avoid that more people look into this in the future.
> 
> Acked-by: Heiko Carstens <hca@linux.ibm.com>

Applied, thank you!