Re: [GIT PULL] s390 patches for the 4.19 merge window #2

[Harald Freudenberger]

On 05.09.2018 02:16, Kees Cook wrote:
> On Fri, Aug 24, 2018 at 12:42 AM, Martin Schwidefsky
> <schwidefsky@xxxxxxxxxx> wrote:
>> Harald Freudenberger (5):
>>       s390/zcrypt: hex string mask improvements for apmask and aqmask.
> This (and an earlier 2017 commit) adds VLAs, which are being
> removed[1] from the kernel:
>
> drivers/s390/crypto/ap_bus.c:980:3: warning: ISO C90 forbids variable
> length array âclrmâ [-Wvla]
> drivers/s390/crypto/ap_bus.c:981:3: warning: ISO C90 forbids variable
> length array âsetmâ [-Wvla]
> drivers/s390/crypto/ap_bus.c:995:3: warning: ISO C90 forbids variable
> length array âsetmâ [-Wvla]
>
> static int process_mask_arg(const char *str,
>                             unsigned long *bitmap, int bits,
>                             struct mutex *lock)
> ...
>                 DECLARE_BITMAP(clrm, bits);
>                 DECLARE_BITMAP(setm, bits);
>
> Can someone please adjust this to make these fixed size again?
>
> Thanks!
>
> -Kees
>
> [1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@xxxxxxxxxxxxxx
>
Here is a simple solution which just hard codes the upper limit to 256 bits.
As all callers are only internal to the ap bus code and do obey to this limit
this shouldn't be a real limitation.

However, a version which really allocates will follow and then I'll discuss
with Martin, which version to post upstream.

========================================

From: Harald Freudenberger <freude@xxxxxxxxxxxxx>
Date: Thu, 6 Sep 2018 11:29:36 +0200
Subject: [PATCH] s390/zcrypt: remove VLA use in ap bus code

The internal function to parse sysfs arguments uses VLAs -
dynamic bitmap arrays on the stack where the size is determined
by a invocation argument.

Reworked the code to have fixed sizes and check, if the caller
comply to this limit. However, one code path uses two of these
bitmap arrays and thus has a stack footprint of 64 bytes.

Signed-off-by: Harald Freudenberger <freude@xxxxxxxxxxxxx>
---
Âdrivers/s390/crypto/ap_bus.c | 10 +++++-----
Â1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/s390/crypto/ap_bus.c b/drivers/s390/crypto/ap_bus.c
index ec891bc7d10a..20f2fa2276c8 100644
--- a/drivers/s390/crypto/ap_bus.c
+++ b/drivers/s390/crypto/ap_bus.c
@@ -972,13 +972,13 @@ static int process_mask_arg(const char *str,
Â{
ÂÂÂÂ int i;
Â
-ÂÂÂ /* bits needs to be a multiple of 8 */
-ÂÂÂ if (bits & 0x07)
+ÂÂÂ /* bits needs to be a multiple of 8 and <= 256 */
+ÂÂÂ if (bits & 0x07 || bits > 256)
ÂÂÂÂ ÂÂÂ return -EINVAL;
Â
ÂÂÂÂ if (*str == '+' || *str == '-') {
-ÂÂÂ ÂÂÂ DECLARE_BITMAP(clrm, bits);
-ÂÂÂ ÂÂÂ DECLARE_BITMAP(setm, bits);
+ÂÂÂ ÂÂÂ DECLARE_BITMAP(clrm, 256);
+ÂÂÂ ÂÂÂ DECLARE_BITMAP(setm, 256);
Â
ÂÂÂÂ ÂÂÂ i = str2clrsetmasks(str, clrm, setm, bits);
ÂÂÂÂ ÂÂÂ if (i)
@@ -992,7 +992,7 @@ static int process_mask_arg(const char *str,
ÂÂÂÂ ÂÂÂ ÂÂÂ ÂÂÂ set_bit_inv(i, bitmap);
ÂÂÂÂ ÂÂÂ }
ÂÂÂÂ } else {
-ÂÂÂ ÂÂÂ DECLARE_BITMAP(setm, bits);
+ÂÂÂ ÂÂÂ DECLARE_BITMAP(setm, 256);
Â
ÂÂÂÂ ÂÂÂ i = hex2bitmap(str, setm, bits);
ÂÂÂÂ ÂÂÂ if (i)
-- 
2.17.1