Re: [PATCH 1/3] crypto: ensure algif_hash does not pass a zero-sized state

2015-10-09 Thread Herbert Xu 
On Fri, Oct 09, 2015 at 11:29:44AM +0100, Russell King wrote:
> If the algorithm passed a zero statesize, do not pass a valid pointer
> into the export/import functions.  Passing a valid pointer covers up
> bugs in driver code which then go on to smash the kernel stack.
> Instead, pass NULL, which will cause any attempt to write to the
> pointer to fail.
> 
> Signed-off-by: Russell King 

The state size should never be zero for a hash algorithm.  Having
a zero state means that the hash output must always be identical.
Such an algorithm would be quite useless.

So how about adding a check upon hash registration to verify that
the state size is greater than zero? The place to do it would be
shash_prepare_alg and ahash_prepare_alg.

Thanks,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] crypto: ensure algif_hash does not pass a zero-sized state

2015-10-09 Thread Herbert Xu 
On Fri, Oct 09, 2015 at 11:41:07AM +0100, Russell King - ARM Linux wrote:
>
> Do you mean something like this?  As statesize is an unsigned int, testing
> for zero should be sufficient.

Yes looks great to me.

Thanks,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] crypto: ensure algif_hash does not pass a zero-sized state

2015-10-09 Thread Russell King - ARM Linux 
On Fri, Oct 09, 2015 at 06:34:28PM +0800, Herbert Xu wrote:
> On Fri, Oct 09, 2015 at 11:29:44AM +0100, Russell King wrote:
> > If the algorithm passed a zero statesize, do not pass a valid pointer
> > into the export/import functions.  Passing a valid pointer covers up
> > bugs in driver code which then go on to smash the kernel stack.
> > Instead, pass NULL, which will cause any attempt to write to the
> > pointer to fail.
> > 
> > Signed-off-by: Russell King 
> 
> The state size should never be zero for a hash algorithm.  Having
> a zero state means that the hash output must always be identical.
> Such an algorithm would be quite useless.
> 
> So how about adding a check upon hash registration to verify that
> the state size is greater than zero? The place to do it would be
> shash_prepare_alg and ahash_prepare_alg.

Do you mean something like this?  As statesize is an unsigned int, testing
for zero should be sufficient.

diff --git a/crypto/ahash.c b/crypto/ahash.c
index 8acb886032ae..9c1dc8d6106a 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -544,7 +544,8 @@ static int ahash_prepare_alg(struct ahash_alg *alg)
struct crypto_alg *base = >halg.base;
 
if (alg->halg.digestsize > PAGE_SIZE / 8 ||
-   alg->halg.statesize > PAGE_SIZE / 8)
+   alg->halg.statesize > PAGE_SIZE / 8 ||
+   alg->halg.statesize == 0)
return -EINVAL;
 
base->cra_type = _ahash_type;
diff --git a/crypto/shash.c b/crypto/shash.c
index ecb1e3d39bf0..ab3384b38542 100644
--- a/crypto/shash.c
+++ b/crypto/shash.c
@@ -585,7 +585,8 @@ static int shash_prepare_alg(struct shash_alg *alg)
 
if (alg->digestsize > PAGE_SIZE / 8 ||
alg->descsize > PAGE_SIZE / 8 ||
-   alg->statesize > PAGE_SIZE / 8)
+   alg->statesize > PAGE_SIZE / 8 ||
+   alg->statesize == 0)
return -EINVAL;
 
base->cra_type = _shash_type;

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html