All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.