Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1407726 > unrolled thread

[PATCH 0/5] refactor mpi_read_from_buffer()

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] refactor mpi_read_from_buffer() Nicolai Stange <nicstange@gmail.com> - 2016-05-26 23:30 +0200
    [PATCH 4/5] lib/mpi: mpi_read_from_buffer(): sanitize short buffer printk Nicolai Stange <nicstange@gmail.com> - 2016-05-26 23:30 +0200
    [PATCH 5/5] lib/mpi: refactor mpi_read_from_buffer() in terms of mpi_read_raw_data() Nicolai Stange <nicstange@gmail.com> - 2016-05-26 23:30 +0200
    Re: [PATCH 0/5] refactor mpi_read_from_buffer() Herbert Xu <herbert@gondor.apana.org.au> - 2016-05-31 12:20 +0200
      Re: [PATCH 0/5] refactor mpi_read_from_buffer() Nicolai Stange <nicstange@gmail.com> - 2016-05-31 21:10 +0200

#1407726 — [PATCH 0/5] refactor mpi_read_from_buffer()

mpi_read_from_buffer() and mpi_read_raw_data() do almost the same and share a
fair amount of common code.

This patchset attempts to rewrite mpi_read_from_buffer() in order to implement
it in terms of mpi_read_raw_data().

The patches 1 and 3, i.e.
  "lib/mpi: mpi_read_from_buffer(): return error code"
and
  "lib/mpi: mpi_read_from_buffer(): return -EINVAL upon too short buffer"
do the groundwork in that they move any error detection unique to
mpi_read_from_buffer() out of the data handling loop.

The patches 2 and 4, that is
  "lib/digsig: digsig_verify_rsa(): return -EINVAL if modulo length is zero"
and
  "lib/mpi: mpi_read_from_buffer(): sanitize short buffer printk"
are not strictly necessary for the refactoring: they cleanup some minor oddities
related to error handling I came across.

Finally, the last patch in this series,
  "lib/mpi: refactor mpi_read_from_buffer() in terms of mpi_read_raw_data()"
actually does what this series is all about.


Applicable to linux-next-20160325.

A note on testing: allmodconfig on x86_64 builds fine. 
However, the only caller of mpi_read_from_buffer() is digsig_verify_rsa()
and this one is solely used by the IMA/EVM infrastructure.
In my current setup, I don't have any IMA/EVM stuff in place and thus,
I can't do any runtime tests without putting *much* effort into it.
I would really appreciate if someone with a working IMA/EVM setup could do some
brief testing...

Nicolai Stange (5):
  lib/mpi: mpi_read_from_buffer(): return error code
  lib/digsig: digsig_verify_rsa(): return -EINVAL if modulo length is
    zero
  lib/mpi: mpi_read_from_buffer(): return -EINVAL upon too short buffer
  lib/mpi: mpi_read_from_buffer(): sanitize short buffer printk
  lib/mpi: refactor mpi_read_from_buffer() in terms of
    mpi_read_raw_data()

 lib/digsig.c       | 16 +++++++++++-----
 lib/mpi/mpicoder.c | 46 +++++++++++++---------------------------------
 2 files changed, 24 insertions(+), 38 deletions(-)

-- 
2.8.2

[toc] | [next] | [standalone]

#1407728 — [PATCH 4/5] lib/mpi: mpi_read_from_buffer(): sanitize short buffer printk

The first two bytes of the input buffer encode its expected length and
mpi_read_from_buffer() prints a console message if the given buffer is too
short.

However, there are some oddities with how this message is printed:
- It is printed at the default loglevel. This is different from the
  one used in the case that the first two bytes' value is unsupportedly
  large, i.e. KERN_INFO.
- The format specifier '%d' is used for unsigned ints.
- It prints the values of nread and *ret_nread. This is redundant since
  the former is always the latter + 1.

Clean this up as follows:
- Use pr_info() rather than printk() with no loglevel.
- Use the format specifiers '%u' in place if '%d'.
- Do not print the redundant 'nread' but the more helpful 'nbytes' value.

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 lib/mpi/mpicoder.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/mpi/mpicoder.c b/lib/mpi/mpicoder.c
index 869c66c..2f4d039 100644
--- a/lib/mpi/mpicoder.c
+++ b/lib/mpi/mpicoder.c
@@ -99,8 +99,8 @@ MPI mpi_read_from_buffer(const void *xbuffer, unsigned *ret_nread)
 
 	nbytes = DIV_ROUND_UP(nbits, 8);
 	if (nbytes + 2 > *ret_nread) {
-		printk("MPI: mpi larger than buffer nread=%d ret_nread=%d\n",
-				*ret_nread + 1, *ret_nread);
+		pr_info("MPI: mpi larger than buffer nbytes=%u ret_nread=%u\n",
+				nbytes, *ret_nread);
 		return ERR_PTR(-EINVAL);
 	}
 
-- 
2.8.2

[toc] | [prev] | [next] | [standalone]

#1407729 — [PATCH 5/5] lib/mpi: refactor mpi_read_from_buffer() in terms of mpi_read_raw_data()

mpi_read_from_buffer() and mpi_read_raw_data() do basically the same thing
except that the former extracts the number of payload bits from the first
two bytes of the input buffer.

Besides that, the data copying logic is exactly the same.

Replace the open coded buffer to MPI instance conversion by a call to
mpi_read_raw_data().

Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
 lib/mpi/mpicoder.c | 24 +++---------------------
 1 file changed, 3 insertions(+), 21 deletions(-)

diff --git a/lib/mpi/mpicoder.c b/lib/mpi/mpicoder.c
index 2f4d039..e8a5742 100644
--- a/lib/mpi/mpicoder.c
+++ b/lib/mpi/mpicoder.c
@@ -82,10 +82,8 @@ EXPORT_SYMBOL_GPL(mpi_read_raw_data);
 MPI mpi_read_from_buffer(const void *xbuffer, unsigned *ret_nread)
 {
 	const uint8_t *buffer = xbuffer;
-	int i, j;
-	unsigned nbits, nbytes, nlimbs;
-	mpi_limb_t a;
-	MPI val = NULL;
+	unsigned int nbits, nbytes;
+	MPI val;
 
 	if (*ret_nread < 2)
 		return ERR_PTR(-EINVAL);
@@ -95,7 +93,6 @@ MPI mpi_read_from_buffer(const void *xbuffer, unsigned *ret_nread)
 		pr_info("MPI: mpi too large (%u bits)\n", nbits);
 		return ERR_PTR(-EINVAL);
 	}
-	buffer += 2;
 
 	nbytes = DIV_ROUND_UP(nbits, 8);
 	if (nbytes + 2 > *ret_nread) {
@@ -104,24 +101,9 @@ MPI mpi_read_from_buffer(const void *xbuffer, unsigned *ret_nread)
 		return ERR_PTR(-EINVAL);
 	}
 
-	nlimbs = DIV_ROUND_UP(nbytes, BYTES_PER_MPI_LIMB);
-	val = mpi_alloc(nlimbs);
+	val = mpi_read_raw_data(buffer + 2, nbytes);
 	if (!val)
 		return ERR_PTR(-ENOMEM);
-	i = BYTES_PER_MPI_LIMB - nbytes % BYTES_PER_MPI_LIMB;
-	i %= BYTES_PER_MPI_LIMB;
-	val->nbits = nbits;
-	j = val->nlimbs = nlimbs;
-	val->sign = 0;
-	for (; j > 0; j--) {
-		a = 0;
-		for (; i < BYTES_PER_MPI_LIMB; i++) {
-			a <<= 8;
-			a |= *buffer++;
-		}
-		i = 0;
-		val->d[j - 1] = a;
-	}
 
 	*ret_nread = nbytes + 2;
 	return val;
-- 
2.8.2

[toc] | [prev] | [next] | [standalone]

#1410083

On Thu, May 26, 2016 at 11:19:50PM +0200, Nicolai Stange wrote:
> mpi_read_from_buffer() and mpi_read_raw_data() do almost the same and share a
> fair amount of common code.
> 
> This patchset attempts to rewrite mpi_read_from_buffer() in order to implement
> it in terms of mpi_read_raw_data().
> 
> The patches 1 and 3, i.e.
>   "lib/mpi: mpi_read_from_buffer(): return error code"
> and
>   "lib/mpi: mpi_read_from_buffer(): return -EINVAL upon too short buffer"
> do the groundwork in that they move any error detection unique to
> mpi_read_from_buffer() out of the data handling loop.
> 
> The patches 2 and 4, that is
>   "lib/digsig: digsig_verify_rsa(): return -EINVAL if modulo length is zero"
> and
>   "lib/mpi: mpi_read_from_buffer(): sanitize short buffer printk"
> are not strictly necessary for the refactoring: they cleanup some minor oddities
> related to error handling I came across.
> 
> Finally, the last patch in this series,
>   "lib/mpi: refactor mpi_read_from_buffer() in terms of mpi_read_raw_data()"
> actually does what this series is all about.
> 
> 
> Applicable to linux-next-20160325.

All applied.
-- 
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

[toc] | [prev] | [next] | [standalone]

#1410545

Herbert Xu <herbert@gondor.apana.org.au> writes:

> On Thu, May 26, 2016 at 11:19:50PM +0200, Nicolai Stange wrote:
>> mpi_read_from_buffer() and mpi_read_raw_data() do almost the same and share a
>> fair amount of common code.
>> 
>> This patchset attempts to rewrite mpi_read_from_buffer() in order to implement
>> it in terms of mpi_read_raw_data().
>> 
>> The patches 1 and 3, i.e.
>>   "lib/mpi: mpi_read_from_buffer(): return error code"
>> and
>>   "lib/mpi: mpi_read_from_buffer(): return -EINVAL upon too short buffer"
>> do the groundwork in that they move any error detection unique to
>> mpi_read_from_buffer() out of the data handling loop.
>> 
>> The patches 2 and 4, that is
>>   "lib/digsig: digsig_verify_rsa(): return -EINVAL if modulo length is zero"
>> and
>>   "lib/mpi: mpi_read_from_buffer(): sanitize short buffer printk"
>> are not strictly necessary for the refactoring: they cleanup some minor oddities
>> related to error handling I came across.
>> 
>> Finally, the last patch in this series,
>>   "lib/mpi: refactor mpi_read_from_buffer() in terms of mpi_read_raw_data()"
>> actually does what this series is all about.
>> 
>> 
>> Applicable to linux-next-20160325.
>
> All applied.

Thanks! (As well as for applying the separately sent patches, of course)

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web