[PATCH] s390/pkey: prevent overflow in size calculation for memdup_user()

[PATCH] s390/pkey: prevent overflow in size calculation for memdup_user()

[Fedor Pchelkin]

Number of apqn target list entries contained in 'nr_apqns' variable is
determined by userspace via an ioctl call so the result of the product in
calculation of size passed to memdup_user() may overflow.

In this case the actual size of the allocated area and the value
describing it won't be in sync leading to various types of unpredictable
behaviour later.

Return an error if an overflow is detected. Note that it is different
from when nr_apqns is zero - that case is considered valid and should be
handled in subsequent pkey_handler implementations.

Found by Linux Verification Center (linuxtesting.org).

Fixes: f2bbc96e7cfa ("s390/pkey: add CCA AES cipher key support")
Cc: stable@vger.kernel.org
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
---
 drivers/s390/crypto/pkey_api.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/s390/crypto/pkey_api.c b/drivers/s390/crypto/pkey_api.c
index cef60770f68b..a731fc9c62a7 100644
--- a/drivers/s390/crypto/pkey_api.c
+++ b/drivers/s390/crypto/pkey_api.c
@@ -83,10 +83,15 @@ static void *_copy_key_from_user(void __user *ukey, size_t keylen)
 
 static void *_copy_apqns_from_user(void __user *uapqns, size_t nr_apqns)
 {
+	size_t size;
+
 	if (!uapqns || nr_apqns == 0)
 		return NULL;
 
-	return memdup_user(uapqns, nr_apqns * sizeof(struct pkey_apqn));
+	if (check_mul_overflow(nr_apqns, sizeof(struct pkey_apqn), &size))
+		return ERR_PTR(-EINVAL);
+
+	return memdup_user(uapqns, size);
 }
 
 static int pkey_ioctl_genseck(struct pkey_genseck __user *ugs)
-- 
2.49.0

Re: [PATCH] s390/pkey: prevent overflow in size calculation for memdup_user()

[Heiko Carstens]

On Wed, Jun 11, 2025 at 08:21:15PM +0300, Fedor Pchelkin wrote:
> Number of apqn target list entries contained in 'nr_apqns' variable is
> determined by userspace via an ioctl call so the result of the product in
> calculation of size passed to memdup_user() may overflow.
> 
> In this case the actual size of the allocated area and the value
> describing it won't be in sync leading to various types of unpredictable
> behaviour later.
> 
> Return an error if an overflow is detected. Note that it is different
> from when nr_apqns is zero - that case is considered valid and should be
> handled in subsequent pkey_handler implementations.
> 
> Found by Linux Verification Center (linuxtesting.org).
> 
> Fixes: f2bbc96e7cfa ("s390/pkey: add CCA AES cipher key support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
> ---
>  drivers/s390/crypto/pkey_api.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/s390/crypto/pkey_api.c b/drivers/s390/crypto/pkey_api.c
> index cef60770f68b..a731fc9c62a7 100644
> --- a/drivers/s390/crypto/pkey_api.c
> +++ b/drivers/s390/crypto/pkey_api.c
> @@ -83,10 +83,15 @@ static void *_copy_key_from_user(void __user *ukey, size_t keylen)
>  
>  static void *_copy_apqns_from_user(void __user *uapqns, size_t nr_apqns)
>  {
> +	size_t size;
> +
>  	if (!uapqns || nr_apqns == 0)
>  		return NULL;
>  
> -	return memdup_user(uapqns, nr_apqns * sizeof(struct pkey_apqn));
> +	if (check_mul_overflow(nr_apqns, sizeof(struct pkey_apqn), &size))
> +		return ERR_PTR(-EINVAL);
> +
> +	return memdup_user(uapqns, size);

Thanks! Is there any specific reason why this is open-coding
memdup_array_user()?

If not, please send a new version which does the simple conversion.