Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] Bug fix to ELF Loader which rejects valid ELFs
@ 2020-08-11 14:44 Burrow, Ryan - 0553 - MITLL
  2020-08-11 15:05 ` Matthew Wilcox
  0 siblings, 1 reply; 3+ messages in thread
From: Burrow, Ryan - 0553 - MITLL @ 2020-08-11 14:44 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: linux-kernel

[-- Attachment #1: Type: text/plain, Size: 954 bytes --]


Check was incorrectly being applied to size of elf phdrs, instead
of the number. The ELF standard allows for up to 65535 headers, but
the check was being compared to the number of headers multiplied by
the size of a program header.
---
 fs/binfmt_elf.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 9fe3b51c116a..5605b5205f84 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -445,11 +445,11 @@ static struct elf_phdr *load_elf_phdrs(const struct
elfhdr *elf_ex,
 		goto out;
 
 	/* Sanity check the number of program headers... */
-	/* ...and their total size. */
-	size = sizeof(struct elf_phdr) * elf_ex->e_phnum;
-	if (size == 0 || size > 65536 || size > ELF_MIN_ALIGN)
+	if (elf_ex->e_phnum == 0 || elf_ex->phnum > 65535)
 		goto out;
 
+	size = sizeof(struct elf_phdr) * elf_ex->e_phnum;
+
 	elf_phdata = kmalloc(size, GFP_KERNEL);
 	if (!elf_phdata)
 		goto out;
-- 
2.17.1

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5529 bytes --]

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

* Re: [PATCH] Bug fix to ELF Loader which rejects valid ELFs
  2020-08-11 14:44 [PATCH] Bug fix to ELF Loader which rejects valid ELFs Burrow, Ryan - 0553 - MITLL
@ 2020-08-11 15:05 ` Matthew Wilcox
  2020-08-11 16:39   ` Burrow, Ryan - 0553 - MITLL
  0 siblings, 1 reply; 3+ messages in thread
From: Matthew Wilcox @ 2020-08-11 15:05 UTC (permalink / raw)
  To: Burrow, Ryan - 0553 - MITLL; +Cc: linux-fsdevel, linux-kernel

On Tue, Aug 11, 2020 at 02:44:08PM +0000, Burrow, Ryan - 0553 - MITLL wrote:
>  	/* Sanity check the number of program headers... */
> -	/* ...and their total size. */
> -	size = sizeof(struct elf_phdr) * elf_ex->e_phnum;
> -	if (size == 0 || size > 65536 || size > ELF_MIN_ALIGN)
> +	if (elf_ex->e_phnum == 0 || elf_ex->phnum > 65535)

umm, did you compile-test this?

assuming you meant e_phnum, it's a 16-bit quantity, so it can't be bigger
than 65535.

>  		goto out;
>  
> +	size = sizeof(struct elf_phdr) * elf_ex->e_phnum;

use array_size() here?




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

* RE: [PATCH] Bug fix to ELF Loader which rejects valid ELFs
  2020-08-11 15:05 ` Matthew Wilcox
@ 2020-08-11 16:39   ` Burrow, Ryan - 0553 - MITLL
  0 siblings, 0 replies; 3+ messages in thread
From: Burrow, Ryan - 0553 - MITLL @ 2020-08-11 16:39 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: linux-fsdevel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1371 bytes --]

> -----Original Message-----
> From: Matthew Wilcox <willy@infradead.org>
> Sent: Tuesday, August 11, 2020 11:05 AM
> To: Burrow, Ryan - 0553 - MITLL <Ryan.Burrow@ll.mit.edu>
> Cc: linux-fsdevel@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] Bug fix to ELF Loader which rejects valid ELFs
> 
> On Tue, Aug 11, 2020 at 02:44:08PM +0000, Burrow, Ryan - 0553 - MITLL
wrote:
> >  	/* Sanity check the number of program headers... */
> > -	/* ...and their total size. */
> > -	size = sizeof(struct elf_phdr) * elf_ex->e_phnum;
> > -	if (size == 0 || size > 65536 || size > ELF_MIN_ALIGN)
> > +	if (elf_ex->e_phnum == 0 || elf_ex->phnum > 65535)
> 
> umm, did you compile-test this?
> 

My apologies, I had made these edits in the context of other changes which I
didn't want to include in this patch - I replicated these changes
individually and (mistakenly) assumed I had done so correctly.

> assuming you meant e_phnum, it's a 16-bit quantity, so it can't be bigger
> than 65535.
> 
> >  		goto out;
> >
> > +	size = sizeof(struct elf_phdr) * elf_ex->e_phnum;
> 
> use array_size() here?
> 

That's a good catch - I had really just moved the previous allocation of
size down to after the check, but I'll use array_size instead. Just to
confirm the correct process, should I do an inline response with the updated
commit, or submit a new patch email?


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5529 bytes --]

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

end of thread, other threads:[~2020-08-11 16:39 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-11 14:44 [PATCH] Bug fix to ELF Loader which rejects valid ELFs Burrow, Ryan - 0553 - MITLL
2020-08-11 15:05 ` Matthew Wilcox
2020-08-11 16:39   ` Burrow, Ryan - 0553 - MITLL

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).