Skip to contentSkip to conversation

alpha: kmalloc failure ignored in process_reloc_for_got()

PATCH In review Roel Kluin 2 participants 3 messages Sep 8, 2009 – Sep 18, 2009 Alpha port

lore
Share & tools
lore

Roel Kluin

author
#0 lore raw

PATCH 1/1 alpha: kmalloc failure ignored in process_reloc_for_got()

Prevent NULL dereference if kmalloc() fails.

Signed-off-by Roel Kluin <[email protected]>

---
Found with sed: http://kernelnewbies.org/roelkluin

arch/alpha/kernel/module.c +2 -0
index ebc3c89..8ceec20 100644
@@ -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;
#1 lore raw
Quoted 11 lines · On Wed, 09 Sep 2009 01:40:10 +0200 Roel Kluin <[email protected]> wrote:

Prevent NULL dereference if kmalloc() fails.

Signed-off-by Roel Kluin <[email protected]>

---
Found with sed: http://kernelnewbies.org/roelkluin

arch/alpha/kernel/module.c +0 -0
index ebc3c89..8ceec20 100644

arch/alpha/kernel/module.c process_reloc_for_got(Elf64_Rela *rela, · line 80

Quoted 8 lines
arch/alpha/kernel/module.c +2 -0
@@ -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

author
#2 lore raw

Prevent NULL dereference if kmalloc() fails.

Signed-off-by Roel Kluin <[email protected]>

---

Quoted 2 lines

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.

arch/alpha/kernel/module.c +13 -5
index ebc3c89..5132d98 100644
@@ -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