[PATCH] crypto: lib/mpi - Extend support for scope-based resource management

[PATCH] crypto: lib/mpi - Extend support for scope-based resource management

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 17 Sep 2024 11:20:29 +0200

Scope-based resource management became supported for some
programming interfaces by contributions of Peter Zijlstra on 2023-05-26.
See also the commit 54da6a0924311c7cf5015533991e44fb8eb12773 ("locking:
Introduce __cleanup() based infrastructure").

Thus add a macro call so that the attribute “__free(mpi_free)” can be
applied accordingly.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 include/linux/mpi.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/mpi.h b/include/linux/mpi.h
index 47be46f36435..47db8fa5fcc8 100644
--- a/include/linux/mpi.h
+++ b/include/linux/mpi.h
@@ -19,6 +19,8 @@

 #include <linux/types.h>
 #include <linux/scatterlist.h>
+#include <linux/cleanup.h>
+#include <linux/err.h>

 #define BYTES_PER_MPI_LIMB	(BITS_PER_LONG / 8)
 #define BITS_PER_MPI_LIMB	BITS_PER_LONG
@@ -44,6 +46,8 @@ typedef struct gcry_mpi *MPI;
 /*-- mpiutil.c --*/
 MPI mpi_alloc(unsigned nlimbs);
 void mpi_free(MPI a);
+DEFINE_FREE(mpi_free, MPI, if (!IS_ERR_OR_NULL(T_)) mpi_free(T_))
+
 int mpi_resize(MPI a, unsigned nlimbs);

 MPI mpi_copy(MPI a);
--
2.46.0

Re: [PATCH] crypto: lib/mpi - Extend support for scope-based resource management

[kernel test robot]

Hi Markus,

kernel test robot noticed the following build errors:

[auto build test ERROR on herbert-crypto-2.6/master]
[also build test ERROR on herbert-cryptodev-2.6/master linus/master v6.11 next-20240917]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Markus-Elfring/crypto-lib-mpi-Extend-support-for-scope-based-resource-management/20240917-173519
base:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git master
patch link:    https://lore.kernel.org/r/bc5ce9ad-acbd-4f3b-91d6-10cf62bf5afc%40web.de
patch subject: [PATCH] crypto: lib/mpi - Extend support for scope-based resource management
config: i386-buildonly-randconfig-002-20240918 (https://download.01.org/0day-ci/archive/20240918/202409180725.ZV8DCvII-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240918/202409180725.ZV8DCvII-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409180725.ZV8DCvII-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/irqflags.h:17,
                    from arch/x86/include/asm/special_insns.h:10,
                    from arch/x86/include/asm/processor.h:25,
                    from arch/x86/include/asm/timex.h:5,
                    from include/linux/timex.h:67,
                    from include/linux/time32.h:13,
                    from include/linux/time.h:60,
                    from include/linux/stat.h:19,
                    from include/linux/module.h:13,
                    from crypto/dh.c:9:
   include/linux/mpi.h: In function '__free_mpi_free':
>> include/linux/mpi.h:49:48: error: 'T_' undeclared (first use in this function)
      49 | DEFINE_FREE(mpi_free, MPI, if (!IS_ERR_OR_NULL(T_)) mpi_free(T_))
         |                                                ^~
   include/linux/cleanup.h:62:78: note: in definition of macro 'DEFINE_FREE'
      62 |         static inline void __free_##_name(void *p) { _type _T = *(_type *)p; _free; }
         |                                                                              ^~~~~
   include/linux/mpi.h:49:48: note: each undeclared identifier is reported only once for each function it appears in
      49 | DEFINE_FREE(mpi_free, MPI, if (!IS_ERR_OR_NULL(T_)) mpi_free(T_))
         |                                                ^~
   include/linux/cleanup.h:62:78: note: in definition of macro 'DEFINE_FREE'
      62 |         static inline void __free_##_name(void *p) { _type _T = *(_type *)p; _free; }
         |                                                                              ^~~~~
>> include/linux/cleanup.h:62:60: warning: unused variable '_T' [-Wunused-variable]
      62 |         static inline void __free_##_name(void *p) { _type _T = *(_type *)p; _free; }
         |                                                            ^~
   include/linux/mpi.h:49:1: note: in expansion of macro 'DEFINE_FREE'
      49 | DEFINE_FREE(mpi_free, MPI, if (!IS_ERR_OR_NULL(T_)) mpi_free(T_))
         | ^~~~~~~~~~~


vim +/T_ +49 include/linux/mpi.h

    45	
    46	/*-- mpiutil.c --*/
    47	MPI mpi_alloc(unsigned nlimbs);
    48	void mpi_free(MPI a);
  > 49	DEFINE_FREE(mpi_free, MPI, if (!IS_ERR_OR_NULL(T_)) mpi_free(T_))
    50	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] crypto: lib/mpi - Extend support for scope-based resource management

[kernel test robot]

Hi Markus,

kernel test robot noticed the following build errors:

[auto build test ERROR on herbert-crypto-2.6/master]
[also build test ERROR on herbert-cryptodev-2.6/master linus/master v6.11 next-20240917]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Markus-Elfring/crypto-lib-mpi-Extend-support-for-scope-based-resource-management/20240917-173519
base:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git master
patch link:    https://lore.kernel.org/r/bc5ce9ad-acbd-4f3b-91d6-10cf62bf5afc%40web.de
patch subject: [PATCH] crypto: lib/mpi - Extend support for scope-based resource management
config: i386-buildonly-randconfig-001-20240918 (https://download.01.org/0day-ci/archive/20240918/202409180851.4taVWgfI-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240918/202409180851.4taVWgfI-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409180851.4taVWgfI-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from crypto/dh.c:14:
>> include/linux/mpi.h:49:48: error: use of undeclared identifier 'T_'
      49 | DEFINE_FREE(mpi_free, MPI, if (!IS_ERR_OR_NULL(T_)) mpi_free(T_))
         |                                                ^
   include/linux/mpi.h:49:62: error: use of undeclared identifier 'T_'
      49 | DEFINE_FREE(mpi_free, MPI, if (!IS_ERR_OR_NULL(T_)) mpi_free(T_))
         |                                                              ^
   2 errors generated.


vim +/T_ +49 include/linux/mpi.h

    45	
    46	/*-- mpiutil.c --*/
    47	MPI mpi_alloc(unsigned nlimbs);
    48	void mpi_free(MPI a);
  > 49	DEFINE_FREE(mpi_free, MPI, if (!IS_ERR_OR_NULL(T_)) mpi_free(T_))
    50	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v2] crypto: lib/mpi - Extend support for scope-based resource management

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 18 Sep 2024 11:06:35 +0200

Scope-based resource management became supported for some
programming interfaces by contributions of Peter Zijlstra on 2023-05-26.
See also the commit 54da6a0924311c7cf5015533991e44fb8eb12773 ("locking:
Introduce __cleanup() based infrastructure").

Thus add a macro call so that the attribute “__free(mpi_free)” can be
applied accordingly.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---

V2:
The kernel build service pointed out that the proposed identifier “T_”
was not recognised by the compiler.
Thus reserved identifiers need still be applied also at such a place instead
so far.
I became curious under which circumstances corresponding development concerns
will be reconsidered any more.


 include/linux/mpi.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/mpi.h b/include/linux/mpi.h
index 47be46f36435..6fbcb88ce296 100644
--- a/include/linux/mpi.h
+++ b/include/linux/mpi.h
@@ -19,6 +19,8 @@

 #include <linux/types.h>
 #include <linux/scatterlist.h>
+#include <linux/cleanup.h>
+#include <linux/err.h>

 #define BYTES_PER_MPI_LIMB	(BITS_PER_LONG / 8)
 #define BITS_PER_MPI_LIMB	BITS_PER_LONG
@@ -44,6 +46,8 @@ typedef struct gcry_mpi *MPI;
 /*-- mpiutil.c --*/
 MPI mpi_alloc(unsigned nlimbs);
 void mpi_free(MPI a);
+DEFINE_FREE(mpi_free, MPI, if (!IS_ERR_OR_NULL(_T)) mpi_free(_T))
+
 int mpi_resize(MPI a, unsigned nlimbs);

 MPI mpi_copy(MPI a);
--
2.46.0

Re: [PATCH v2] crypto: lib/mpi - Extend support for scope-based resource management

[Herbert Xu]

On Wed, Sep 18, 2024 at 11:26:23AM +0200, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 18 Sep 2024 11:06:35 +0200
> 
> Scope-based resource management became supported for some

This patch needs to be sent to linux-crypto.

Thanks,
-- 
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

[PATCH v2 RESEND] crypto: lib/mpi - Extend support for scope-based resource management

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 18 Sep 2024 11:06:35 +0200

Scope-based resource management became supported for some
programming interfaces by contributions of Peter Zijlstra on 2023-05-26.
See also the commit 54da6a0924311c7cf5015533991e44fb8eb12773 ("locking:
Introduce __cleanup() based infrastructure").

Thus add a macro call so that the attribute “__free(mpi_free)” can be
applied accordingly.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---

V2:
The kernel build service pointed out that the proposed identifier “T_”
was not recognised by the compiler.
Thus reserved identifiers need still be applied also at such a place instead
so far.
I became curious under which circumstances corresponding development concerns
will be reconsidered any more.


 include/linux/mpi.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/mpi.h b/include/linux/mpi.h
index 47be46f36435..6fbcb88ce296 100644
--- a/include/linux/mpi.h
+++ b/include/linux/mpi.h
@@ -19,6 +19,8 @@

 #include <linux/types.h>
 #include <linux/scatterlist.h>
+#include <linux/cleanup.h>
+#include <linux/err.h>

 #define BYTES_PER_MPI_LIMB	(BITS_PER_LONG / 8)
 #define BITS_PER_MPI_LIMB	BITS_PER_LONG
@@ -44,6 +46,8 @@ typedef struct gcry_mpi *MPI;
 /*-- mpiutil.c --*/
 MPI mpi_alloc(unsigned nlimbs);
 void mpi_free(MPI a);
+DEFINE_FREE(mpi_free, MPI, if (!IS_ERR_OR_NULL(_T)) mpi_free(_T))
+
 int mpi_resize(MPI a, unsigned nlimbs);

 MPI mpi_copy(MPI a);
--
2.46.0

Re: [PATCH v2 RESEND] crypto: lib/mpi - Extend support for scope-based resource management

[Herbert Xu]

On Sat, Oct 05, 2024 at 08:27:03AM +0200, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 18 Sep 2024 11:06:35 +0200
> 
> Scope-based resource management became supported for some
> programming interfaces by contributions of Peter Zijlstra on 2023-05-26.
> See also the commit 54da6a0924311c7cf5015533991e44fb8eb12773 ("locking:
> Introduce __cleanup() based infrastructure").
> 
> Thus add a macro call so that the attribute “__free(mpi_free)” can be
> applied accordingly.

So where are the users of this?

Thanks,
-- 
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

Re: [v2] crypto: lib/mpi - Extend support for scope-based resource management

[Markus Elfring]

>> Scope-based resource management became supported for some
>> programming interfaces by contributions of Peter Zijlstra on 2023-05-26.
>> See also the commit 54da6a0924311c7cf5015533991e44fb8eb12773 ("locking:
>> Introduce __cleanup() based infrastructure").
>>
>> Thus add a macro call so that the attribute “__free(mpi_free)” can be
>> applied accordingly.
>
> So where are the users of this?
Do you find the proposed software extension generally reasonable?

Can any more source code places benefit from such a programming interface adjustment?
https://elixir.bootlin.com/linux/v6.12-rc2/A/ident/mpi_free

Regards,
Markus

Re: [v2] crypto: lib/mpi - Extend support for scope-based resource management

[Herbert Xu]

On Thu, Oct 10, 2024 at 10:58:26AM +0200, Markus Elfring wrote:
>
> Do you find the proposed software extension generally reasonable?
> 
> Can any more source code places benefit from such a programming interface adjustment?
> https://elixir.bootlin.com/linux/v6.12-rc2/A/ident/mpi_free

Please submit actual patches using this.

Thanks,
-- 
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

Re: [v2] crypto: lib/mpi - Extend support for scope-based resource management

[Markus Elfring]

>> Do you find the proposed software extension generally reasonable?
>>
>> Can any more source code places benefit from such a programming interface adjustment?
>> https://elixir.bootlin.com/linux/v6.12-rc2/A/ident/mpi_free
>
> Please submit actual patches using this.
I would appreciate more positive indications according to scope-based resource management
extensions before.
I hope that change resistance/acceptance can be clarified already.

Would there be a need to make the activation of discussed special macro calls
depend on other constraints?

Regards,
Markus

[PATCH v3 0/3] lib: Extending support for scope-based resource management

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 12 Oct 2024 15:48:05 +0200

Some development ideas were taken into account according to evolving discussions.

Markus Elfring (3):
  crypto: lib/mpi - Extend support for scope-based resource management
  lib/digsig: Use scope-based resource management for two MPI variables
    in digsig_verify_rsa()
  lib/digsig: Use scope-based resource management for two variables
    in digsig_verify_rsa()

 include/linux/mpi.h |   4 ++
 lib/digsig.c        | 107 +++++++++++++++++++++++---------------------
 2 files changed, 59 insertions(+), 52 deletions(-)


V3:
* Applications were added as requested (by Herbert Xu) for the proposed
  programming interface extension.

* Further adjustments were provided for the demonstration of an evolving
  programming interface.


V2:
The kernel build service pointed out that the proposed identifier “T_”
was not recognised by the compiler.
Thus reserved identifiers need still be applied also at such a place instead
so far.
I became curious under which circumstances corresponding development concerns
will be reconsidered any more.

--
2.46.1

[PATCH v3 1/3] crypto: lib/mpi - Extend support for scope-based resource management

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 18 Sep 2024 11:06:35 +0200

Scope-based resource management became supported for some
programming interfaces by contributions of Peter Zijlstra on 2023-05-26.
See also the commit 54da6a0924311c7cf5015533991e44fb8eb12773 ("locking:
Introduce __cleanup() based infrastructure").

Thus add a macro call so that the attribute “__free(mpi_free)” can be
applied accordingly.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---

V3:
This suggestion was integrated into a patch series.


 include/linux/mpi.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/mpi.h b/include/linux/mpi.h
index 47be46f36435..6fbcb88ce296 100644
--- a/include/linux/mpi.h
+++ b/include/linux/mpi.h
@@ -19,6 +19,8 @@

 #include <linux/types.h>
 #include <linux/scatterlist.h>
+#include <linux/cleanup.h>
+#include <linux/err.h>

 #define BYTES_PER_MPI_LIMB	(BITS_PER_LONG / 8)
 #define BITS_PER_MPI_LIMB	BITS_PER_LONG
@@ -44,6 +46,8 @@ typedef struct gcry_mpi *MPI;
 /*-- mpiutil.c --*/
 MPI mpi_alloc(unsigned nlimbs);
 void mpi_free(MPI a);
+DEFINE_FREE(mpi_free, MPI, if (!IS_ERR_OR_NULL(_T)) mpi_free(_T))
+
 int mpi_resize(MPI a, unsigned nlimbs);

 MPI mpi_copy(MPI a);
--
2.46.1

[PATCH v3 2/3] lib/digsig: Use scope-based resource management for two MPI variables in digsig_verify_rsa()

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 12 Oct 2024 14:21:28 +0200

The support for scope-based resource management was extended.

* Thus use the attribute “__free(mpi_free)”.

* Reduce the scopes for the local variables “nret”, “in” and “res”.

* Omit two mpi_free() calls accordingly.

* Update jump targets.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---

V3:
Applications were added as requested (by Herbert Xu) for the proposed
programming interface extension.


 lib/digsig.c | 80 ++++++++++++++++++++++++++++------------------------
 1 file changed, 43 insertions(+), 37 deletions(-)

diff --git a/lib/digsig.c b/lib/digsig.c
index 04b5e55ed95f..2481120094ab 100644
--- a/lib/digsig.c
+++ b/lib/digsig.c
@@ -71,11 +71,11 @@ static int digsig_verify_rsa(struct key *key,
 	int err = -EINVAL;
 	unsigned long len;
 	unsigned long mlen, mblen;
-	unsigned nret, l;
+	unsigned int l;
 	int head, i;
 	unsigned char *out1 = NULL;
 	const char *m;
-	MPI in = NULL, res = NULL, pkey[2];
+	MPI pkey[2];
 	uint8_t *p, *datap;
 	const uint8_t *endp;
 	const struct user_key_payload *ukp;
@@ -112,7 +112,7 @@ static int digsig_verify_rsa(struct key *key,
 		pkey[i] = mpi_read_from_buffer(datap, &remaining);
 		if (IS_ERR(pkey[i])) {
 			err = PTR_ERR(pkey[i]);
-			goto err;
+			goto free_keys;
 		}
 		datap += remaining;
 	}
@@ -122,57 +122,63 @@ static int digsig_verify_rsa(struct key *key,

 	if (mlen == 0) {
 		err = -EINVAL;
-		goto err;
+		goto free_keys;
 	}

 	err = -ENOMEM;

 	out1 = kzalloc(mlen, GFP_KERNEL);
 	if (!out1)
-		goto err;
+		goto free_keys;

-	nret = siglen;
-	in = mpi_read_from_buffer(sig, &nret);
-	if (IS_ERR(in)) {
-		err = PTR_ERR(in);
-		goto err;
-	}
+	{
+		unsigned int nret = siglen;
+		MPI in __free(mpi_free) = mpi_read_from_buffer(sig, &nret);

-	res = mpi_alloc(mpi_get_nlimbs(in) * 2);
-	if (!res)
-		goto err;
+		if (IS_ERR(in)) {
+			err = PTR_ERR(in);
+			goto in_exit;
+		}

-	err = mpi_powm(res, in, pkey[1], pkey[0]);
-	if (err)
-		goto err;
+		{
+			MPI res __free(mpi_free) = mpi_alloc(mpi_get_nlimbs(in) * 2);

-	if (mpi_get_nlimbs(res) * BYTES_PER_MPI_LIMB > mlen) {
-		err = -EINVAL;
-		goto err;
-	}
+			if (!res)
+				goto res_exit;

-	p = mpi_get_buffer(res, &l, NULL);
-	if (!p) {
-		err = -EINVAL;
-		goto err;
-	}
+			err = mpi_powm(res, in, pkey[1], pkey[0]);
+			if (err)
+				goto res_exit;

-	len = mlen;
-	head = len - l;
-	memset(out1, 0, head);
-	memcpy(out1 + head, p, l);
+			if (mpi_get_nlimbs(res) * BYTES_PER_MPI_LIMB > mlen) {
+				err = -EINVAL;
+				goto res_exit;
+			}

-	kfree(p);
+			p = mpi_get_buffer(res, &l, NULL);
+			if (!p) {
+				err = -EINVAL;
+				goto res_exit;
+			}

-	m = pkcs_1_v1_5_decode_emsa(out1, len, mblen, &len);
+			len = mlen;
+			head = len - l;
+			memset(out1, 0, head);
+			memcpy(out1 + head, p, l);

-	if (!m || len != hlen || memcmp(m, h, hlen))
-		err = -EINVAL;
+			kfree(p);
+
+			m = pkcs_1_v1_5_decode_emsa(out1, len, mblen, &len);
+
+			if (!m || len != hlen || memcmp(m, h, hlen))
+				err = -EINVAL;
+res_exit:
+		}
+in_exit:
+	}

-err:
-	mpi_free(in);
-	mpi_free(res);
 	kfree(out1);
+free_keys:
 	while (--i >= 0)
 		mpi_free(pkey[i]);
 err1:
--
2.46.1

[PATCH v3 3/3] lib/digsig: Use scope-based resource management for two variables in digsig_verify_rsa()

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 12 Oct 2024 15:28:22 +0200

Scope-based resource management became supported for some
programming interfaces by contributions of Peter Zijlstra on 2023-05-26.
See also the commit 54da6a0924311c7cf5015533991e44fb8eb12773 ("locking:
Introduce __cleanup() based infrastructure").

* Thus use the attribute “__free(kfree)”.

* Reduce the scopes for the local variables “out1” and “p”.

* Omit explicit kfree() calls accordingly.

* Add a jump target.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---

V3:
Further adjustments were provided for the demonstration of an evolving
programming interface.


 lib/digsig.c | 87 +++++++++++++++++++++++++---------------------------
 1 file changed, 42 insertions(+), 45 deletions(-)

diff --git a/lib/digsig.c b/lib/digsig.c
index 2481120094ab..1a24677af643 100644
--- a/lib/digsig.c
+++ b/lib/digsig.c
@@ -73,10 +73,9 @@ static int digsig_verify_rsa(struct key *key,
 	unsigned long mlen, mblen;
 	unsigned int l;
 	int head, i;
-	unsigned char *out1 = NULL;
 	const char *m;
 	MPI pkey[2];
-	uint8_t *p, *datap;
+	uint8_t *datap;
 	const uint8_t *endp;
 	const struct user_key_payload *ukp;
 	struct pubkey_hdr *pkh;
@@ -126,58 +125,56 @@ static int digsig_verify_rsa(struct key *key,
 	}

 	err = -ENOMEM;
-
-	out1 = kzalloc(mlen, GFP_KERNEL);
-	if (!out1)
-		goto free_keys;
-
 	{
-		unsigned int nret = siglen;
-		MPI in __free(mpi_free) = mpi_read_from_buffer(sig, &nret);
-
-		if (IS_ERR(in)) {
-			err = PTR_ERR(in);
-			goto in_exit;
-		}
+		unsigned char *out1 __free(kfree) = kzalloc(mlen, GFP_KERNEL);

+		if (out1)
 		{
-			MPI res __free(mpi_free) = mpi_alloc(mpi_get_nlimbs(in) * 2);
-
-			if (!res)
-				goto res_exit;
-
-			err = mpi_powm(res, in, pkey[1], pkey[0]);
-			if (err)
-				goto res_exit;
-
-			if (mpi_get_nlimbs(res) * BYTES_PER_MPI_LIMB > mlen) {
-				err = -EINVAL;
-				goto res_exit;
-			}
+			unsigned int nret = siglen;
+			MPI in __free(mpi_free) = mpi_read_from_buffer(sig, &nret);

-			p = mpi_get_buffer(res, &l, NULL);
-			if (!p) {
-				err = -EINVAL;
-				goto res_exit;
+			if (IS_ERR(in)) {
+				err = PTR_ERR(in);
+				goto in_exit;
 			}

-			len = mlen;
-			head = len - l;
-			memset(out1, 0, head);
-			memcpy(out1 + head, p, l);
-
-			kfree(p);
-
-			m = pkcs_1_v1_5_decode_emsa(out1, len, mblen, &len);
-
-			if (!m || len != hlen || memcmp(m, h, hlen))
-				err = -EINVAL;
+			{
+				MPI res __free(mpi_free) = mpi_alloc(mpi_get_nlimbs(in) * 2);
+
+				if (!res)
+					goto res_exit;
+
+				err = mpi_powm(res, in, pkey[1], pkey[0]);
+				if (err)
+					goto res_exit;
+
+				if (mpi_get_nlimbs(res) * BYTES_PER_MPI_LIMB > mlen) {
+					err = -EINVAL;
+					goto res_exit;
+				}
+
+				{
+					uint8_t *p __free(kfree) = mpi_get_buffer(res, &l, NULL);
+
+					if (!p) {
+						err = -EINVAL;
+						goto p_exit;
+					}
+
+					len = mlen;
+					head = len - l;
+					memset(out1, 0, head);
+					memcpy(out1 + head, p, l);
+					m = pkcs_1_v1_5_decode_emsa(out1, len, mblen, &len);
+					if (!m || len != hlen || memcmp(m, h, hlen))
+						err = -EINVAL;
+p_exit:
+				}
 res_exit:
-		}
+			}
 in_exit:
+		}
 	}
-
-	kfree(out1);
 free_keys:
 	while (--i >= 0)
 		mpi_free(pkey[i]);
--
2.46.1

Re: [PATCH v3 2/3] lib/digsig: Use scope-based resource management for two MPI variables in digsig_verify_rsa()

[kernel test robot]

Hi Markus,

kernel test robot noticed the following build warnings:

[auto build test WARNING on akpm-mm/mm-nonmm-unstable]
[also build test WARNING on herbert-crypto-2.6/master herbert-cryptodev-2.6/master linus/master v6.12-rc3 next-20241015]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Markus-Elfring/crypto-lib-mpi-Extend-support-for-scope-based-resource-management/20241012-231156
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-nonmm-unstable
patch link:    https://lore.kernel.org/r/300a0376-f003-4862-bb16-7e004733c9c1%40web.de
patch subject: [PATCH v3 2/3] lib/digsig: Use scope-based resource management for two MPI variables in digsig_verify_rsa()
config: arm-randconfig-002-20241016 (https://download.01.org/0day-ci/archive/20241016/202410160438.SOIZeFku-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 70e0a7e7e6a8541bcc46908c592eed561850e416)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241016/202410160438.SOIZeFku-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410160438.SOIZeFku-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from lib/digsig.c:25:
   In file included from include/linux/mpi.h:21:
   In file included from include/linux/scatterlist.h:8:
   In file included from include/linux/mm.h:2213:
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
>> lib/digsig.c:176:3: warning: label at end of compound statement is a C23 extension [-Wc23-extensions]
     176 |                 }
         |                 ^
   lib/digsig.c:178:2: warning: label at end of compound statement is a C23 extension [-Wc23-extensions]
     178 |         }
         |         ^
   3 warnings generated.


vim +176 lib/digsig.c

    16	
    17	#include <linux/err.h>
    18	#include <linux/module.h>
    19	#include <linux/slab.h>
    20	#include <linux/key.h>
    21	#include <linux/crypto.h>
    22	#include <crypto/hash.h>
    23	#include <crypto/sha1.h>
    24	#include <keys/user-type.h>
  > 25	#include <linux/mpi.h>
    26	#include <linux/digsig.h>
    27	
    28	static struct crypto_shash *shash;
    29	
    30	static const char *pkcs_1_v1_5_decode_emsa(const unsigned char *msg,
    31							unsigned long  msglen,
    32							unsigned long  modulus_bitlen,
    33							unsigned long *outlen)
    34	{
    35		unsigned long modulus_len, ps_len, i;
    36	
    37		modulus_len = (modulus_bitlen >> 3) + (modulus_bitlen & 7 ? 1 : 0);
    38	
    39		/* test message size */
    40		if ((msglen > modulus_len) || (modulus_len < 11))
    41			return NULL;
    42	
    43		/* separate encoded message */
    44		if (msg[0] != 0x00 || msg[1] != 0x01)
    45			return NULL;
    46	
    47		for (i = 2; i < modulus_len - 1; i++)
    48			if (msg[i] != 0xFF)
    49				break;
    50	
    51		/* separator check */
    52		if (msg[i] != 0)
    53			/* There was no octet with hexadecimal value 0x00
    54			to separate ps from m. */
    55			return NULL;
    56	
    57		ps_len = i - 2;
    58	
    59		*outlen = (msglen - (2 + ps_len + 1));
    60	
    61		return msg + 2 + ps_len + 1;
    62	}
    63	
    64	/*
    65	 * RSA Signature verification with public key
    66	 */
    67	static int digsig_verify_rsa(struct key *key,
    68			    const char *sig, int siglen,
    69			       const char *h, int hlen)
    70	{
    71		int err = -EINVAL;
    72		unsigned long len;
    73		unsigned long mlen, mblen;
    74		unsigned int l;
    75		int head, i;
    76		unsigned char *out1 = NULL;
    77		const char *m;
    78		MPI pkey[2];
    79		uint8_t *p, *datap;
    80		const uint8_t *endp;
    81		const struct user_key_payload *ukp;
    82		struct pubkey_hdr *pkh;
    83	
    84		down_read(&key->sem);
    85		ukp = user_key_payload_locked(key);
    86	
    87		if (!ukp) {
    88			/* key was revoked before we acquired its semaphore */
    89			err = -EKEYREVOKED;
    90			goto err1;
    91		}
    92	
    93		if (ukp->datalen < sizeof(*pkh))
    94			goto err1;
    95	
    96		pkh = (struct pubkey_hdr *)ukp->data;
    97	
    98		if (pkh->version != 1)
    99			goto err1;
   100	
   101		if (pkh->algo != PUBKEY_ALGO_RSA)
   102			goto err1;
   103	
   104		if (pkh->nmpi != 2)
   105			goto err1;
   106	
   107		datap = pkh->mpi;
   108		endp = ukp->data + ukp->datalen;
   109	
   110		for (i = 0; i < pkh->nmpi; i++) {
   111			unsigned int remaining = endp - datap;
   112			pkey[i] = mpi_read_from_buffer(datap, &remaining);
   113			if (IS_ERR(pkey[i])) {
   114				err = PTR_ERR(pkey[i]);
   115				goto free_keys;
   116			}
   117			datap += remaining;
   118		}
   119	
   120		mblen = mpi_get_nbits(pkey[0]);
   121		mlen = DIV_ROUND_UP(mblen, 8);
   122	
   123		if (mlen == 0) {
   124			err = -EINVAL;
   125			goto free_keys;
   126		}
   127	
   128		err = -ENOMEM;
   129	
   130		out1 = kzalloc(mlen, GFP_KERNEL);
   131		if (!out1)
   132			goto free_keys;
   133	
   134		{
   135			unsigned int nret = siglen;
   136			MPI in __free(mpi_free) = mpi_read_from_buffer(sig, &nret);
   137	
   138			if (IS_ERR(in)) {
   139				err = PTR_ERR(in);
   140				goto in_exit;
   141			}
   142	
   143			{
   144				MPI res __free(mpi_free) = mpi_alloc(mpi_get_nlimbs(in) * 2);
   145	
   146				if (!res)
   147					goto res_exit;
   148	
   149				err = mpi_powm(res, in, pkey[1], pkey[0]);
   150				if (err)
   151					goto res_exit;
   152	
   153				if (mpi_get_nlimbs(res) * BYTES_PER_MPI_LIMB > mlen) {
   154					err = -EINVAL;
   155					goto res_exit;
   156				}
   157	
   158				p = mpi_get_buffer(res, &l, NULL);
   159				if (!p) {
   160					err = -EINVAL;
   161					goto res_exit;
   162				}
   163	
   164				len = mlen;
   165				head = len - l;
   166				memset(out1, 0, head);
   167				memcpy(out1 + head, p, l);
   168	
   169				kfree(p);
   170	
   171				m = pkcs_1_v1_5_decode_emsa(out1, len, mblen, &len);
   172	
   173				if (!m || len != hlen || memcmp(m, h, hlen))
   174					err = -EINVAL;
   175	res_exit:
 > 176			}
   177	in_exit:
   178		}
   179	
   180		kfree(out1);
   181	free_keys:
   182		while (--i >= 0)
   183			mpi_free(pkey[i]);
   184	err1:
   185		up_read(&key->sem);
   186	
   187		return err;
   188	}
   189	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v3 2/3] lib/digsig: Use scope-based resource management for two MPI variables in digsig_verify_rsa()

[kernel test robot]

Hi Markus,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-nonmm-unstable]
[also build test ERROR on herbert-crypto-2.6/master herbert-cryptodev-2.6/master linus/master v6.12-rc3 next-20241016]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Markus-Elfring/crypto-lib-mpi-Extend-support-for-scope-based-resource-management/20241012-231156
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-nonmm-unstable
patch link:    https://lore.kernel.org/r/300a0376-f003-4862-bb16-7e004733c9c1%40web.de
patch subject: [PATCH v3 2/3] lib/digsig: Use scope-based resource management for two MPI variables in digsig_verify_rsa()
config: hexagon-randconfig-r064-20241016 (https://download.01.org/0day-ci/archive/20241016/202410161914.lY62TWL3-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241016/202410161914.lY62TWL3-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410161914.lY62TWL3-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from lib/digsig.c:25:
   In file included from include/linux/mpi.h:21:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
                                                     ^
   In file included from lib/digsig.c:25:
   In file included from include/linux/mpi.h:21:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
                                                     ^
   In file included from lib/digsig.c:25:
   In file included from include/linux/mpi.h:21:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
>> lib/digsig.c:176:3: error: expected statement
                   }
                   ^
   lib/digsig.c:178:2: error: expected statement
           }
           ^
   6 warnings and 2 errors generated.


vim +176 lib/digsig.c

    63	
    64	/*
    65	 * RSA Signature verification with public key
    66	 */
    67	static int digsig_verify_rsa(struct key *key,
    68			    const char *sig, int siglen,
    69			       const char *h, int hlen)
    70	{
    71		int err = -EINVAL;
    72		unsigned long len;
    73		unsigned long mlen, mblen;
    74		unsigned int l;
    75		int head, i;
    76		unsigned char *out1 = NULL;
    77		const char *m;
    78		MPI pkey[2];
    79		uint8_t *p, *datap;
    80		const uint8_t *endp;
    81		const struct user_key_payload *ukp;
    82		struct pubkey_hdr *pkh;
    83	
    84		down_read(&key->sem);
    85		ukp = user_key_payload_locked(key);
    86	
    87		if (!ukp) {
    88			/* key was revoked before we acquired its semaphore */
    89			err = -EKEYREVOKED;
    90			goto err1;
    91		}
    92	
    93		if (ukp->datalen < sizeof(*pkh))
    94			goto err1;
    95	
    96		pkh = (struct pubkey_hdr *)ukp->data;
    97	
    98		if (pkh->version != 1)
    99			goto err1;
   100	
   101		if (pkh->algo != PUBKEY_ALGO_RSA)
   102			goto err1;
   103	
   104		if (pkh->nmpi != 2)
   105			goto err1;
   106	
   107		datap = pkh->mpi;
   108		endp = ukp->data + ukp->datalen;
   109	
   110		for (i = 0; i < pkh->nmpi; i++) {
   111			unsigned int remaining = endp - datap;
   112			pkey[i] = mpi_read_from_buffer(datap, &remaining);
   113			if (IS_ERR(pkey[i])) {
   114				err = PTR_ERR(pkey[i]);
   115				goto free_keys;
   116			}
   117			datap += remaining;
   118		}
   119	
   120		mblen = mpi_get_nbits(pkey[0]);
   121		mlen = DIV_ROUND_UP(mblen, 8);
   122	
   123		if (mlen == 0) {
   124			err = -EINVAL;
   125			goto free_keys;
   126		}
   127	
   128		err = -ENOMEM;
   129	
   130		out1 = kzalloc(mlen, GFP_KERNEL);
   131		if (!out1)
   132			goto free_keys;
   133	
   134		{
   135			unsigned int nret = siglen;
   136			MPI in __free(mpi_free) = mpi_read_from_buffer(sig, &nret);
   137	
   138			if (IS_ERR(in)) {
   139				err = PTR_ERR(in);
   140				goto in_exit;
   141			}
   142	
   143			{
   144				MPI res __free(mpi_free) = mpi_alloc(mpi_get_nlimbs(in) * 2);
   145	
   146				if (!res)
   147					goto res_exit;
   148	
   149				err = mpi_powm(res, in, pkey[1], pkey[0]);
   150				if (err)
   151					goto res_exit;
   152	
   153				if (mpi_get_nlimbs(res) * BYTES_PER_MPI_LIMB > mlen) {
   154					err = -EINVAL;
   155					goto res_exit;
   156				}
   157	
   158				p = mpi_get_buffer(res, &l, NULL);
   159				if (!p) {
   160					err = -EINVAL;
   161					goto res_exit;
   162				}
   163	
   164				len = mlen;
   165				head = len - l;
   166				memset(out1, 0, head);
   167				memcpy(out1 + head, p, l);
   168	
   169				kfree(p);
   170	
   171				m = pkcs_1_v1_5_decode_emsa(out1, len, mblen, &len);
   172	
   173				if (!m || len != hlen || memcmp(m, h, hlen))
   174					err = -EINVAL;
   175	res_exit:
 > 176			}
   177	in_exit:
   178		}
   179	
   180		kfree(out1);
   181	free_keys:
   182		while (--i >= 0)
   183			mpi_free(pkey[i]);
   184	err1:
   185		up_read(&key->sem);
   186	
   187		return err;
   188	}
   189	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki