[PATCH] alpha: kmalloc failure ignored in process_reloc_for_got()

[PATCH] alpha: kmalloc failure ignored in process_reloc_for_got()

[Roel Kluin]

Prevent NULL dereference if kmalloc() fails.

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
Found with sed: http://kernelnewbies.org/roelkluin

diff --git a/arch/alpha/kernel/module.c b/arch/alpha/kernel/module.c
index ebc3c89..8ceec20 100644
--- a/arch/alpha/kernel/module.c
+++ b/arch/alpha/kernel/module.c
@@ -73,6 +73,8 @@ process_reloc_for_got(Elf64_Rela *rela,
 		}
 
 	g = kmalloc (sizeof (*g), GFP_KERNEL);
+	if (g == NULL)
+		return;
 	g->next = chains[r_sym].next;
 	g->r_addend = r_addend;
 	g->got_offset = *poffset;

Re: [PATCH] alpha: kmalloc failure ignored in process_reloc_for_got()

[Andrew Morton]

On Wed, 09 Sep 2009 01:40:10 +0200
Roel Kluin <roel.kluin@gmail.com> wrote:

> Prevent NULL dereference if kmalloc() fails.
> 
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> Found with sed: http://kernelnewbies.org/roelkluin
> 
> diff --git a/arch/alpha/kernel/module.c b/arch/alpha/kernel/module.c
> index ebc3c89..8ceec20 100644
> --- a/arch/alpha/kernel/module.c
> +++ b/arch/alpha/kernel/module.c
> @@ -73,6 +73,8 @@ process_reloc_for_got(Elf64_Rela *rela,
>  		}
>  
>  	g = kmalloc (sizeof (*g), GFP_KERNEL);
> +	if (g == NULL)
> +		return;
>  	g->next = chains[r_sym].next;
>  	g->r_addend = r_addend;
>  	g->got_offset = *poffset;

I don't know if that's an improvement.  afacit the kernel will now
blunder along and do something wrong.  An oops might well be
preferable behaviour.

IOW, we should handle this failure properly - back out, clean
everything up, return -ENOMEM to userspace

Re: [PATCH] alpha: kmalloc failure ignored in process_reloc_for_got()

[Roel Kluin]

Prevent NULL dereference if kmalloc() fails.

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
> we should handle this failure properly - back out, clean
> everything up, return -ENOMEM to userspace

Is this better? I have to admit I didn't build test it,
I may be able to do that later.

diff --git a/arch/alpha/kernel/module.c b/arch/alpha/kernel/module.c
index ebc3c89..5132d98 100644
--- a/arch/alpha/kernel/module.c
+++ b/arch/alpha/kernel/module.c
@@ -51,7 +51,7 @@ struct got_entry {
 	int got_offset;
 };
 
-static inline void
+static inline int
 process_reloc_for_got(Elf64_Rela *rela,
 		      struct got_entry *chains, Elf64_Xword *poffset)
 {
@@ -61,7 +61,7 @@ process_reloc_for_got(Elf64_Rela *rela,
 	struct got_entry *g;
 
 	if (r_type != R_ALPHA_LITERAL)
-		return;
+		return 0;
 
 	for (g = chains + r_sym; g ; g = g->next)
 		if (g->r_addend == r_addend) {
@@ -73,6 +73,8 @@ process_reloc_for_got(Elf64_Rela *rela,
 		}
 
 	g = kmalloc (sizeof (*g), GFP_KERNEL);
+	if (g == NULL)
+		return -ENOMEM;
 	g->next = chains[r_sym].next;
 	g->r_addend = r_addend;
 	g->got_offset = *poffset;
@@ -84,6 +86,7 @@ process_reloc_for_got(Elf64_Rela *rela,
 	   42 valid relocation types, and a 32-bit field.  Co-opt the
 	   bits above 256 to store the got offset for this reloc.  */
 	rela->r_info |= g->got_offset << 8;
+	return 0;
 }
 
 int
@@ -94,6 +97,7 @@ module_frob_arch_sections(Elf64_Ehdr *hdr, Elf64_Shdr *sechdrs,
 	Elf64_Rela *rela;
 	Elf64_Shdr *esechdrs, *symtab, *s, *got;
 	unsigned long nsyms, nrela, i;
+	int ret = 0;
 
 	esechdrs = sechdrs + hdr->e_shnum;
 	symtab = got = NULL;
@@ -137,9 +141,12 @@ module_frob_arch_sections(Elf64_Ehdr *hdr, Elf64_Shdr *sechdrs,
 		if (s->sh_type == SHT_RELA) {
 			nrela = s->sh_size / sizeof(Elf64_Rela);
 			rela = (void *)hdr + s->sh_offset;
-			for (i = 0; i < nrela; ++i)
-				process_reloc_for_got(rela+i, chains,
+			for (i = 0; i < nrela; ++i) {
+				ret = process_reloc_for_got(rela+i, chains,
 						      &got->sh_size);
+				if (ret != 0)
+					goto out;
+			}
 		}
 
 	/* Free the memory we allocated.  */
@@ -150,9 +157,10 @@ module_frob_arch_sections(Elf64_Ehdr *hdr, Elf64_Shdr *sechdrs,
 			kfree(g);
 		}
 	}
+out:
 	kfree(chains);
 
-	return 0;
+	return ret;
 }
 
 int