[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v4 07/16] livepatch/arm/x86: Check payload for for unwelcomed symbols.
Hi Konrad,
On 19/09/2016 19:32, Konrad Rzeszutek Wilk wrote:
On Mon, Sep 19, 2016 at 08:48:10AM -0600, Jan Beulich wrote:
On 19.09.16 at 16:13, <julien.grall@xxxxxxx> wrote:
On 19/09/2016 16:11, Jan Beulich wrote:
On 19.09.16 at 15:33, <julien.grall@xxxxxxx> wrote:
On 19/09/2016 11:27, Jan Beulich wrote:
On 16.09.16 at 18:38, <konrad.wilk@xxxxxxxxxx> wrote:
--- a/xen/arch/arm/livepatch.c
+++ b/xen/arch/arm/livepatch.c
@@ -117,6 +117,20 @@ bool arch_livepatch_symbol_ok(const struct livepatch_elf
*elf,
return true;
}
+bool arch_livepatch_symbol_deny(const struct livepatch_elf *elf,
+ const struct livepatch_elf_sym *sym)
+{
+#ifdef CONFIG_ARM_32
+ /*
+ * Xen does not use Thumb instructions - and we should not see any of
+ * them. If we do, abort.
+ */
+ if ( sym->name && *sym->name == '$' && sym->name[1] == 't' )
Please use sym->name[0] for readability. Also, you may want to check the
length of the symbol before checking the second character.
Why would the length check be needed? If the first character is $,
then looking at the second one is always valid (and it being nul will
be properly dealt with by the expression above).
Because you may have a payload which is not valid? Or maybe you consider
that all the payload are trusted.
If all symbols' names are inside their string tables and the string
tables are both contained inside the image and have a zero byte
at their end (all of which gets verified afair), nothing bad can
happen I think.
Exactly. All of those checks are done so we are sure that the
sym->name[0] points to something.
Hmmm right, sorry for the confusion.
bool arch_livepatch_symbol_deny(const struct livepatch_elf *elf,
const struct livepatch_elf_sym *sym)
{
#ifdef CONFIG_ARM_32
/*
* Xen does not use Thumb instructions - and we should not see any
* of
* them. If we do, abort.
*/
if ( sym->name && sym->name[0] == '$' && sym->name[1] == 't' )
{
if ( sym->name[2] && sym->name[2] != '.' )
return false;
return true;
}
#endif
return false;
}
The second one is fine by me with a small change:
if ( sym->name[2] && sym->name[2] != '.' )
return false;
return true;
Could be replaced by:
return ( !sym->name[2] || sym->name[2] == '.' );
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
