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 now the version which does the bitmap handling dynamic with kmalloc/kfree: ======================================== From: Harald Freudenberger <freude@xxxxxxxxxxxxx> Date: Thu, 6 Sep 2018 11:08:43 +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. Removed the use of VLAs on the stack. Now the code dynamically allocates and frees these arrays with kmalloc/kfree. Signed-off-by: Harald Freudenberger <freude@xxxxxxxxxxxxx> --- drivers/s390/crypto/ap_bus.c | 48 ++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/drivers/s390/crypto/ap_bus.c b/drivers/s390/crypto/ap_bus.c index ec891bc7d10a..308461e5b965 100644 --- a/drivers/s390/crypto/ap_bus.c +++ b/drivers/s390/crypto/ap_bus.c @@ -970,21 +970,26 @@ static int process_mask_arg(const char *str, unsigned long *bitmap, int bits, struct mutex *lock) { - int i; + int i, rc = 0; + unsigned long *clrm, *setm, *m = NULL; /* bits needs to be a multiple of 8 */ if (bits & 0x07) return -EINVAL; if (*str == '+' || *str == '-') { - DECLARE_BITMAP(clrm, bits); - DECLARE_BITMAP(setm, bits); - - i = str2clrsetmasks(str, clrm, setm, bits); - if (i) - return i; - if (mutex_lock_interruptible(lock)) - return -ERESTARTSYS; + m = kmalloc(2 * BITS_TO_LONGS(bits) * sizeof(long), GFP_KERNEL); + if (!m) + return -ENOMEM; + clrm = &m[0]; + setm = &m[BITS_TO_LONGS(bits)]; + rc = str2clrsetmasks(str, clrm, setm, bits); + if (rc) + goto out; + if (mutex_lock_interruptible(lock)) { + rc = -ERESTARTSYS; + goto out; + } for (i = 0; i < bits; i++) { if (test_bit_inv(i, clrm)) clear_bit_inv(i, bitmap); @@ -992,22 +997,27 @@ static int process_mask_arg(const char *str, set_bit_inv(i, bitmap); } } else { - DECLARE_BITMAP(setm, bits); - - i = hex2bitmap(str, setm, bits); - if (i) - return i; - if (mutex_lock_interruptible(lock)) - return -ERESTARTSYS; + m = kmalloc(BITS_TO_LONGS(bits) * sizeof(long), GFP_KERNEL); + if (!m) + return -ENOMEM; + rc = hex2bitmap(str, m, bits); + if (rc) + goto out; + if (mutex_lock_interruptible(lock)) { + rc = -ERESTARTSYS; + goto out; + } for (i = 0; i < bits; i++) - if (test_bit_inv(i, setm)) + if (test_bit_inv(i, m)) set_bit_inv(i, bitmap); else clear_bit_inv(i, bitmap); } mutex_unlock(lock); - - return 0; +out: + if (m) + kfree(m); + return rc; } /* -- 2.17.1
