lkml.org 
[lkml]   [2021]   [Jul]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] security: keys: trusted: Fix memory leaks on allocated blob
On Fri, Jul 23, 2021 at 06:21:21PM +0100, Colin King wrote:
> @@ -441,6 +449,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> rc = -EPERM;
>
> return rc;
> +
> +err:
> + kfree(blob);

This needs to be:

if (blob != payload->blob)
kfree(blob);

Otherwise it leads to a use after free.

> + return rc;
> }

How this is allocated is pretty scary looking!

security/keys/trusted-keys/trusted_tpm2.c
96 static int tpm2_key_decode(struct trusted_key_payload *payload,
97 struct trusted_key_options *options,
98 u8 **buf)
99 {
100 int ret;
101 struct tpm2_key_context ctx;
102 u8 *blob;
103
104 memset(&ctx, 0, sizeof(ctx));
105
106 ret = asn1_ber_decoder(&tpm2key_decoder, &ctx, payload->blob,
107 payload->blob_len);
108 if (ret < 0)
109 return ret;

Old form?

110
111 if (ctx.priv_len + ctx.pub_len > MAX_BLOB_SIZE)
112 return -EINVAL;

It's really scary to me that if the lengths are too large for kmalloc()
then we just use "payload->blob".

113
114 blob = kmalloc(ctx.priv_len + ctx.pub_len + 4, GFP_KERNEL);

blob is allocated here.

115 if (!blob)
116 return -ENOMEM;
117
118 *buf = blob;
119 options->keyhandle = ctx.parent;
120
121 memcpy(blob, ctx.priv, ctx.priv_len);
122 blob += ctx.priv_len;
123
124 memcpy(blob, ctx.pub, ctx.pub_len);
125
126 return 0;
127 }

[ snip ]

371 u32 attrs;
372
373 rc = tpm2_key_decode(payload, options, &blob);
374 if (rc) {
375 /* old form */
376 blob = payload->blob;
^^^^^^^^^^^^^^^^^^^^

377 payload->old_format = 1;
378 }
379

regards,
dan carpenter

\
 
 \ /
  Last update: 2021-07-26 10:53    [W:0.071 / U:0.156 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site