Re: [PATCH 1/2] crypto: ccp - Reduce stack frame size with KASAN

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/28/2017 04:58 AM, Arnd Bergmann wrote:> The newly added AES GCM implementation uses one of the largest stack frames
in the kernel, around 1KB on normal 64-bit kernels, and 1.6KB when
CONFIG_KASAN
is enabled:

drivers/crypto/ccp/ccp-ops.c: In function 'ccp_run_aes_gcm_cmd':
drivers/crypto/ccp/ccp-ops.c:851:1: error: the frame size of 1632 bytes
is larger than 1536 bytes [-Werror=frame-larger-than=]

This is problematic for multiple reasons:

 - The crypto functions are often used in deep call chains, e.g. behind
   mm, fs and dm layers, making it more likely to run into an actual stack
   overflow

 - Using this much stack space is an indicator that the code is not
   written to be as efficient as it could be.

I'm not sure I agree that A -> B, but I will certainly look into this.

 - While this goes unnoticed at the moment in mainline with the frame size
   warning being disabled when KASAN is in use, I would like to enable
   the warning again, and the current code is slightly above my arbitrary
   pick for a limit of 1536 bytes (I already did patches for every other
   driver exceeding this).

I've got my stack frame size (also) set to 1536, and would have paid more attention
had a warning occurred due to my code.

A more drastic refactoring of the driver might be needed to reduce the
stack usage more substantially, but this patch is fairly simple and
at least addresses the third one of the problems I mentioned, reducing the
stack size by about 150 bytes and bringing it below the warning limit
I picked.

Again, I'll devote some time to this.

diff --git a/drivers/crypto/ccp/ccp-dev.h b/drivers/crypto/ccp/ccp-dev.h
index 3a45c2af2fbd..c5ea0796a891 100644
--- a/drivers/crypto/ccp/ccp-dev.h
+++ b/drivers/crypto/ccp/ccp-dev.h
@@ -432,24 +432,24 @@ struct ccp_dma_info {
         unsigned int offset;
         unsigned int length;
         enum dma_data_direction dir;
-};
+} __packed __aligned(4);

My gcc 4.8 doesn't understand __aligned(). Shouldn't we use
#pragma(4) here?


 struct ccp_dm_workarea {
         struct device *dev;
         struct dma_pool *dma_pool;
-       unsigned int length;

         u8 *address;
         struct ccp_dma_info dma;
+       unsigned int length;
 };

 struct ccp_sg_workarea {
         struct scatterlist *sg;
         int nents;
+       unsigned int dma_count;

         struct scatterlist *dma_sg;
         struct device *dma_dev;
-       unsigned int dma_count;
         enum dma_data_direction dma_dir;

         unsigned int sg_used;

I'm okay with rearranging, but I'm going to submit an alternative patch.




[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]

  Powered by Linux