2006-08-19 23:29:55

by Solar Designer

[permalink] [raw]
Subject: [PATCH] introduce CONFIG_BINFMT_ELF_AOUT

Willy,

I propose the attached patch (extracted from 2.4.33-ow1) for inclusion
into 2.4.34-pre. (2.6 kernels could benefit from the same change, too.)

The patch adds a new compile-time option to control the support for
"ELF binaries with a.out format interpreters or a.out libraries".
Without this patch, such support is enabled on every system that enables
the support for ELF binaries - although 99% (100%?) of systems don't
need this hybrid functionality. Moreover, this functionality poses a
security risk - as proven in practice:

http://www.isec.pl/vulnerabilities/isec-0021-uselib.txt

This uselib() vulnerability did not affect default kernel builds with
the -ow patch specifically due to separation of the unneeded/risky code
into CONFIG_BINFMT_ELF_AOUT and having this option disabled by default.
(Yes, this change in -ow patches pre-dates the discovery of the uselib()
vulnerability.)

The patch also changes CONFIG_BINFMT_AOUT to be disabled by default on
archs that had it default to enabled. The a.out support is similarly
risky and not audited/hardened with the same scrutiny that the ELF
support has received.

Thanks,

Alexander


Attachments:
(No filename) (1.12 kB)
linux-2.4.33-ow1-CONFIG_BINFMT_ELF_AOUT.diff (26.22 kB)
Download all attachments

2006-08-20 00:17:13

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH] introduce CONFIG_BINFMT_ELF_AOUT

On Sun, Aug 20, 2006 at 03:25:56AM +0400, Solar Designer wrote:
> Willy,
>
> I propose the attached patch (extracted from 2.4.33-ow1) for inclusion
> into 2.4.34-pre. (2.6 kernels could benefit from the same change, too.)
>
> The patch adds a new compile-time option to control the support for
> "ELF binaries with a.out format interpreters or a.out libraries".
> Without this patch, such support is enabled on every system that enables
> the support for ELF binaries - although 99% (100%?) of systems don't
> need this hybrid functionality.

I remember having used this patch in a not-so-distant past without any
side effect. Also, 2.4 now mostly runs on servers with a well known
userland, so I believe that being able to disable ELF_AOUT may serve
some users who either want to harden their system or simply reduce its
footprint.

> Moreover, this functionality poses a
> security risk - as proven in practice:
>
> http://www.isec.pl/vulnerabilities/isec-0021-uselib.txt
>
> This uselib() vulnerability did not affect default kernel builds with
> the -ow patch specifically due to separation of the unneeded/risky code
> into CONFIG_BINFMT_ELF_AOUT and having this option disabled by default.
> (Yes, this change in -ow patches pre-dates the discovery of the uselib()
> vulnerability.)

I remember about it (the vuln), I even used it as a PoC.

> The patch also changes CONFIG_BINFMT_AOUT to be disabled by default on
> archs that had it default to enabled.

However, I don't agree with this part in mainline. While I'm happy to
let the user disable useless/dangerous/untested features, there are
people who build kernels by appending just a few lines to default configs.
I don't want to change their default settings without them noticing this,
even if there's virtually no risk of breaking anything. Same goes for
BINFMT_MISC which got disabled by default in your patch.

A general thumb rule is to allow people to hold the 'Enter' key pressed
during make oldconfig and get identical features as before. This is really
important to maintain the rate of wrong bug reports very low.

> The a.out support is similarly risky and not audited/hardened with the
> same scrutiny that the ELF support has received.

I know and agree with you on this matter. Most people compiling 2.4 for
servers right now most probably do not enable support for a.out already.

So to resume, what I can propose you is :
- you split the defconfig changes from the rest and let them in a
state compatible with 2.4.33 features, which even implies setting
CONFIG_BINFMT_ELF_AOUT to 'y', even if this sounds gross to you.
- I merge the changes to support the new option
- you just have to maintain the patch for the defconfig files in owl.

I can also do the split myself if you don't have time, but this work
will get less priority then (since my time is finite too).

Also, you spoke about 2.6. I would like that you keep a list of the
patches from your tree that get merged into 2.4 and which should be
proposed to 2.6. Maybe you'll only propose them when you work on 2.6-owl,
but I would like to ensure that those enhancements don't get lost once
they are in 2.4 mainline.

> Thanks,
>
> Alexander

Thanks,
Willy

2006-08-21 00:14:30

by Marcelo Tosatti

[permalink] [raw]
Subject: Re: [PATCH] introduce CONFIG_BINFMT_ELF_AOUT

On Sun, Aug 20, 2006 at 03:25:56AM +0400, Solar Designer wrote:
> Willy,
>
> I propose the attached patch (extracted from 2.4.33-ow1) for inclusion
> into 2.4.34-pre. (2.6 kernels could benefit from the same change, too.)
>
> The patch adds a new compile-time option to control the support for
> "ELF binaries with a.out format interpreters or a.out libraries".
> Without this patch, such support is enabled on every system that enables
> the support for ELF binaries - although 99% (100%?) of systems don't
> need this hybrid functionality. Moreover, this functionality poses a
> security risk - as proven in practice:
>
> http://www.isec.pl/vulnerabilities/isec-0021-uselib.txt
>
> This uselib() vulnerability did not affect default kernel builds with
> the -ow patch specifically due to separation of the unneeded/risky code
> into CONFIG_BINFMT_ELF_AOUT and having this option disabled by default.
> (Yes, this change in -ow patches pre-dates the discovery of the uselib()
> vulnerability.)
>
> The patch also changes CONFIG_BINFMT_AOUT to be disabled by default on
> archs that had it default to enabled. The a.out support is similarly
> risky and not audited/hardened with the same scrutiny that the ELF
> support has received.

I dislike this change. "Make a.out configurable" is a:

- "Hide the problems" trick, making it less likely for any potential bug to
be really fixed.

- Change not suitable for v2.4 inclusion: its not fixing _any_ serious problem.

We had this discussion before, didnt we?

2006-08-21 00:37:27

by Solar Designer

[permalink] [raw]
Subject: Re: [PATCH] introduce CONFIG_BINFMT_ELF_AOUT

On Sun, Aug 20, 2006 at 09:16:29PM -0300, Marcelo Tosatti wrote:
> I dislike this change.

Which one? The introduction of CONFIG_BINFMT_ELF_AOUT or having it and
CONFIG_BINFMT_AOUT disabled by default - or both?

> We had this discussion before, didnt we?

Yes, you had proposed the same thing that Willy did - to introduce
CONFIG_BINFMT_ELF_AOUT but have it default to enabled, and to not
change any other defaults. I simply haven't had the time (nor
motivation since this almost defeats the purpose of the patch) to
re-arrange the patch for that yet, so I decided to post what I readily
had first for public comment. I should have mentioned this past
discussion in my posting, sorry.

Thanks,

Alexander

2006-08-21 00:49:22

by Marcelo Tosatti

[permalink] [raw]
Subject: Re: [PATCH] introduce CONFIG_BINFMT_ELF_AOUT

Hi Solar,

On Mon, Aug 21, 2006 at 04:33:21AM +0400, Solar Designer wrote:
> On Sun, Aug 20, 2006 at 09:16:29PM -0300, Marcelo Tosatti wrote:
> > I dislike this change.
>
> Which one? The introduction of CONFIG_BINFMT_ELF_AOUT or having it and
> CONFIG_BINFMT_AOUT disabled by default - or both?

Both actually. Its not 2.4 material at this point in time.

> > We had this discussion before, didnt we?
>
> Yes, you had proposed the same thing that Willy did - to introduce
> CONFIG_BINFMT_ELF_AOUT but have it default to enabled, and to not
> change any other defaults. I simply haven't had the time (nor
> motivation since this almost defeats the purpose of the patch) to
> re-arrange the patch for that yet, so I decided to post what I readily
> had first for public comment. I should have mentioned this past
> discussion in my posting, sorry.

No problem.

To be sincere, I'd prefer to see fixes for potential security bugs in
the a.out code rather than making it optional (so, it appears that I've
got a different opinion now).

> Thanks,

Thank you for resubmitting your patches...

2006-08-21 01:11:04

by Solar Designer

[permalink] [raw]
Subject: Re: [PATCH] introduce CONFIG_BINFMT_ELF_AOUT

Willy,

On Sun, Aug 20, 2006 at 02:16:37AM +0200, Willy Tarreau wrote:
> Most people compiling 2.4 for
> servers right now most probably do not enable support for a.out already.

I'm afraid that most don't change the default, not being aware that this
is an unreasonable security risk.

> - you split the defconfig changes from the rest and let them in a
> state compatible with 2.4.33 features, which even implies setting
> CONFIG_BINFMT_ELF_AOUT to 'y', even if this sounds gross to you.
> - I merge the changes to support the new option

I can do that, but:

- it almost defeats the purpose of the patch since most people won't
know to change the defaults;
- Marcelo is of the opinion that it's "not 2.4 material at this point in
time".

Given the above, do you still want me to resubmit a reworked patch like
that?

> - you just have to maintain the patch for the defconfig files in owl.

I submit these patches in hope that they will be useful for mainstream
kernels, not in an attempt to simplify maintenance of -ow patches.

Thanks,

Alexander

2006-08-21 04:51:57

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH] introduce CONFIG_BINFMT_ELF_AOUT

On Mon, Aug 21, 2006 at 05:07:00AM +0400, Solar Designer wrote:
> Willy,
>
> On Sun, Aug 20, 2006 at 02:16:37AM +0200, Willy Tarreau wrote:
> > Most people compiling 2.4 for
> > servers right now most probably do not enable support for a.out already.
>
> I'm afraid that most don't change the default, not being aware that this
> is an unreasonable security risk.
>
> > - you split the defconfig changes from the rest and let them in a
> > state compatible with 2.4.33 features, which even implies setting
> > CONFIG_BINFMT_ELF_AOUT to 'y', even if this sounds gross to you.
> > - I merge the changes to support the new option
>
> I can do that, but:
>
> - it almost defeats the purpose of the patch since most people won't
> know to change the defaults;
> - Marcelo is of the opinion that it's "not 2.4 material at this point in
> time".
>
> Given the above, do you still want me to resubmit a reworked patch like
> that?

Well, do not bother then.

> > - you just have to maintain the patch for the defconfig files in owl.
>
> I submit these patches in hope that they will be useful for mainstream
> kernels, not in an attempt to simplify maintenance of -ow patches.

I'm perfectly aware of this. You proposed me some of your patches which
have proved useful in your tree, I agreed to review them but other people
are more reluctant than me because those patches are prevention measures
and don't fix anything. Well, end of the story. Keep them in -ow, and I
will also push some of them in my own tree because I understand why they
can help. That's just a matter of opinion.

> Thanks,
>
> Alexander

Thanks,
Willy