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.
