diff mbox series

mm/vmalloc: Add check for KMEM_CACHE

Message ID 20221124040226.17953-1-jiasheng@iscas.ac.cn
State New
Headers
Series mm/vmalloc: Add check for KMEM_CACHE |

Commit Message

Jiasheng Jiang Nov. 24, 2022, 4:02 a.m. UTC 
  As KMEM_CACHE may return NULL pointer, it should
be better to check the return value in order to
avoid NULL pointer dereference in kmem_cache_zalloc.

Fixes: 68ad4a330433 ("mm/vmalloc.c: keep track of free blocks for vmap allocation")
Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
---
 mm/vmalloc.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

Comments

Matthew Wilcox Nov. 24, 2022, 4:25 a.m. UTC | #1 
On Thu, Nov 24, 2022 at 12:02:26PM +0800, Jiasheng Jiang wrote:
> As KMEM_CACHE may return NULL pointer, it should
> be better to check the return value in order to
> avoid NULL pointer dereference in kmem_cache_zalloc.

You've made the code more complex.  And for what?  If that
call fails, the system will not boot under any circumstances.

NAK this patch, and any more like it.

> Fixes: 68ad4a330433 ("mm/vmalloc.c: keep track of free blocks for vmap allocation")
> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
> ---
>  mm/vmalloc.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index ccaa461998f3..df3e59f614cc 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -2426,15 +2426,17 @@ void __init vmalloc_init(void)
>  	}
>  
>  	/* Import existing vmlist entries. */
> -	for (tmp = vmlist; tmp; tmp = tmp->next) {
> -		va = kmem_cache_zalloc(vmap_area_cachep, GFP_NOWAIT);
> -		if (WARN_ON_ONCE(!va))
> -			continue;
> +	if (!WARN_ON_ONCE(!vmap_area_cachep)) {
> +		for (tmp = vmlist; tmp; tmp = tmp->next) {
> +			va = kmem_cache_zalloc(vmap_area_cachep, GFP_NOWAIT);
> +			if (WARN_ON_ONCE(!va))
> +				continue;
>  
> -		va->va_start = (unsigned long)tmp->addr;
> -		va->va_end = va->va_start + tmp->size;
> -		va->vm = tmp;
> -		insert_vmap_area(va, &vmap_area_root, &vmap_area_list);
> +			va->va_start = (unsigned long)tmp->addr;
> +			va->va_end = va->va_start + tmp->size;
> +			va->vm = tmp;
> +			insert_vmap_area(va, &vmap_area_root, &vmap_area_list);
> +		}
>  	}
>  
>  	/*
> -- 
> 2.25.1
> 
>
Andrew Morton Nov. 24, 2022, 4:25 a.m. UTC | #2 
On Thu, 24 Nov 2022 12:02:26 +0800 Jiasheng Jiang <jiasheng@iscas.ac.cn> wrote:

> As KMEM_CACHE may return NULL pointer, it should
> be better to check the return value in order to
> avoid NULL pointer dereference in kmem_cache_zalloc.
> 
> ...
>
> @@ -2426,15 +2426,17 @@ void __init vmalloc_init(void)

Linux assumes that allocation attempts from __init code will succeed.

Because if they fail so early in the boot process, the system is so
utterly messed up that we may as well just go oops.
diff mbox series

Patch

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index ccaa461998f3..df3e59f614cc 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -2426,15 +2426,17 @@  void __init vmalloc_init(void)
 	}
 
 	/* Import existing vmlist entries. */
-	for (tmp = vmlist; tmp; tmp = tmp->next) {
-		va = kmem_cache_zalloc(vmap_area_cachep, GFP_NOWAIT);
-		if (WARN_ON_ONCE(!va))
-			continue;
+	if (!WARN_ON_ONCE(!vmap_area_cachep)) {
+		for (tmp = vmlist; tmp; tmp = tmp->next) {
+			va = kmem_cache_zalloc(vmap_area_cachep, GFP_NOWAIT);
+			if (WARN_ON_ONCE(!va))
+				continue;
 
-		va->va_start = (unsigned long)tmp->addr;
-		va->va_end = va->va_start + tmp->size;
-		va->vm = tmp;
-		insert_vmap_area(va, &vmap_area_root, &vmap_area_list);
+			va->va_start = (unsigned long)tmp->addr;
+			va->va_end = va->va_start + tmp->size;
+			va->vm = tmp;
+			insert_vmap_area(va, &vmap_area_root, &vmap_area_list);
+		}
 	}
 
 	/*