2016-03-15 21:11:23

by Rob Landley

[permalink] [raw]
Subject: [PATCH] Remove v850 from linux/elf-em.h

From: Rob Landley <[email protected]>

The v850 port was removed by commits f606ddf42fd4 and 07a887d399b8 in 2008.
These #defines are not used in the current kernel.

Signed-off-by: Rob Landley <[email protected]>
---
include/uapi/linux/elf-em.h | 3 ---
1 file changed, 3 deletions(-)

diff --git a/include/uapi/linux/elf-em.h b/include/uapi/linux/elf-em.h
index b56dfcf..c3fdfe7 100644
--- a/include/uapi/linux/elf-em.h
+++ b/include/uapi/linux/elf-em.h
@@ -30,7 +30,6 @@
#define EM_X86_64 62 /* AMD x86-64 */
#define EM_S390 22 /* IBM S/390 */
#define EM_CRIS 76 /* Axis Communications 32-bit embedded processor */
-#define EM_V850 87 /* NEC v850 */
#define EM_M32R 88 /* Renesas M32R */
#define EM_MN10300 89 /* Panasonic/MEI MN10300, AM33 */
#define EM_OPENRISC 92 /* OpenRISC 32-bit embedded processor */
@@ -50,8 +49,6 @@
*/
#define EM_ALPHA 0x9026

-/* Bogus old v850 magic number, used by old tools. */
-#define EM_CYGNUS_V850 0x9080
/* Bogus old m32r magic number, used by old tools. */
#define EM_CYGNUS_M32R 0x9041
/* This is the old interim value for S/390 architecture */
--


2016-03-15 23:22:23

by David Daney

[permalink] [raw]
Subject: Re: [PATCH] Remove v850 from linux/elf-em.h

On 03/15/2016 02:10 PM, Rob Landley wrote:
> From: Rob Landley <[email protected]>
>
> The v850 port was removed by commits f606ddf42fd4 and 07a887d399b8 in 2008.
> These #defines are not used in the current kernel.
>
> Signed-off-by: Rob Landley <[email protected]>
> ---
> include/uapi/linux/elf-em.h | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/include/uapi/linux/elf-em.h b/include/uapi/linux/elf-em.h
> index b56dfcf..c3fdfe7 100644
> --- a/include/uapi/linux/elf-em.h
> +++ b/include/uapi/linux/elf-em.h
> @@ -30,7 +30,6 @@
> #define EM_X86_64 62 /* AMD x86-64 */
> #define EM_S390 22 /* IBM S/390 */
> #define EM_CRIS 76 /* Axis Communications 32-bit embedded processor */
> -#define EM_V850 87 /* NEC v850 */

Can you do this to userspace visible files?

I thought only additions and obvious corrections were allowed. Removing
symbols could cause build breakage for something.

> #define EM_M32R 88 /* Renesas M32R */
> #define EM_MN10300 89 /* Panasonic/MEI MN10300, AM33 */
> #define EM_OPENRISC 92 /* OpenRISC 32-bit embedded processor */
> @@ -50,8 +49,6 @@
> */
> #define EM_ALPHA 0x9026
>
> -/* Bogus old v850 magic number, used by old tools. */
> -#define EM_CYGNUS_V850 0x9080
> /* Bogus old m32r magic number, used by old tools. */
> #define EM_CYGNUS_M32R 0x9041
> /* This is the old interim value for S/390 architecture */
>

2016-03-16 07:11:57

by Rob Landley

[permalink] [raw]
Subject: Re: [PATCH] Remove v850 from linux/elf-em.h

On 03/15/2016 06:22 PM, David Daney wrote:
> On 03/15/2016 02:10 PM, Rob Landley wrote:
>> From: Rob Landley <[email protected]>
>>
>> The v850 port was removed by commits f606ddf42fd4 and 07a887d399b8 in
>> 2008.
>> These #defines are not used in the current kernel.
>>
>> Signed-off-by: Rob Landley <[email protected]>
>> ---
>> include/uapi/linux/elf-em.h | 3 ---
>> 1 file changed, 3 deletions(-)
>>
>> diff --git a/include/uapi/linux/elf-em.h b/include/uapi/linux/elf-em.h
>> index b56dfcf..c3fdfe7 100644
>> --- a/include/uapi/linux/elf-em.h
>> +++ b/include/uapi/linux/elf-em.h
>> @@ -30,7 +30,6 @@
>> #define EM_X86_64 62 /* AMD x86-64 */
>> #define EM_S390 22 /* IBM S/390 */
>> #define EM_CRIS 76 /* Axis Communications 32-bit embedded
>> processor */
>> -#define EM_V850 87 /* NEC v850 */
>
> Can you do this to userspace visible files?

Commit 6f6f467eaaa0 did and nobody seemed to mind?

> I thought only additions and obvious corrections were allowed. Removing
> symbols could cause build breakage for something.

There's a /usr/include/elf.h in glibc with 3 times as many EM_BLAH
symbols as this one defines. Something that cares about identifying
architectures Linux doesn't actually support would presumably use that
header instead of this one.

This is the only architecture included in this file that isn't currently
supported by Linux (except the mips symbols with the comment about them
not being used, which have been there since the first version of the file).

I found this while auditing toybox's "file" command. (We just copied the
constant values into our own table, but when making the table I was
checking which values the actual Linux ELF loaders in the various arch
directories would bind to.)

Rob

2016-03-17 22:09:28

by David Daney

[permalink] [raw]
Subject: Re: [PATCH] Remove v850 from linux/elf-em.h

On 03/16/2016 12:11 AM, Rob Landley wrote:
> On 03/15/2016 06:22 PM, David Daney wrote:
>> On 03/15/2016 02:10 PM, Rob Landley wrote:
>>> From: Rob Landley <[email protected]>
>>>
>>> The v850 port was removed by commits f606ddf42fd4 and 07a887d399b8 in
>>> 2008.
>>> These #defines are not used in the current kernel.
>>>
>>> Signed-off-by: Rob Landley <[email protected]>
>>> ---
>>> include/uapi/linux/elf-em.h | 3 ---
>>> 1 file changed, 3 deletions(-)
>>>
>>> diff --git a/include/uapi/linux/elf-em.h b/include/uapi/linux/elf-em.h
>>> index b56dfcf..c3fdfe7 100644
>>> --- a/include/uapi/linux/elf-em.h
>>> +++ b/include/uapi/linux/elf-em.h
>>> @@ -30,7 +30,6 @@
>>> #define EM_X86_64 62 /* AMD x86-64 */
>>> #define EM_S390 22 /* IBM S/390 */
>>> #define EM_CRIS 76 /* Axis Communications 32-bit embedded
>>> processor */
>>> -#define EM_V850 87 /* NEC v850 */
>>
>> Can you do this to userspace visible files?
>
> Commit 6f6f467eaaa0 did and nobody seemed to mind?

Evidence of prior art doesn't prove correctness.

>
>> I thought only additions and obvious corrections were allowed. Removing
>> symbols could cause build breakage for something.
>
> There's a /usr/include/elf.h in glibc with 3 times as many EM_BLAH
> symbols as this one defines. Something that cares about identifying
> architectures Linux doesn't actually support would presumably use that
> header instead of this one.
>
> This is the only architecture included in this file that isn't currently
> supported by Linux (except the mips symbols with the comment about them
> not being used, which have been there since the first version of the file).
>
> I found this while auditing toybox's "file" command. (We just copied the
> constant values into our own table, but when making the table I was
> checking which values the actual Linux ELF loaders in the various arch
> directories would bind to.)

The symbols are not hurting anything, they also do not have incorrect
values. They are however visible to userspace. I am not going to
comment on it any more, but there is a higher burden for removing things
that might break userspace programs, than there is for removing code
guaranteed not to be userspace visible.

I think the admonition about not breaking userspace has to be considered
in this case.

It could be that there is nothing that uses these symbols, but we have
no way of knowing for sure. So, unless there is a compelling reason to
do this, the lowest risk strategy is to do nothing.


David Daney


2016-03-18 02:33:20

by Rob Landley

[permalink] [raw]
Subject: Re: [PATCH] Remove v850 from linux/elf-em.h

On 03/17/2016 05:09 PM, David Daney wrote:
> On 03/16/2016 12:11 AM, Rob Landley wrote:
>> On 03/15/2016 06:22 PM, David Daney wrote:
>>> Can you do this to userspace visible files?
>>
>> Commit 6f6f467eaaa0 did and nobody seemed to mind?
>
> Evidence of prior art doesn't prove correctness.

It proves that you can, in fact, do this, by showing it's been done
before. It doesn't run anything through a theorem prover, I just
answered your question.

>> I found this while auditing toybox's "file" command. (We just copied the
>> constant values into our own table, but when making the table I was
>> checking which values the actual Linux ELF loaders in the various arch
>> directories would bind to.)
>
> The symbols are not hurting anything, they also do not have incorrect
> values.

These are the only values for an architecture we don't support. All the
other values are for architectures we support. (Yes, I went through the
full list. The only two not in use by an elf loader are the two MIPS
ones with the comment about being unused, but they've been around
forever and we still support mips.)

> They are however visible to userspace. I am not going to
> comment on it any more, but there is a higher burden for removing things
> that might break userspace programs, than there is for removing code
> guaranteed not to be userspace visible.
>
> I think the admonition about not breaking userspace has to be considered
> in this case.

You objected, I explained why I disagreed, you then replied that it must
be considered. So my previous email didn't count as "considering". (Only
agreement counts as considering?)

As I explained last email, userspace uses the libc header, not the linux
header, when it wants symbols for architectures Linux does not support.
Symbols Linux stopped supporting have been removed from this particular
header before. These are the only current "leaked" symbols in this file.
I cc'd the guy who removed them, in case he had a reason for keeping them.

> It could be that there is nothing that uses these symbols, but we have
> no way of knowing for sure. So, unless there is a compelling reason to
> do this, the lowest risk strategy is to do nothing.

The lowest risk strategy is usually to do nothing. Your machine could be
getting exploited connecting to the net to read this email. You are
exposing yourself to risk every time you inhale.

> David Daney

Rob

2016-03-18 17:47:00

by David Daney

[permalink] [raw]
Subject: Re: [PATCH] Remove v850 from linux/elf-em.h

On 03/17/2016 07:32 PM, Rob Landley wrote:
[...]
>
> As I explained last email, userspace uses the libc header, not the linux
> header,

The fallacy in this argument is the assertion that we know what
userspace does.

Userspace could easily do:

#include <linux/elf-em.h>
.
.
.
case SYMBOL_YOU_WANT_TO_REMOVE:

¡BOOM! it is broken.




2016-03-19 01:18:30

by Rob Landley

[permalink] [raw]
Subject: Re: [PATCH] Remove v850 from linux/elf-em.h

On 03/18/2016 12:46 PM, David Daney wrote:
> I am not going to comment on it any more, but [commenting more]

Yes you are. (And did then too.)

> On 03/17/2016 07:32 PM, Rob Landley wrote:
> [...]
>>
>> As I explained last email, userspace uses the libc header, not the linux
>> header,
>
> The fallacy in this argument is the assertion that we know what
> userspace does.

Userspace programs that did that already broke on earlier symbol removals.

> Userspace could easily do:
>
> #include <linux/elf-em.h>
> .
> case SYMBOL_YOU_WANT_TO_REMOVE:
>
> ¡BOOM! it is broken.

So you're assuming I don't know how headers get used by userspace.
That's nice. Clearly, I never would have thought of that.

Once again, "As I explained last email", symbols have been removed from
this particular header before. Therefore userspace programs that did
that without an #ifdef would have been broken by the previous commits
that removed symbols. There is a header they can use, provided by libc,
which exports all the defined ELF symbols _including_ the symbols the
Linux kernel does not suse. The Linux kernel header in question only
exports the symbols that this particular kernel version supports. When
architectures are added or removed, the corresponding symbols get added
or removed. I didn't invent the concept, it's happened before to this
header.

(In a larger context, we've removed bigger things. Remember when there
was a feature-removal-schedule.txt that listed things pending removal,
before Linus decided it was unnecessary and to just use his own
judgement? Yeah, take it up with him.)

And now I'm going to take your advice above. Please refer to my earlier
messages in this thread for answers to what you're about to say.

Rob

2016-03-21 16:39:38

by David Daney

[permalink] [raw]
Subject: Re: [PATCH] Remove v850 from linux/elf-em.h

On 03/18/2016 06:15 PM, Rob Landley wrote:
> On 03/18/2016 12:46 PM, David Daney wrote:
>> I am not going to comment on it any more, but [commenting more]
>
> Yes you are. (And did then too.)
>
>> On 03/17/2016 07:32 PM, Rob Landley wrote:
>> [...]
>>>
>>> As I explained last email, userspace uses the libc header, not the linux
>>> header,
>>
>> The fallacy in this argument is the assertion that we know what
>> userspace does.
>
> Userspace programs that did that already broke on earlier symbol removals.
>
>> Userspace could easily do:
>>
>> #include <linux/elf-em.h>
>> .
>> case SYMBOL_YOU_WANT_TO_REMOVE:
>>
>> ¡BOOM! it is broken.
>
> So you're assuming I don't know how headers get used by userspace.

Yes, exactly. Don't feel bad about it though, because nobody else knows
either.

> That's nice. Clearly, I never would have thought of that.
>
> Once again, "As I explained last email", symbols have been removed from
> this particular header before.

Since you know, a priori, that symbols in that file are never used by
userspace, why not send a patch that moves it to include/linux/elf-em.h
so that userspace doesn't see it? Then you could remove as many symbols
as you like.

David Daney

[...]