public inbox for linux-crypto@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] crypto: qat - Return pointer directly in adf_ctl_alloc_resources
@ 2025-09-13  4:24 Herbert Xu
  2025-09-13 10:09 ` Thorsten Blum
  2025-09-17 11:56 ` Giovanni Cabiddu
  0 siblings, 2 replies; 4+ messages in thread
From: Herbert Xu @ 2025-09-13  4:24 UTC (permalink / raw)
  To: Linux Crypto Mailing List, Thorsten Blum; +Cc: Giovanni Cabiddu, qat-linux

Returning values through arguments is confusing and that has
upset the compiler with the recent change to memdup_user:

../drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c: In function ‘adf_ctl_ioctl’:
../drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c:308:26: warning: ‘ctl_data’ may be used uninitialized [-Wmaybe-uninitialized]
  308 |                  ctl_data->device_id);
      |                          ^~
../drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c:294:39: note: ‘ctl_data’ was declared here
  294 |         struct adf_user_cfg_ctl_data *ctl_data;
      |                                       ^~~~~~~~
In function ‘adf_ctl_ioctl_dev_stop’,
    inlined from ‘adf_ctl_ioctl’ at ../drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c:386:9:
../drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c:273:48: warning: ‘ctl_data’ may be used uninitialized [-Wmaybe-uninitialized]
  273 |         ret = adf_ctl_is_device_in_use(ctl_data->device_id);
      |                                        ~~~~~~~~^~~~~~~~~~~
../drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c: In function ‘adf_ctl_ioctl’:
../drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c:261:39: note: ‘ctl_data’ was declared here
  261 |         struct adf_user_cfg_ctl_data *ctl_data;
      |                                       ^~~~~~~~
In function ‘adf_ctl_ioctl_dev_config’,
    inlined from ‘adf_ctl_ioctl’ at ../drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c:382:9:
../drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c:192:54: warning: ‘ctl_data’ may be used uninitialized [-Wmaybe-uninitialized]
  192 |         accel_dev = adf_devmgr_get_dev_by_id(ctl_data->device_id);
      |                                              ~~~~~~~~^~~~~~~~~~~
../drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c: In function ‘adf_ctl_ioctl’:
../drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c:185:39: note: ‘ctl_data’ was declared here
  185 |         struct adf_user_cfg_ctl_data *ctl_data;
      |                                       ^~~~~~~~

Fix this by returning the pointer directly.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c b/drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c
index 84bc89e16742..c2e6f0cb7480 100644
--- a/drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c
+++ b/drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c
@@ -89,19 +89,14 @@ static int adf_chr_drv_create(void)
 	return -EFAULT;
 }
 
-static int adf_ctl_alloc_resources(struct adf_user_cfg_ctl_data **ctl_data,
-				   unsigned long arg)
+static struct adf_user_cfg_ctl_data *adf_ctl_alloc_resources(unsigned long arg)
 {
 	struct adf_user_cfg_ctl_data *cfg_data;
 
 	cfg_data = memdup_user((void __user *)arg, sizeof(*cfg_data));
-	if (IS_ERR(cfg_data)) {
+	if (IS_ERR(cfg_data))
 		pr_err("QAT: failed to copy from user cfg_data.\n");
-		return PTR_ERR(cfg_data);
-	}
-
-	*ctl_data = cfg_data;
-	return 0;
+	return cfg_data;
 }
 
 static int adf_add_key_value_data(struct adf_accel_dev *accel_dev,
@@ -181,13 +176,13 @@ static int adf_copy_key_value_data(struct adf_accel_dev *accel_dev,
 static int adf_ctl_ioctl_dev_config(struct file *fp, unsigned int cmd,
 				    unsigned long arg)
 {
-	int ret;
 	struct adf_user_cfg_ctl_data *ctl_data;
 	struct adf_accel_dev *accel_dev;
+	int ret = 0;
 
-	ret = adf_ctl_alloc_resources(&ctl_data, arg);
-	if (ret)
-		return ret;
+	ctl_data = adf_ctl_alloc_resources(arg);
+	if (IS_ERR(ctl_data))
+		return PTR_ERR(ctl_data);
 
 	accel_dev = adf_devmgr_get_dev_by_id(ctl_data->device_id);
 	if (!accel_dev) {
@@ -260,9 +255,9 @@ static int adf_ctl_ioctl_dev_stop(struct file *fp, unsigned int cmd,
 	int ret;
 	struct adf_user_cfg_ctl_data *ctl_data;
 
-	ret = adf_ctl_alloc_resources(&ctl_data, arg);
-	if (ret)
-		return ret;
+	ctl_data = adf_ctl_alloc_resources(arg);
+	if (IS_ERR(ctl_data))
+		return PTR_ERR(ctl_data);
 
 	if (adf_devmgr_verify_id(ctl_data->device_id)) {
 		pr_err("QAT: Device %d not found\n", ctl_data->device_id);
@@ -294,9 +289,9 @@ static int adf_ctl_ioctl_dev_start(struct file *fp, unsigned int cmd,
 	struct adf_user_cfg_ctl_data *ctl_data;
 	struct adf_accel_dev *accel_dev;
 
-	ret = adf_ctl_alloc_resources(&ctl_data, arg);
-	if (ret)
-		return ret;
+	ctl_data = adf_ctl_alloc_resources(arg);
+	if (IS_ERR(ctl_data))
+		return PTR_ERR(ctl_data);
 
 	ret = -ENODEV;
 	accel_dev = adf_devmgr_get_dev_by_id(ctl_data->device_id);
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] crypto: qat - Return pointer directly in adf_ctl_alloc_resources
  2025-09-13  4:24 [PATCH] crypto: qat - Return pointer directly in adf_ctl_alloc_resources Herbert Xu
@ 2025-09-13 10:09 ` Thorsten Blum
  2025-09-17 11:56 ` Giovanni Cabiddu
  1 sibling, 0 replies; 4+ messages in thread
From: Thorsten Blum @ 2025-09-13 10:09 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Linux Crypto Mailing List, Giovanni Cabiddu, qat-linux

Hi Herbert,

On 13. Sep 2025 Herbert Xu wrote:
> Returning values through arguments is confusing and that has
> upset the compiler with the recent change to memdup_user:
> ...
> Fix this by returning the pointer directly.

I didn't notice the warnings, but thanks for fixing it.

Reviewed-by: Thorsten Blum <thorsten.blum@linux.dev>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] crypto: qat - Return pointer directly in adf_ctl_alloc_resources
  2025-09-13  4:24 [PATCH] crypto: qat - Return pointer directly in adf_ctl_alloc_resources Herbert Xu
  2025-09-13 10:09 ` Thorsten Blum
@ 2025-09-17 11:56 ` Giovanni Cabiddu
  2025-09-18  2:27   ` Herbert Xu
  1 sibling, 1 reply; 4+ messages in thread
From: Giovanni Cabiddu @ 2025-09-17 11:56 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Linux Crypto Mailing List, Thorsten Blum, qat-linux

On Sat, Sep 13, 2025 at 12:24:55PM +0800, Herbert Xu wrote:
> Returning values through arguments is confusing and that has
> upset the compiler with the recent change to memdup_user:
> 
> ../drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c: In function ‘adf_ctl_ioctl’:
> ../drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c:308:26: warning: ‘ctl_data’ may be used uninitialized [-Wmaybe-uninitialized]
>   308 |                  ctl_data->device_id);
>       |                          ^~
> ../drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c:294:39: note: ‘ctl_data’ was declared here
>   294 |         struct adf_user_cfg_ctl_data *ctl_data;
>       |                                       ^~~~~~~~
> In function ‘adf_ctl_ioctl_dev_stop’,
>     inlined from ‘adf_ctl_ioctl’ at ../drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c:386:9:
> ../drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c:273:48: warning: ‘ctl_data’ may be used uninitialized [-Wmaybe-uninitialized]
>   273 |         ret = adf_ctl_is_device_in_use(ctl_data->device_id);
>       |                                        ~~~~~~~~^~~~~~~~~~~
> ../drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c: In function ‘adf_ctl_ioctl’:
> ../drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c:261:39: note: ‘ctl_data’ was declared here
>   261 |         struct adf_user_cfg_ctl_data *ctl_data;
>       |                                       ^~~~~~~~
> In function ‘adf_ctl_ioctl_dev_config’,
>     inlined from ‘adf_ctl_ioctl’ at ../drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c:382:9:
> ../drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c:192:54: warning: ‘ctl_data’ may be used uninitialized [-Wmaybe-uninitialized]
>   192 |         accel_dev = adf_devmgr_get_dev_by_id(ctl_data->device_id);
>       |                                              ~~~~~~~~^~~~~~~~~~~
> ../drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c: In function ‘adf_ctl_ioctl’:
> ../drivers/crypto/intel/qat/qat_common/adf_ctl_drv.c:185:39: note: ‘ctl_data’ was declared here
>   185 |         struct adf_user_cfg_ctl_data *ctl_data;
>       |                                       ^~~~~~~~
I don't see this warning with W=1 on gcc 15.2.1,
    make M=drivers/crypto/intel/qat W=1 -j
What version is reporting this warning?

> 
> Fix this by returning the pointer directly.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Acked-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>

Regards,

-- 
Giovanni

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] crypto: qat - Return pointer directly in adf_ctl_alloc_resources
  2025-09-17 11:56 ` Giovanni Cabiddu
@ 2025-09-18  2:27   ` Herbert Xu
  0 siblings, 0 replies; 4+ messages in thread
From: Herbert Xu @ 2025-09-18  2:27 UTC (permalink / raw)
  To: Giovanni Cabiddu; +Cc: Linux Crypto Mailing List, Thorsten Blum, qat-linux

On Wed, Sep 17, 2025 at 12:56:41PM +0100, Giovanni Cabiddu wrote:
>
> I don't see this warning with W=1 on gcc 15.2.1,
>     make M=drivers/crypto/intel/qat W=1 -j
> What version is reporting this warning?

It's not visible with W=1 because uninitialized warnings are
disabled by default.  I saw them with

make KBUILD_CFLAGS_KERNEL=-Wmaybe-uninitialized CFLAGS_MODULE=-Wmaybe-uninitialized

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-09-18  2:27 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-13  4:24 [PATCH] crypto: qat - Return pointer directly in adf_ctl_alloc_resources Herbert Xu
2025-09-13 10:09 ` Thorsten Blum
2025-09-17 11:56 ` Giovanni Cabiddu
2025-09-18  2:27   ` Herbert Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox