[PATCH] give elf_check_arch() a chance for checking endian mismatch

[PATCH] give elf_check_arch() a chance for checking endian mismatch

[Roy Lee]

I'd like to warn for endianess mismatch, or unsupported feature( hardware
float point support) when loading an ELF file.
The problem is that if the endianess mismatches happens, the logic inside
the elf_check_arch() won't have a chance to be executed.
Because the "elf_ex.e_type" would be misinterpreted and the elf_check_arch()
will be skipped.
This patch should give elf_check_arch() more chance to do some machine
dependent checking.

Roy

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 672a3b9..2ffbe49 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -567,10 +567,10 @@ static int load_elf_binary(struct linux_binprm *bprm,
struct pt_regs *regs)
        if (memcmp(loc->elf_ex.e_ident, ELFMAG, SELFMAG) != 0)
                goto out;

-       if (loc->elf_ex.e_type != ET_EXEC && loc->elf_ex.e_type != ET_DYN)
-               goto out;
        if (!elf_check_arch(&loc->elf_ex))
                goto out;
+       if (loc->elf_ex.e_type != ET_EXEC && loc->elf_ex.e_type != ET_DYN)
+               goto out;
        if (!bprm->file->f_op||!bprm->file->f_op->mmap)
                goto out;

Re: [PATCH] give elf_check_arch() a chance for checking endian mismatch

[Valdis.Kletnieks]

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

On Thu, 20 Mar 2008 03:03:30 +0800, Roy Lee said:

(Sorry for late reply, catching up finally)

> I'd like to warn for endianess mismatch, or unsupported feature( hardware
> float point support) when loading an ELF file.

> -       if (loc->elf_ex.e_type != ET_EXEC && loc->elf_ex.e_type != ET_DYN)
> -               goto out;
>         if (!elf_check_arch(&loc->elf_ex))
>                 goto out;
> +       if (loc->elf_ex.e_type != ET_EXEC && loc->elf_ex.e_type != ET_DYN)
> +               goto out;

It would seem to me that if it isn't an ET_EXEC or ET_DYN, the question of
whether it passes elf_check_arch() would be rather moot?  In any case, you
don't get to actually *warn* for it, because you end up with a 'goto out'
in either case, and elf_check_arch is a pretty small macro on most archs.

Also, if you change the order there, do you want to also change the order
in load_elf_interp(), where the equivalent test is done? 

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

RE: [PATCH] give elf_check_arch() a chance for checking endian mismatch

[Roy Lee]

> -----Original Message-----
> From: Valdis.Kletnieks@vt.edu [mailto:Valdis.Kletnieks@vt.edu]
> Sent: Sunday, March 23, 2008 1:06 PM
> To: Roy Lee
> Cc: linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] give elf_check_arch() a chance for checking endian
> mismatch
> 
> On Thu, 20 Mar 2008 03:03:30 +0800, Roy Lee said:
> 
> (Sorry for late reply, catching up finally)
> 
> > I'd like to warn for endianess mismatch, or unsupported feature(
> > hardware float point support) when loading an ELF file.
> 
> > -       if (loc->elf_ex.e_type != ET_EXEC && loc->elf_ex.e_type !=
> ET_DYN)
> > -               goto out;
> >         if (!elf_check_arch(&loc->elf_ex))
> >                 goto out;
> > +       if (loc->elf_ex.e_type != ET_EXEC && loc->elf_ex.e_type !=
> ET_DYN)
> > +               goto out;
> 
> It would seem to me that if it isn't an ET_EXEC or ET_DYN, the question of
> whether it passes elf_check_arch() would be rather moot?  In any case, you
> don't get to actually *warn* for it, because you end up with a 'goto out'
> in either case, and elf_check_arch is a pretty small macro on most archs.
> 

Endianness mismatch is not uncommon for bi-endian processors if their user
didn't 
carefully choose their tool-chain or missing flags during compilation.
It makes sense to add some logics for checking and issuing warning when this
happens.

Without changing the order of elf_check_arch(), even if the loaded file is
an 
ET_EXEC or ET_DYN, wrong endianness result in misinterpretation of
'elf_ex.e_type', and
end up skipping elf_check_arch() in which we implement endianness checking
as well as 
other ABI checking logics.

Changing the order of elf_check_arch() should not break current logics on
most archs.
If the loaded file isn't an ET_EXEC, ET_DYN or even not an ELF at all,
it has great chance to return false in the elf_check_arch() since the loaded

file will not satisfy their checking logic. Even if that's not the case, the
postponed
checking of ET_EXEC and ET_DYN will still redirect the flow to 'goto out'.

> Also, if you change the order there, do you want to also change the order
> in load_elf_interp(), where the equivalent test is done?
> 

Yes, I forgot to add them :)