lkml.org 
[lkml]   [2014]   [Sep]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] security, crypto: LLVMLinux: Remove VLAIS from ima_crypto.c
On Fri, 5 Sep 2014, behanw@converseincode.com wrote:

> From: Behan Webster <behanw@converseincode.com>
>
> Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99
> compliant equivalent. This patch allocates the appropriate amount of memory
> using an char array.
>
> The new code can be compiled with both gcc and clang.
>
> struct shash_desc contains a flexible array member member ctx declared with
> CRYPTO_MINALIGN_ATTR, so sizeof(struct shash_desc) aligns the beginning
> of the array declared after struct shash_desc with long long.
>
> No trailing padding is required because it is not a struct type that can
> be used in an array.
>
> The CRYPTO_MINALIGN_ATTR is required so that desc is aligned with long long
> as would be the case for a struct containing a member with
> CRYPTO_MINALIGN_ATTR.
>
> Signed-off-by: Behan Webster <behanw@converseincode.com>
> Signed-off-by: Mark Charlebois <charlebm@gmail.com>
> Signed-off-by: Jan-Simon Möller <dl9pf@gmx.de>

This SOB chain is completely ass backwards. See Documentation/...

> ---
> security/integrity/ima/ima_crypto.c | 53 +++++++++++++++++--------------------
> 1 file changed, 25 insertions(+), 28 deletions(-)
>
> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> index 0bd7328..a6aa2b0 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -380,17 +380,16 @@ static int ima_calc_file_hash_tfm(struct file *file,
> loff_t i_size, offset = 0;
> char *rbuf;
> int rc, read = 0;
> - struct {
> - struct shash_desc shash;
> - char ctx[crypto_shash_descsize(tfm)];
> - } desc;
> + char desc[sizeof(struct shash_desc) +
> + crypto_shash_descsize(tfm)] CRYPTO_MINALIGN_ATTR;
> + struct shash_desc *shash = (struct shash_desc *)desc;

That anon struct should have never happened in the first place. Not
your problem, but you are not making it any better. You replace open
coded crap with even more unreadable crap.

Whats wrong with

SHASH_DESC_ON_STACK(shash, tfm);

or something along that line and hide all the stack allocation, type
conversion crap and make my favourite compiler happy in a single
place?

Nothing wrong as far as I can tell, it just would add the benefit that
you can halfways ensure that nobody fatfingers the magic allocation
and conversion conventions.

Brainlessly replacing crap by more crap just to make it compile with
your favourite compiler is obviously convers to the goals of proper
written code, but considering your mail domain ....

Thanks,

tglx
\
 
 \ /
  Last update: 2014-09-06 02:41    [W:0.070 / U:0.244 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site