On Wed, 2019-06-05 at 11:51 -0700, Jerry Snitselaar wrote:
> The trousers is failing annocheck hardened check due to
> __no_optimize being used for __tspi_memset(). Instead of
> __no_optimize use a asm memory barrier.
> 
> Signed-off-by: Jerry Snitselaar <[email protected]>
> ---
>  src/include/spi_utils.h    | 2 +-
>  src/tspi/tsp_context_mem.c | 6 ++++--
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/src/include/spi_utils.h b/src/include/spi_utils.h
> index 11255b2..6ef21ce 100644
> --- a/src/include/spi_utils.h
> +++ b/src/include/spi_utils.h
> @@ -53,7 +53,7 @@ MUTEX_DECLARE_EXTERN(mem_cache_lock);
>  void *calloc_tspi(TSS_HCONTEXT, UINT32);
>  TSS_RESULT free_tspi(TSS_HCONTEXT, void *);
>  TSS_RESULT __tspi_add_mem_entry(TSS_HCONTEXT, void *);
> -void * __no_optimize __tspi_memset(void *, int, size_t);
> +void * __tspi_memset(void *, int, size_t);
>  
>  /* secrets.c */
>  
> diff --git a/src/tspi/tsp_context_mem.c b/src/tspi/tsp_context_mem.c
> index 2982df9..2769af3 100644
> --- a/src/tspi/tsp_context_mem.c
> +++ b/src/tspi/tsp_context_mem.c
> @@ -258,8 +258,10 @@ free_tspi(TSS_HCONTEXT tspContext, void
> *memPointer)
>  }
>  
>  /* definition for a memset that cannot be optimized away */
> -void * __no_optimize
> +void *
>  __tspi_memset(void *s, int c, size_t n)
>  {
> -     return memset(s, c, n);
> +     memset(s, c, n);
> +     asm volatile("" ::: "memory");
> +     return s;
>  }

Thank you for the patch, Jerry.  

I've been reading a bit about this one.  Please anyone feel free to
correct my understanding. 

Reading this: 
https://stackoverflow.com/questions/14950614/working-of-asm-volatile-memory
It sounds like using
+       asm volatile("" ::: "memory");
helps prevent the compiler from re-ordering instructions.  Operations
before the statement are guaranteed to be carried out prior to the ones
after the statement. 

Then I noticed that the comment above the original __no_optimize
states: 
>  /* definition for a memset that cannot be optimized away */
this made me worry that perhaps the _no_optimize is preventing more
types of optimizations than just re-ordering.  But after reading this
post: 
https://stackoverflow.com/questions/3604569/what-kinds-of-optimizations-does-volatile-prevent-in-c
and seeing what is actually in the _no_optimize functions, I am
convincing myself that this patch should be okay, assuming testing goes
well.

One remaining question is, have you tested this patch?  If so, did you
test on x86_64, ppc, s390, etc?

Thanks,
Debbie




_______________________________________________
TrouSerS-tech mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/trousers-tech

Reply via email to