From: Eric Biggers <[email protected]>
crypto_shash_descsize() returns the size of the shash_desc context
needed to compute the hash, not the size of the hash itself.
crypto_shash_digestsize() would be correct, or alternatively using
c->hash_len and c->hmac_desc_len which already store the correct values.
But actually it's simpler to just use stack arrays, so do that instead.
Fixes: 49525e5eecca ("ubifs: Add helper functions for authentication support")
Fixes: da8ef65f9573 ("ubifs: Authenticate replayed journal")
Cc: <[email protected]> # v4.20+
Cc: Sascha Hauer <[email protected]>
Signed-off-by: Eric Biggers <[email protected]>
---
fs/ubifs/auth.c | 17 ++++-------------
fs/ubifs/replay.c | 13 ++-----------
2 files changed, 6 insertions(+), 24 deletions(-)
diff --git a/fs/ubifs/auth.c b/fs/ubifs/auth.c
index 8cdbd53d780ca7..f985a3fbbb36a1 100644
--- a/fs/ubifs/auth.c
+++ b/fs/ubifs/auth.c
@@ -79,13 +79,9 @@ int ubifs_prepare_auth_node(struct ubifs_info *c, void *node,
struct shash_desc *inhash)
{
struct ubifs_auth_node *auth = node;
- u8 *hash;
+ u8 hash[UBIFS_HASH_ARR_SZ];
int err;
- hash = kmalloc(crypto_shash_descsize(c->hash_tfm), GFP_NOFS);
- if (!hash)
- return -ENOMEM;
-
{
SHASH_DESC_ON_STACK(hash_desc, c->hash_tfm);
@@ -94,21 +90,16 @@ int ubifs_prepare_auth_node(struct ubifs_info *c, void *node,
err = crypto_shash_final(hash_desc, hash);
if (err)
- goto out;
+ return err;
}
err = ubifs_hash_calc_hmac(c, hash, auth->hmac);
if (err)
- goto out;
+ return err;
auth->ch.node_type = UBIFS_AUTH_NODE;
ubifs_prepare_node(c, auth, ubifs_auth_node_sz(c), 0);
-
- err = 0;
-out:
- kfree(hash);
-
- return err;
+ return 0;
}
static struct shash_desc *ubifs_get_desc(const struct ubifs_info *c,
diff --git a/fs/ubifs/replay.c b/fs/ubifs/replay.c
index b28ac4dfb4070a..01fcf79750472b 100644
--- a/fs/ubifs/replay.c
+++ b/fs/ubifs/replay.c
@@ -601,18 +601,12 @@ static int authenticate_sleb(struct ubifs_info *c, struct ubifs_scan_leb *sleb,
struct ubifs_scan_node *snod;
int n_nodes = 0;
int err;
- u8 *hash, *hmac;
+ u8 hash[UBIFS_HASH_ARR_SZ];
+ u8 hmac[UBIFS_HMAC_ARR_SZ];
if (!ubifs_authenticated(c))
return sleb->nodes_cnt;
- hash = kmalloc(crypto_shash_descsize(c->hash_tfm), GFP_NOFS);
- hmac = kmalloc(c->hmac_desc_len, GFP_NOFS);
- if (!hash || !hmac) {
- err = -ENOMEM;
- goto out;
- }
-
list_for_each_entry(snod, &sleb->nodes, list) {
n_nodes++;
@@ -662,9 +656,6 @@ static int authenticate_sleb(struct ubifs_info *c, struct ubifs_scan_leb *sleb,
err = 0;
}
out:
- kfree(hash);
- kfree(hmac);
-
return err ? err : n_nodes - n_not_auth;
}
--
2.26.2
On Fri, May 01, 2020 at 10:59:45PM -0700, Eric Biggers wrote:
> From: Eric Biggers <[email protected]>
>
> crypto_shash_descsize() returns the size of the shash_desc context
> needed to compute the hash, not the size of the hash itself.
>
> crypto_shash_digestsize() would be correct, or alternatively using
> c->hash_len and c->hmac_desc_len which already store the correct values.
> But actually it's simpler to just use stack arrays, so do that instead.
>
> Fixes: 49525e5eecca ("ubifs: Add helper functions for authentication support")
> Fixes: da8ef65f9573 ("ubifs: Authenticate replayed journal")
> Cc: <[email protected]> # v4.20+
> Cc: Sascha Hauer <[email protected]>
> Signed-off-by: Eric Biggers <[email protected]>
Looks better that way, thanks.
Acked-by: Sascha Hauer <[email protected]>
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Mon, May 04, 2020 at 09:16:44AM +0200, Sascha Hauer wrote:
> On Fri, May 01, 2020 at 10:59:45PM -0700, Eric Biggers wrote:
> > From: Eric Biggers <[email protected]>
> >
> > crypto_shash_descsize() returns the size of the shash_desc context
> > needed to compute the hash, not the size of the hash itself.
> >
> > crypto_shash_digestsize() would be correct, or alternatively using
> > c->hash_len and c->hmac_desc_len which already store the correct values.
> > But actually it's simpler to just use stack arrays, so do that instead.
> >
> > Fixes: 49525e5eecca ("ubifs: Add helper functions for authentication support")
> > Fixes: da8ef65f9573 ("ubifs: Authenticate replayed journal")
> > Cc: <[email protected]> # v4.20+
> > Cc: Sascha Hauer <[email protected]>
> > Signed-off-by: Eric Biggers <[email protected]>
>
> Looks better that way, thanks.
>
> Acked-by: Sascha Hauer <[email protected]>
>
Richard, could you take this through the ubifs tree for 5.8?
- Eric
----- Ursprüngliche Mail -----
> Von: "Eric Biggers" <[email protected]>
> An: "Sascha Hauer" <[email protected]>, "richard" <[email protected]>
> CC: "linux-mtd" <[email protected]>, "Linux Crypto Mailing List" <[email protected]>, "stable"
> <[email protected]>
> Gesendet: Freitag, 15. Mai 2020 21:17:04
> Betreff: Re: [PATCH] ubifs: fix wrong use of crypto_shash_descsize()
> On Mon, May 04, 2020 at 09:16:44AM +0200, Sascha Hauer wrote:
>> On Fri, May 01, 2020 at 10:59:45PM -0700, Eric Biggers wrote:
>> > From: Eric Biggers <[email protected]>
>> >
>> > crypto_shash_descsize() returns the size of the shash_desc context
>> > needed to compute the hash, not the size of the hash itself.
>> >
>> > crypto_shash_digestsize() would be correct, or alternatively using
>> > c->hash_len and c->hmac_desc_len which already store the correct values.
>> > But actually it's simpler to just use stack arrays, so do that instead.
>> >
>> > Fixes: 49525e5eecca ("ubifs: Add helper functions for authentication support")
>> > Fixes: da8ef65f9573 ("ubifs: Authenticate replayed journal")
>> > Cc: <[email protected]> # v4.20+
>> > Cc: Sascha Hauer <[email protected]>
>> > Signed-off-by: Eric Biggers <[email protected]>
>>
>> Looks better that way, thanks.
>>
>> Acked-by: Sascha Hauer <[email protected]>
>>
>
> Richard, could you take this through the ubifs tree for 5.8?
Sure. I actually will send a PR with various MTD related fixes
for 5.7.
Thanks,
//richard
On Fri, May 15, 2020 at 10:50 PM Richard Weinberger <[email protected]> wrote:
>
> ----- Ursprüngliche Mail -----
> > Von: "Eric Biggers" <[email protected]>
> > An: "Sascha Hauer" <[email protected]>, "richard" <[email protected]>
> > CC: "linux-mtd" <[email protected]>, "Linux Crypto Mailing List" <[email protected]>, "stable"
> > <[email protected]>
> > Gesendet: Freitag, 15. Mai 2020 21:17:04
> > Betreff: Re: [PATCH] ubifs: fix wrong use of crypto_shash_descsize()
>
> > On Mon, May 04, 2020 at 09:16:44AM +0200, Sascha Hauer wrote:
> >> On Fri, May 01, 2020 at 10:59:45PM -0700, Eric Biggers wrote:
> >> > From: Eric Biggers <[email protected]>
> >> >
> >> > crypto_shash_descsize() returns the size of the shash_desc context
> >> > needed to compute the hash, not the size of the hash itself.
> >> >
> >> > crypto_shash_digestsize() would be correct, or alternatively using
> >> > c->hash_len and c->hmac_desc_len which already store the correct values.
> >> > But actually it's simpler to just use stack arrays, so do that instead.
> >> >
> >> > Fixes: 49525e5eecca ("ubifs: Add helper functions for authentication support")
> >> > Fixes: da8ef65f9573 ("ubifs: Authenticate replayed journal")
> >> > Cc: <[email protected]> # v4.20+
> >> > Cc: Sascha Hauer <[email protected]>
> >> > Signed-off-by: Eric Biggers <[email protected]>
> >>
> >> Looks better that way, thanks.
> >>
> >> Acked-by: Sascha Hauer <[email protected]>
> >>
> >
> > Richard, could you take this through the ubifs tree for 5.8?
>
> Sure. I actually will send a PR with various MTD related fixes
> for 5.7.
Applied. Thanks for fixing!
--
Thanks,
//richard