Re: [RFC][PATCH 2/2] binfmt_elf: FatELF support for kernel modules.

[AmÃrico Wang]

On Mon, Oct 19, 2009 at 10:41 PM, Ryan C. Gordon <icculus@xxxxxxxxxxx> wrote:
>
> Allows kernel modules to be FatELF binaries.
>
> Details, rationale, tools, and patches for handling FatELF binaries can be
> found at http://icculus.org/fatelf/
>
> Please note that this requires an updated depmod and modprobe to be truly
> effective, but an unmodified insmod can work with FatELF binaries.
>
> Signed-off-by: Ryan C. Gordon <icculus@xxxxxxxxxxx>
> ---
> Âkernel/module.c | Â 72 +++++++++++++++++++++++++++++++++++++++++++++++++++----
> Â1 files changed, 67 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index 8b7d880..cda8f79 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2066,13 +2066,69 @@ static inline void kmemleak_load_module(struct module *mod, Elf_Ehdr *hdr,
> Â}
> Â#endif
>
> +/*
> + * See if we're a valid FatELF binary, find the right record, and
> + * Âreturn the offset of that record within the binary. Returns NULL if there's
> + * Âa problem, or a pointer to the real ELF header if we're okay.
> + * ÂIf we don't see the FatELF magic number, we assume this is a regular ELF
> + * Âbinary and let the regular ELF checks handle it.
> + *
> + * This is a simplified version of examine_fatelf in fs/binfmt_elf.c
> + */
> +static Elf_Ehdr *examine_fatelf_module(const unsigned char *hdr,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âconst unsigned long len)
> +{
> + Â Â Â Elf_Ehdr elf;
> + Â Â Â int records, i;
> + Â Â Â const fatelf_hdr *fatelf = (const fatelf_hdr *) hdr;


As for 'Elf_Ehdr', isn't 'Fatelf_hdr' better? :)

> +
> + Â Â Â if (likely(le32_to_cpu(fatelf->magic) != FATELF_MAGIC)) {
> + Â Â Â Â Â Â Â return (Elf_Ehdr *) hdr; Â/* not FatELF; not an error. */
> + Â Â Â } else if (unlikely(le16_to_cpu(fatelf->version) != 1)) {
> + Â Â Â Â Â Â Â return NULL; /* Unrecognized format version. */
> + Â Â Â }
> +


These braces are unnecessary.

> + Â Â Â memset(&elf, 0, sizeof (elf));
> +
> + Â Â Â records = (int) fatelf->num_records; Â/* uint8, no byteswap needed */
> + Â Â Â for (i = 0; i < records; i++) {
> + Â Â Â Â Â Â Â const fatelf_record *record = &fatelf->records[i];
> +
> + Â Â Â Â Â Â Â /* Fill in the data elf_check_arch() might care about. */
> + Â Â Â Â Â Â Â elf.e_ident[EI_OSABI] = record->osabi;
> + Â Â Â Â Â Â Â elf.e_ident[EI_CLASS] = record->word_size;
> + Â Â Â Â Â Â Â elf.e_ident[EI_DATA] = record->byte_order;
> + Â Â Â Â Â Â Â elf.e_machine = le16_to_cpu(record->machine);
> +
> + Â Â Â Â Â Â Â if (likely(!elf_check_arch(&elf))) {
> + Â Â Â Â Â Â Â Â Â Â Â continue; Â/* Unsupported CPU architecture. */

This 'continue' can be removed.


> + Â Â Â Â Â Â Â } else {
> + Â Â Â Â Â Â Â Â Â Â Â const __u64 rec_offset = le64_to_cpu(record->offset);
> + Â Â Â Â Â Â Â Â Â Â Â const __u64 rec_size = le64_to_cpu(record->size);
> + Â Â Â Â Â Â Â Â Â Â Â const __u64 end_offset = rec_offset + rec_size;
> + Â Â Â Â Â Â Â Â Â Â Â const unsigned long uloff = (unsigned long) rec_offset;
> +
> + Â Â Â Â Â Â Â Â Â Â Â if (unlikely(end_offset < rec_offset)) {
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â continue; Â/* overflow (corrupt file?)... */


ditto

> + Â Â Â Â Â Â Â Â Â Â Â } else if (unlikely(end_offset > len)) {
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â continue; Â/* past EOF. */

ditto

> + Â Â Â Â Â Â Â Â Â Â Â }
> +
> + Â Â Â Â Â Â Â Â Â Â Â return (Elf_Ehdr *) (hdr + uloff);
> + Â Â Â Â Â Â Â }
> + Â Â Â }
> +
> + Â Â Â return NULL; Â/* no binaries we could use. */
> +}
> +
> Â/* Allocate and load the module: note that size of section 0 is always
> Â Âzero, and we rely on this for optional sections. */
> Âstatic noinline struct module *load_module(void __user *umod,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âunsigned long len,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âconst char __user *uargs)
> Â{
> - Â Â Â Elf_Ehdr *hdr;
> + Â Â Â Elf_Ehdr *hdr_alloc; Â/* returned from vmalloc */
> + Â Â Â Elf_Ehdr *hdr; Â/* adjusted hdr_alloc for FatELF */
> Â Â Â ÂElf_Shdr *sechdrs;
> Â Â Â Âchar *secstrings, *args, *modmagic, *strtab = NULL;
> Â Â Â Âchar *staging;
> @@ -2094,14 +2150,20 @@ static noinline struct module *load_module(void __user *umod,
>
> Â Â Â Â/* Suck in entire file: we'll want most of it. */
> Â Â Â Â/* vmalloc barfs on "unusual" numbers. ÂCheck here */
> - Â Â Â if (len > 64 * 1024 * 1024 || (hdr = vmalloc(len)) == NULL)
> + Â Â Â if (len > 64 * 1024 * 1024 || (hdr_alloc = vmalloc(len)) == NULL)
> Â Â Â Â Â Â Â Âreturn ERR_PTR(-ENOMEM);
>
> - Â Â Â if (copy_from_user(hdr, umod, len) != 0) {
> + Â Â Â if (copy_from_user(hdr_alloc, umod, len) != 0) {
> Â Â Â Â Â Â Â Âerr = -EFAULT;
> Â Â Â Â Â Â Â Âgoto free_hdr;
> Â Â Â Â}
>
> + Â Â Â hdr = examine_fatelf_module((unsigned char *) hdr_alloc, len);
> + Â Â Â if (hdr == NULL) {
> + Â Â Â Â Â Â Â err = -ENOEXEC;
> + Â Â Â Â Â Â Â goto free_hdr;
> + Â Â Â }
> +
> Â Â Â Â/* Sanity checks against insmoding binaries or wrong arch,
> Â Â Â Â Â Âweird elf version */
> Â Â Â Âif (memcmp(hdr->e_ident, ELFMAG, SELFMAG) != 0
> @@ -2505,7 +2567,7 @@ static noinline struct module *load_module(void __user *umod,
> Â Â Â Âadd_notes_attrs(mod, hdr->e_shnum, secstrings, sechdrs);
>
> Â Â Â Â/* Get rid of temporary copy */
> - Â Â Â vfree(hdr);
> + Â Â Â vfree(hdr_alloc);
>
> Â Â Â Âtrace_module_load(mod);
>
> @@ -2538,7 +2600,7 @@ static noinline struct module *load_module(void __user *umod,
> Â Â Â Âkfree(args);
> Â Â Â Âkfree(strmap);
> Âfree_hdr:
> - Â Â Â vfree(hdr);
> + Â Â Â vfree(hdr_alloc);
> Â Â Â Âreturn ERR_PTR(err);
>
> Âtruncated:
> --
> 1.6.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at Âhttp://vger.kernel.org/majordomo-info.html
> Please read the FAQ at Âhttp://www.tux.org/lkml/
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/