mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Quentin Casasnovas <quentin.casasnovas@oracle.com>
To: Quentin Casasnovas <quentin.casasnovas@oracle.com>,
	Rusty Russell <rusty@rustcorp.com.au>,
	Michal Marek <mmarek@suse.cz>
Cc: lkml <linux-kernel@vger.kernel.org>,
	Oleg Nesterov <oleg@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH 7/7] modpost: handle relocations mismatch in __ex_table.
Date: Wed, 18 Mar 2015 10:09:48 +0100	[thread overview]
Message-ID: <20150318090948.GL19131@chrystal.uk.oracle.com> (raw)
In-Reply-To: <1426596002-26128-8-git-send-email-quentin.casasnovas@oracle.com>

Adding Rusty and Michal to CC.

On Tue, Mar 17, 2015 at 01:40:02PM +0100, Quentin Casasnovas wrote:
> __ex_table is a simple table section where each entry is a pair of
> addresses - the first address is an address which can fault in kernel
> space, and the second address points to where the kernel should jump to
> when handling that fault.  This is how copy_from_user() does not crash the
> kernel if userspace gives a borked pointer for example.
> 
> If one of these addresses point to a non-executable section, something is
> seriously wrong since it either means the kernel will never fault from
> there or it will not be able to jump to there.  As both cases are serious
> enough, we simply error out in these cases so the build fails and the
> developper has to fix the issue.
> 
> In case the section is executable, but it isn't referenced in our list of
> authorized sections to point to from __ex_table, we just dump a warning
> giving more information about it.  We do this in case the new section is
> executable but isn't supposed to be executed by the kernel.  This happened
> with .altinstr_replacement, which is executable but is only used to copy
> instructions from - we should never have our instruction pointer pointing
> in .altinstr_replacement.  Admitedly, a proper fix in that case would be to
> just set .altinstr_replacement NX, but we need to warn about future cases
> like this.
> 
> Signed-off-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
> ---
>  scripts/mod/modpost.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 141 insertions(+)
> 
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index bf0cf81..dfe9c3c 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -875,6 +875,8 @@ static void check_section(const char *modname, struct elf_info *elf,
>  #define DATA_SECTIONS ".data", ".data.rel"
>  #define TEXT_SECTIONS ".text", ".text.unlikely", ".sched.text", \
>  		".kprobes.text"
> +#define OTHER_TEXT_SECTIONS ".ref.text", ".head.text", ".spinlock.text", \
> +		".fixup", ".entry.text"
>  
>  #define INIT_SECTIONS      ".init.*"
>  #define MEM_INIT_SECTIONS  ".meminit.*"
> @@ -882,6 +884,9 @@ static void check_section(const char *modname, struct elf_info *elf,
>  #define EXIT_SECTIONS      ".exit.*"
>  #define MEM_EXIT_SECTIONS  ".memexit.*"
>  
> +#define ALL_TEXT_SECTIONS  ALL_INIT_TEXT_SECTIONS, ALL_EXIT_TEXT_SECTIONS, \
> +		TEXT_SECTIONS, OTHER_TEXT_SECTIONS
> +
>  /* init data sections */
>  static const char *const init_data_sections[] =
>  	{ ALL_INIT_DATA_SECTIONS, NULL };
> @@ -922,6 +927,7 @@ enum mismatch {
>  	ANY_INIT_TO_ANY_EXIT,
>  	ANY_EXIT_TO_ANY_INIT,
>  	EXPORT_TO_INIT_EXIT,
> +	EXTABLE_TO_NON_TEXT,
>  };
>  
>  struct sectioncheck {
> @@ -936,6 +942,11 @@ struct sectioncheck {
>  
>  };
>  
> +static void extable_mismatch_handler(const char *modname, struct elf_info *elf,
> +				     const struct sectioncheck* const mismatch,
> +				     Elf_Rela *r, Elf_Sym *sym,
> +				     const char *fromsec);
> +
>  static const struct sectioncheck sectioncheck[] = {
>  /* Do not reference init/exit code/data from
>   * normal code and data
> @@ -1013,6 +1024,16 @@ static const struct sectioncheck sectioncheck[] = {
>  	.bad_tosec = { INIT_SECTIONS, EXIT_SECTIONS, NULL },
>  	.mismatch = EXPORT_TO_INIT_EXIT,
>  	.symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL },
> +},
> +{
> +	.fromsec = { "__ex_table", NULL },
> +	/* If you're adding any new black-listed sections in here, consider
> +	 * adding a special 'printer' for them in scripts/check_extable.
> +	 */
> +	.bad_tosec = { ".altinstr_replacement", NULL },
> +	.good_tosec = {ALL_TEXT_SECTIONS , NULL},
> +	.mismatch = EXTABLE_TO_NON_TEXT,
> +	.handler = extable_mismatch_handler,
>  }
>  };
>  
> @@ -1418,6 +1439,10 @@ static void report_sec_mismatch(const char *modname,
>  		tosym, prl_to, prl_to, tosym);
>  		free(prl_to);
>  		break;
> +	case EXTABLE_TO_NON_TEXT:
> +		fatal("There's a special handler for this mismatch type, "
> +		      "we should never get here.");
> +		break;
>  	}
>  	fprintf(stderr, "\n");
>  }
> @@ -1453,6 +1478,120 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
>  	}
>  }
>  
> +static int is_executable_section(struct elf_info* elf, unsigned int section_index)
> +{
> +	if (section_index > elf->num_sections)
> +		fatal("section_index is outside elf->num_sections!\n");
> +
> +	return ((elf->sechdrs[section_index].sh_flags & SHF_EXECINSTR) == SHF_EXECINSTR);
> +}
> +
> +/*
> + * We rely on a gross hack in section_rel[a]() calling find_extable_entry_size()
> + * to know the sizeof(struct exception_table_entry) for the target architecture.
> + */
> +static unsigned int extable_entry_size = 0;
> +static void find_extable_entry_size(const char* const sec, const Elf_Rela* r,
> +				    const void* start, const void* cur)
> +{
> +	/*
> +	 * If we're currently checking the second relocation within __ex_table,
> +	 * that relocation offset tells us the offsetof(struct
> +	 * exception_table_entry, fixup) which is equal to sizeof(struct
> +	 * exception_table_entry) divided by two.  We use that to our advantage
> +	 * since there's no portable way to get that size as every architecture
> +	 * seems to go with different sized types.  Not pretty but better than
> +	 * hard-coding the size for every architecture..
> +	 */
> +	if (!extable_entry_size && cur == start + 1 &&
> +	    strcmp("__ex_table", sec) == 0)
> +		extable_entry_size = r->r_offset * 2;
> +}
> +static inline bool is_extable_fault_address(Elf_Rela *r)
> +{
> +	if (!extable_entry_size == 0)
> +		fatal("extable_entry size hasn't been discovered!\n");
> +
> +	return ((r->r_offset == 0) ||
> +		(r->r_offset % extable_entry_size == 0));
> +}
> +
> +static void report_extable_warnings(const char* modname, struct elf_info* elf,
> +				    const struct sectioncheck* const mismatch,
> +				    Elf_Rela* r, Elf_Sym* sym,
> +				    const char* fromsec, const char* tosec)
> +{
> +	Elf_Sym* fromsym = find_elf_symbol2(elf, r->r_offset, fromsec);
> +	const char* fromsym_name = sym_name(elf, fromsym);
> +	Elf_Sym* tosym = find_elf_symbol(elf, r->r_addend, sym);
> +	const char* tosym_name = sym_name(elf, tosym);
> +	const char* from_pretty_name;
> +	const char* from_pretty_name_p;
> +	const char* to_pretty_name;
> +	const char* to_pretty_name_p;
> +
> +	get_pretty_name(is_function(fromsym),
> +			&from_pretty_name, &from_pretty_name_p);
> +	get_pretty_name(is_function(tosym),
> +			&to_pretty_name, &to_pretty_name_p);
> +
> +	warn("%s(%s+0x%lx): Section mismatch in reference"
> +	     " from the %s %s%s to the %s %s:%s%s\n",
> +	     modname, fromsec, r->r_offset, from_pretty_name,
> +	     fromsym_name, from_pretty_name_p,
> +	     to_pretty_name, tosec, tosym_name, to_pretty_name_p);
> +
> +	if (!match(tosec, mismatch->bad_tosec) &&
> +	    is_executable_section(elf, get_secindex(elf, sym)))
> +		fprintf(stderr,
> +			"The relocation at %s+0x%lx references\n"
> +			"section \"%s\" which is not in the list of\n"
> +			"authorized sections.  If you're adding a new section\n"
> +			"and/or if this reference is valid, add \"%s\" to the\n"
> +			"list of authorized sections to jump to on fault.\n"
> +			"This can be achieved by adding \"%s\" to \n"
> +			"OTHER_TEXT_SECTIONS in scripts/mod/modpost.c.\n",
> +			fromsec, r->r_offset, tosec, tosec, tosec);
> +}
> +
> +static void extable_mismatch_handler(const char* modname, struct elf_info *elf,
> +				     const struct sectioncheck* const mismatch,
> +				     Elf_Rela* r, Elf_Sym* sym,
> +				     const char *fromsec)
> +{
> +	const char* tosec = sec_name(elf, get_secindex(elf, sym));
> +
> +	sec_mismatch_count++;
> +
> +	if (sec_mismatch_verbose)
> +		report_extable_warnings(modname, elf, mismatch, r, sym,
> +					fromsec, tosec);
> +
> +	if (match(tosec, mismatch->bad_tosec))
> +		fatal("The relocation at %s+0x%lx references\n"
> +		      "section \"%s\" which is black-listed.\n"
> +		      "Something is seriously wrong and should be fixed.\n"
> +		      "You might get more information about where this is\n"
> +		      "coming from by using scripts/check_extable.sh %s\n",
> +		      fromsec, r->r_offset, tosec, modname);
> +	else if (!is_executable_section(elf, get_secindex(elf, sym))) {
> +		if (is_extable_fault_address(r))
> +			fatal("The relocation at %s+0x%lx references\n"
> +			      "section \"%s\" which is not executable, IOW\n"
> +			      "it is not possible for the kernel to fault\n"
> +			      "at that address.  Something is seriously wrong\n"
> +			      "and should be fixed.\n",
> +			      fromsec, r->r_offset, tosec);
> +		else
> +			fatal("The relocation at %s+0x%lx references\n"
> +			      "section \"%s\" which is not executable, IOW\n"
> +			      "the kernel will fault if it ever tries to\n"
> +			      "jump to it.  Something is seriously wrong\n"
> +			      "and should be fixed.\n",
> +			      fromsec, r->r_offset, tosec);
> +	}
> +}
> +
>  static void check_section_mismatch(const char *modname, struct elf_info *elf,
>  				   Elf_Rela *r, Elf_Sym *sym, const char *fromsec)
>  {
> @@ -1605,6 +1744,7 @@ static void section_rela(const char *modname, struct elf_info *elf,
>  		/* Skip special sections */
>  		if (is_shndx_special(sym->st_shndx))
>  			continue;
> +		find_extable_entry_size(fromsec, &r, start, rela);
>  		check_section_mismatch(modname, elf, &r, sym, fromsec);
>  	}
>  }
> @@ -1663,6 +1803,7 @@ static void section_rel(const char *modname, struct elf_info *elf,
>  		/* Skip special sections */
>  		if (is_shndx_special(sym->st_shndx))
>  			continue;
> +		find_extable_entry_size(fromsec, &r, start, rel);
>  		check_section_mismatch(modname, elf, &r, sym, fromsec);
>  	}
>  }
> -- 
> 2.0.5
> 

  reply	other threads:[~2015-03-18  9:08 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-17 12:39 [PATCH 0/7] Detect future mis-uses of __ex_table section Quentin Casasnovas
2015-03-17 12:39 ` [PATCH 1/7] modpost: add strict white-listing when referencing sections Quentin Casasnovas
2015-03-17 16:25   ` Linus Torvalds
2015-03-18  9:14     ` Quentin Casasnovas
2015-03-20  1:29   ` Rusty Russell
2015-04-13  9:04     ` Quentin Casasnovas
2015-04-13 11:19       ` Rusty Russell
2015-04-13 11:24       ` Rusty Russell
2015-03-17 12:39 ` [PATCH 2/7] modpost: add .sched.text and .kprobes.text to the TEXT_SECTIONS list Quentin Casasnovas
2015-03-18  9:08   ` Quentin Casasnovas
2015-03-17 12:39 ` [PATCH 3/7] modpost: add handler function pointer to sectioncheck Quentin Casasnovas
2015-03-18  9:08   ` Quentin Casasnovas
2015-03-17 12:39 ` [PATCH 4/7] modpost: factorize symbol pretty print in get_pretty_name() Quentin Casasnovas
2015-03-18  9:08   ` Quentin Casasnovas
2015-03-17 12:40 ` [PATCH 5/7] modpost: mismatch_handler: retrieve tosym information only when needed Quentin Casasnovas
2015-03-18  9:09   ` Quentin Casasnovas
2015-03-17 12:40 ` [PATCH 6/7] scripts: add check_extable.sh script Quentin Casasnovas
2015-03-18  9:09   ` Quentin Casasnovas
2015-03-17 12:40 ` [PATCH 7/7] modpost: handle relocations mismatch in __ex_table Quentin Casasnovas
2015-03-18  9:09   ` Quentin Casasnovas [this message]
2015-04-13 11:18   ` Rusty Russell
2015-04-13 13:33     ` Quentin Casasnovas
2015-04-14 12:14   ` Thierry Reding
2015-04-14 12:35     ` Quentin Casasnovas
2015-04-15  3:27       ` Rusty Russell
2015-04-15  8:35         ` Quentin Casasnovas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150318090948.GL19131@chrystal.uk.oracle.com \
    --to=quentin.casasnovas@oracle.com \
    --cc=bp@alien8.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mmarek@suse.cz \
    --cc=oleg@redhat.com \
    --cc=rusty@rustcorp.com.au \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome