2005-12-01 00:21:12

by David Gibson

Subject: Fix handling of ELF segments with zero filesize

Andrew, please apply

mmap() returns -EINVAL if given a zero length, and thus elf_map() in
binfmt_elf.c does likewise if it attempts to map a (page-aligned) ELF
segment with zero filesize. Such a situation never arises with the
default linker scripts, but there's nothing inherently wrong with
zero-filesize (but non-zero memsize) ELF segments. Custom linker
scripts can generate them, and the kernel should be able to map them;
this patch makes it so.

Signed-off-by: David Gibson <[email protected]>

Index: working-2.6/fs/binfmt_elf.c
===================================================================
--- working-2.6.orig/fs/binfmt_elf.c 2005-11-23 15:56:30.000000000 +1100
+++ working-2.6/fs/binfmt_elf.c 2005-12-01 11:11:01.000000000 +1100
@@ -288,11 +288,17 @@ static unsigned long elf_map(struct file
struct elf_phdr *eppnt, int prot, int type)
{
unsigned long map_addr;
+ unsigned long pageoffset = ELF_PAGEOFFSET(eppnt->p_vaddr);

down_write(&current->mm->mmap_sem);
- map_addr = do_mmap(filep, ELF_PAGESTART(addr),
- eppnt->p_filesz + ELF_PAGEOFFSET(eppnt->p_vaddr), prot, type,
- eppnt->p_offset - ELF_PAGEOFFSET(eppnt->p_vaddr));
+ /* mmap() will return -EINVAL if given a zero size, but a
+ * segment with zero filesize is perfectly valid */
+ if (eppnt->p_filesz + pageoffset)
+ map_addr = do_mmap(filep, ELF_PAGESTART(addr),
+ eppnt->p_filesz + pageoffset, prot, type,
+ eppnt->p_offset - pageoffset);
+ else
+ map_addr = ELF_PAGESTART(addr);
up_write(&current->mm->mmap_sem);
return(map_addr);
}

--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson

2005-12-01 00:26:38

by H. Peter Anvin

Subject: Re: Fix handling of ELF segments with zero filesize

David Gibson wrote:
> Andrew, please apply
>
> mmap() returns -EINVAL if given a zero length, and thus elf_map() in
> binfmt_elf.c does likewise if it attempts to map a (page-aligned) ELF
> segment with zero filesize. Such a situation never arises with the
> default linker scripts, but there's nothing inherently wrong with
> zero-filesize (but non-zero memsize) ELF segments. Custom linker
> scripts can generate them, and the kernel should be able to map them;
> this patch makes it so.
>

More than that: even with some versions of the default linker scripts
they can be created, according to reports I have received on the klibc
mailing list. It just doesn't happen with glibc binaries.

This is a real bug and should be fixed.

-hpa

2005-12-01 05:27:17

by Willy Tarreau

Subject: Re: Fix handling of ELF segments with zero filesize

On Thu, Dec 01, 2005 at 11:20:49AM +1100, David Gibson wrote:
> Andrew, please apply
>
> mmap() returns -EINVAL if given a zero length, and thus elf_map() in
> binfmt_elf.c does likewise if it attempts to map a (page-aligned) ELF
> segment with zero filesize. Such a situation never arises with the
> default linker scripts, but there's nothing inherently wrong with
> zero-filesize (but non-zero memsize) ELF segments. Custom linker
> scripts can generate them, and the kernel should be able to map them;
> this patch makes it so.

David, 2.4 has exactly the same code, do you see anything wrong with
applying this patch to 2.4 too ?

Thanks in advance,
Willy

> Signed-off-by: David Gibson <[email protected]>
>
> Index: working-2.6/fs/binfmt_elf.c
> ===================================================================
> --- working-2.6.orig/fs/binfmt_elf.c 2005-11-23 15:56:30.000000000 +1100
> +++ working-2.6/fs/binfmt_elf.c 2005-12-01 11:11:01.000000000 +1100
> @@ -288,11 +288,17 @@ static unsigned long elf_map(struct file
> struct elf_phdr *eppnt, int prot, int type)
> {
> unsigned long map_addr;
> + unsigned long pageoffset = ELF_PAGEOFFSET(eppnt->p_vaddr);
>
> down_write(&current->mm->mmap_sem);
> - map_addr = do_mmap(filep, ELF_PAGESTART(addr),
> - eppnt->p_filesz + ELF_PAGEOFFSET(eppnt->p_vaddr), prot, type,
> - eppnt->p_offset - ELF_PAGEOFFSET(eppnt->p_vaddr));
> + /* mmap() will return -EINVAL if given a zero size, but a
> + * segment with zero filesize is perfectly valid */
> + if (eppnt->p_filesz + pageoffset)
> + map_addr = do_mmap(filep, ELF_PAGESTART(addr),
> + eppnt->p_filesz + pageoffset, prot, type,
> + eppnt->p_offset - pageoffset);
> + else
> + map_addr = ELF_PAGESTART(addr);
> up_write(&current->mm->mmap_sem);
> return(map_addr);
> }
>
> --
> David Gibson | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2005-12-01 05:37:02

by David Gibson

Subject: Re: Fix handling of ELF segments with zero filesize

On Thu, Dec 01, 2005 at 06:26:42AM +0100, Willy Tarreau wrote:
> On Thu, Dec 01, 2005 at 11:20:49AM +1100, David Gibson wrote:
> > Andrew, please apply
> >
> > mmap() returns -EINVAL if given a zero length, and thus elf_map() in
> > binfmt_elf.c does likewise if it attempts to map a (page-aligned) ELF
> > segment with zero filesize. Such a situation never arises with the
> > default linker scripts, but there's nothing inherently wrong with
> > zero-filesize (but non-zero memsize) ELF segments. Custom linker
> > scripts can generate them, and the kernel should be able to map them;
> > this patch makes it so.
>
> David, 2.4 has exactly the same code, do you see anything wrong with
> applying this patch to 2.4 too ?

Nothing that I can think of.

--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson

2005-12-01 05:45:56

by Willy Tarreau

Subject: Re: Fix handling of ELF segments with zero filesize

On Thu, Dec 01, 2005 at 04:36:41PM +1100, David Gibson wrote:
> On Thu, Dec 01, 2005 at 06:26:42AM +0100, Willy Tarreau wrote:
> > On Thu, Dec 01, 2005 at 11:20:49AM +1100, David Gibson wrote:
> > > Andrew, please apply
> > >
> > > mmap() returns -EINVAL if given a zero length, and thus elf_map() in
> > > binfmt_elf.c does likewise if it attempts to map a (page-aligned) ELF
> > > segment with zero filesize. Such a situation never arises with the
> > > default linker scripts, but there's nothing inherently wrong with
> > > zero-filesize (but non-zero memsize) ELF segments. Custom linker
> > > scripts can generate them, and the kernel should be able to map them;
> > > this patch makes it so.
> >
> > David, 2.4 has exactly the same code, do you see anything wrong with
> > applying this patch to 2.4 too ?
>
> Nothing that I can think of.

Thanks, I'm queueing it for -hf and will resend it to Marcelo if he
misses it.

Regards,
Willy