lkml.org 
[lkml]   [2018]   [Jul]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v4 11/14] treewide: Prepare to remove VLA usage for AHASH_REQUEST_ON_STACK
On Thu, Jul 12, 2018 at 10:20 PM, Herbert Xu
<herbert@gondor.apana.org.au> wrote:
> On Thu, Jul 12, 2018 at 10:17:29PM -0700, Kees Cook wrote:
>>
>> Then why does the instrumented tcrypt output show the huge size? Is
>> tcrypt doing something incorrectly?
>
> tcrypt doesn't even use AHASH_REQUEST_ON_STACK so I don't understand
> your point.

It's using crypto_ahash_set_reqsize(), which is what
AHASH_REQUEST_ON_STACK() reads back via crypto_ahash_reqsize() (i.e.
tfm->reqsize). It sounds like you're saying that there are cases where
an ahash is constructed (and will call crypto_ahash_set_reqsize()) but
where it cannot be used with AHASH_REQUEST_ON_STACK()? What actually
enforces this, since there will be a difference between
crypto_ahash_set_reqsize() (as seen with sha512-mb) and the actually
allowed stack usage. (i.e. where should I perform a check against the
new fixed value?)

>> What is the correct value to use for AHASH_REQUEST_ON_STACK?
>
> As I said to arrive at a fixed value you should examine all sync
> ahash algorithms (e.g., all shash ones plus ahash ones marked as
> sync if there are any).

The "value" for the ahash I understand: it has a request size
(tfm->reqsize, as set by crypto_ahash_set_reqsize()) what should be
used to measure the shash value? (And how does this relate to the
value returned by crypto_ahash_reqsize()?) The closest clue I can find
is this:

crypto_init_shash_ops_async() does:
crt->reqsize = sizeof(struct shash_desc) + crypto_shash_descsize(shash);

and that gets called from crypto_ahash_init_tfm(), so if it starts
with the above reqsize and adds to it with a call to
crypto_ahash_set_reqsize() later, we'll have that maximum?

So, do I want to calculate this answer as:

sizeof(struct shash_desc) + max(shash descsize) + max(ahash-sync reqsize) ?
16 + 360 + 0

It's 0 above because if I look at all the callers of
crypto_ahash_set_reqsize() that do wrapping, all are ASYNC.

So, should this really just be 376? Where is best to validate this
size, as it seems checking in crypto_ahash_set_reqsize() is
inappropriate?

-Kees

--
Kees Cook
Pixel Security
\
 
 \ /
  Last update: 2018-07-15 22:06    [W:0.098 / U:0.700 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site