On 09/05/14 17:18, Thomas Gleixner wrote:
On Fri, 5 Sep 2014, beh...@converseincode.com wrote:
From: Behan Webster <beh...@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 <beh...@converseincode.com>
Signed-off-by: Mark Charlebois <charl...@gmail.com>
Signed-off-by: Jan-Simon Möller <dl...@gmx.de>
This SOB chain is completely ass backwards. See Documentation/...
"The Signed-off-by: tag indicates that the signer was involved in the
development of the patch, or that he/she was in the patch's delivery path."
All three of us were involved. Does that not satisfy this rule?
---
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.
Sadly this is a design pattern used in many places through out the
kernel, and appears to be fundamental to the crypto system. I was
advised *not* to change it, so we haven't.
I agree that it's not a good practice.
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);
Nothing is wrong with that. I would have actually preferred that. But it
would have fundamentally changed a lot more code.
I'm not sure making fundamental changes to the crypto code in order to
make "my favourite compiler happy" is really a better plan. I prefer
smaller more measured steps.
or something along that line and hide all the stack allocation, type
conversion crap and make my favourite compiler happy in a single
place?
At this point it is less about making a particular compiler happy, and
more about making the code at least C99 compliant which enables a
different compiler so that more people can use it to make more
fundamental changes like you are suggesting.
I appreciate your feedback,
Behan
--
Behan Webster
beh...@converseincode.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
