Require mmap handler for a.out executables

Require mmap handler for a.out executables

[Marcel Holtmann]

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

Hi Linus,

with the nasty /proc privilege escalation (CVE-2006-3626) it became
clear that we need to do something more to better protect us against
people exploiting stuff in /proc. Besides the don't allow chmod stuff,
Eugene also proposed to depend the a.out execution on the existence of
the mmap handler. Since we are doing the same for ELF, this makes
totally sense to me.

The attached patch implements the additional check for the mmap handler
and I hope you consider it for upstream inclusion.

Regards

Marcel


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 1448 bytes --]

[PATCH] Require mmap handler for a.out executables

Files supported by fs/proc/base.c, i.e. /proc/<pid>/*, are not capable
of meeting the validity checks in ELF load_elf_*() handling because they
have no mmap handler which is required by ELF. In order to stop a.out
executables being used as part of an exploit attack against /proc-related
vulnerabilities, we make a.out executables depend on ->mmap() existing.

Signed-off-by: Eugene Teo <eteo@redhat.com>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>

---
commit 1597cf8405734e4747c808bb7e04115a6670dccf
tree 49050549aee6406dab0c021c5aa4e9bfc337bd8f
parent 44eb123126d289bac398cac0232309c228386671
author Marcel Holtmann <marcel@holtmann.org> Wed, 26 Jul 2006 12:12:14 +0200
committer Marcel Holtmann <marcel@holtmann.org> Wed, 26 Jul 2006 12:12:14 +0200

 fs/binfmt_aout.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
index f312103..5638acf 100644
--- a/fs/binfmt_aout.c
+++ b/fs/binfmt_aout.c
@@ -278,6 +278,9 @@ static int load_aout_binary(struct linux
 		return -ENOEXEC;
 	}
 
+	if (!bprm->file->f_op || !bprm->file->f_op->mmap)
+		return -ENOEXEC;
+
 	fd_offset = N_TXTOFF(ex);
 
 	/* Check initial limits. This avoids letting people circumvent
@@ -476,6 +479,9 @@ static int load_aout_library(struct file
 		goto out;
 	}
 
+	if (!file->f_op || !file->f_op->mmap)
+		goto out;
+
 	if (N_FLAGS(ex))
 		goto out;
 

Re: Require mmap handler for a.out executables

[Christoph Hellwig]

> diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
> index f312103..5638acf 100644
> --- a/fs/binfmt_aout.c
> +++ b/fs/binfmt_aout.c
> @@ -278,6 +278,9 @@ static int load_aout_binary(struct linux
>  		return -ENOEXEC;
>  	}
>  
> +	if (!bprm->file->f_op || !bprm->file->f_op->mmap)
> +		return -ENOEXEC;
> +

These checks need a big comment explanining why they are there, else people
will remove them again by accident.

Re: Require mmap handler for a.out executables

[Linus Torvalds]

On Thu, 27 Jul 2006, Christoph Hellwig wrote:
>
> > diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
> > index f312103..5638acf 100644
> > --- a/fs/binfmt_aout.c
> > +++ b/fs/binfmt_aout.c
> > @@ -278,6 +278,9 @@ static int load_aout_binary(struct linux
> >  		return -ENOEXEC;
> >  	}
> >  
> > +	if (!bprm->file->f_op || !bprm->file->f_op->mmap)
> > +		return -ENOEXEC;
> > +
> 
> These checks need a big comment explanining why they are there, else people
> will remove them again by accident.

Since we fixed the /proc problem in a different way, I decided that it 
might be best to leave the a.out stuff alone, at least for now. It is 
conceivable that somebody actually might be using executables on some 
strange filesystem that doesn't support mmap, although I can't for the 
moment think of any good reason. 

		Linus

Re: Require mmap handler for a.out executables

[Marcel Holtmann]

Hi Linus,

> > > diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
> > > index f312103..5638acf 100644
> > > --- a/fs/binfmt_aout.c
> > > +++ b/fs/binfmt_aout.c
> > > @@ -278,6 +278,9 @@ static int load_aout_binary(struct linux
> > >  		return -ENOEXEC;
> > >  	}
> > >  
> > > +	if (!bprm->file->f_op || !bprm->file->f_op->mmap)
> > > +		return -ENOEXEC;
> > > +
> > 
> > These checks need a big comment explanining why they are there, else people
> > will remove them again by accident.
> 
> Since we fixed the /proc problem in a different way, I decided that it 
> might be best to leave the a.out stuff alone, at least for now. It is 
> conceivable that somebody actually might be using executables on some 
> strange filesystem that doesn't support mmap, although I can't for the 
> moment think of any good reason. 

what do think about giving this a spin in -mm for some time and see if
it will break for somebody.

Andrew, please include it.

Regards

Marcel

Re: Require mmap handler for a.out executables

[Eugene Teo]

Christoph Hellwig wrote:
>> diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
>> index f312103..5638acf 100644
>> --- a/fs/binfmt_aout.c
>> +++ b/fs/binfmt_aout.c
>> @@ -278,6 +278,9 @@ static int load_aout_binary(struct linux
>>  		return -ENOEXEC;
>>  	}
>>  
>> +	if (!bprm->file->f_op || !bprm->file->f_op->mmap)
>> +		return -ENOEXEC;
>> +
> 
> These checks need a big comment explanining why they are there, else people
> will remove them again by accident.

Here's a resend.

Like what Marcel wrote, Andrew, please include this patch in -mm for testing.
Thanks.

Eugene
--

[PATCH] Require mmap handler for a.out executables

Files supported by fs/proc/base.c, i.e. /proc/<pid>/*, are not capable
of meeting the validity checks in ELF load_elf_*() handling because they
have no mmap handler which is required by ELF. In order to stop a.out
executables being used as part of an exploit attack against /proc-related
vulnerabilities, we make a.out executables depend on ->mmap() existing.

Signed-off-by: Eugene Teo <eteo@redhat.com>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>

diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
index f312103..2042dfa 100644
--- a/fs/binfmt_aout.c
+++ b/fs/binfmt_aout.c
@@ -278,6 +278,12 @@ static int load_aout_binary(struct linux
                return -ENOEXEC;
        }

+       /* Requires a mmap handler. This prevents people from using a.out
+        * as part of an exploit attack against /proc-related vulnerabilities.
+        */
+       if (!bprm->file->f_op || !bprm->file->f_op->mmap)
+               return -ENOEXEC;
+
        fd_offset = N_TXTOFF(ex);

        /* Check initial limits. This avoids letting people circumvent
@@ -476,6 +482,12 @@ static int load_aout_library(struct file
                goto out;
        }

+       /* Requires a mmap handler. This prevents people from using a.out
+        * as part of an exploit attack against /proc-related vulnerabilities.
+        */
+       if (!file->f_op || !file->f_op->mmap)
+               goto out;
+
        if (N_FLAGS(ex))
                goto out;

-- 
eteo redhat.com  ph: +65 6490 4142  http://www.kernel.org/~eugeneteo
gpg fingerprint:  47B9 90F6 AE4A 9C51 37E0  D6E1 EA84 C6A2 58DF 8823

Re: Require mmap handler for a.out executables

[Bodo Eggert]

Marcel Holtmann <marcel@holtmann.org> wrote:

> with the nasty /proc privilege escalation (CVE-2006-3626) it became
> clear that we need to do something more to better protect us against
> people exploiting stuff in /proc. Besides the don't allow chmod stuff,
> Eugene also proposed to depend the a.out execution on the existence of
> the mmap handler. Since we are doing the same for ELF, this makes
> totally sense to me.

Can shell scripts or binfmt_misc be exploited, too? Even if not, I'd
additionally force noexec, nosuid on proc and sysfs mounts.
-- 
Ich danke GMX dafür, die Verwendung meiner Adressen mittels per SPF
verbreiteten Lügen zu sabotieren.

http://david.woodhou.se/why-not-spf.html

Re: Require mmap handler for a.out executables

[Eugene Teo]

Bodo Eggert wrote:
> Marcel Holtmann <marcel@holtmann.org> wrote:
> 
>> with the nasty /proc privilege escalation (CVE-2006-3626) it became
>> clear that we need to do something more to better protect us against
>> people exploiting stuff in /proc. Besides the don't allow chmod stuff,
>> Eugene also proposed to depend the a.out execution on the existence of
>> the mmap handler. Since we are doing the same for ELF, this makes
>> totally sense to me.
> 
> Can shell scripts or binfmt_misc be exploited, too? Even if not, I'd
> additionally force noexec, nosuid on proc and sysfs mounts.

Right. That's why we do not allow chmod() /proc/*/*/* files.
http://kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=6d76fa58b050044994fe25f8753b8023f2b36737

Eugene
-- 
eteo redhat.com  ph: +65 6490 4142  http://www.kernel.org/~eugeneteo
gpg fingerprint:  47B9 90F6 AE4A 9C51 37E0  D6E1 EA84 C6A2 58DF 8823

Re: Require mmap handler for a.out executables

[Alan Cox]

Ar Iau, 2006-07-27 am 19:49 +0200, ysgrifennodd Bodo Eggert:
> Can shell scripts or binfmt_misc be exploited, too? Even if not, I'd
> additionally force noexec, nosuid on proc and sysfs mounts.

Why force them, this is just papering over imagined cracks and running
from shadows. If users want to be paranoid about these file systems or
their distro vendor is smart then the ability to set noexec/nosuid is
already supported and even more can be done with selinux. In fact as its
usually mounted in one place even AppArmor might be able to get it right
8)

Re: Require mmap handler for a.out executables

[Bodo Eggert]

On Thu, 27 Jul 2006, Alan Cox wrote:
> Ar Iau, 2006-07-27 am 19:49 +0200, ysgrifennodd Bodo Eggert:

> > Can shell scripts or binfmt_misc be exploited, too? Even if not, I'd
> > additionally force noexec, nosuid on proc and sysfs mounts.
> 
> Why force them, this is just papering over imagined cracks and running
> from shadows. If users want to be paranoid about these file systems or
> their distro vendor is smart then the ability to set noexec/nosuid is
> already supported and even more can be done with selinux. In fact as its
> usually mounted in one place even AppArmor might be able to get it right
> 8)

s/force/default to/, since it's not OK to let the admin shoot his feet 
unless he _explicitely_ demands to. What if the next crack allows evading 
nosuid by using proc?

Being paranoid doesn't mean they aren't after you ...
-- 
bus error. passengers dumped.