| Message ID | 20210610094952.17068-1-find.dhiraj@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tpm2_load_command leaks memory | expand |
Hi Dhiraj,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on v5.13-rc6]
[also build test ERROR on next-20210616]
[cannot apply to security/next-testing]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Dhiraj-Shah/tpm2_load_command-leaks-memory/20210616-184020
base: 009c9aa5be652675a06d5211e1640e02bbb1c33d
config: s390-randconfig-r014-20210615 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 64720f57bea6a6bf033feef4a5751ab9c0c3b401)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# https://github.com/0day-ci/linux/commit/985c6fcde5d80fed97392f94b906e6b43c164f47
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Dhiraj-Shah/tpm2_load_command-leaks-memory/20210616-184020
git checkout 985c6fcde5d80fed97392f94b906e6b43c164f47
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
>> security/keys/trusted-keys/trusted_tpm2.c:426:3: warning: misleading indentation; statement is not part of the previous 'if' [-Wmisleading-indentation]
goto out;
^
security/keys/trusted-keys/trusted_tpm2.c:424:2: note: previous statement is here
if (tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD) != 0)
^
>> security/keys/trusted-keys/trusted_tpm2.c:426:8: error: use of undeclared label 'out'
goto out;
^
>> security/keys/trusted-keys/trusted_tpm2.c:383:8: error: use of undeclared label 'err'
goto err;
^
>> security/keys/trusted-keys/trusted_tpm2.c:428:21: error: expected parameter declarator
tpm_buf_append_u32(&buf, options->keyhandle);
^
>> security/keys/trusted-keys/trusted_tpm2.c:428:21: error: expected ')'
security/keys/trusted-keys/trusted_tpm2.c:428:20: note: to match this '('
tpm_buf_append_u32(&buf, options->keyhandle);
^
>> security/keys/trusted-keys/trusted_tpm2.c:428:2: warning: declaration specifier missing, defaulting to 'int'
tpm_buf_append_u32(&buf, options->keyhandle);
^
int
>> security/keys/trusted-keys/trusted_tpm2.c:428:20: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
tpm_buf_append_u32(&buf, options->keyhandle);
^
void
>> security/keys/trusted-keys/trusted_tpm2.c:428:2: error: conflicting types for 'tpm_buf_append_u32'
tpm_buf_append_u32(&buf, options->keyhandle);
^
include/linux/tpm.h:394:20: note: previous definition is here
static inline void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
^
security/keys/trusted-keys/trusted_tpm2.c:429:23: error: expected parameter declarator
tpm2_buf_append_auth(&buf, TPM2_RS_PW,
^
security/keys/trusted-keys/trusted_tpm2.c:429:23: error: expected ')'
security/keys/trusted-keys/trusted_tpm2.c:429:22: note: to match this '('
tpm2_buf_append_auth(&buf, TPM2_RS_PW,
^
security/keys/trusted-keys/trusted_tpm2.c:429:2: warning: declaration specifier missing, defaulting to 'int'
tpm2_buf_append_auth(&buf, TPM2_RS_PW,
^
int
security/keys/trusted-keys/trusted_tpm2.c:429:22: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
tpm2_buf_append_auth(&buf, TPM2_RS_PW,
^
>> security/keys/trusted-keys/trusted_tpm2.c:429:2: error: conflicting types for 'tpm2_buf_append_auth'
tpm2_buf_append_auth(&buf, TPM2_RS_PW,
^
security/keys/trusted-keys/trusted_tpm2.c:199:13: note: previous definition is here
static void tpm2_buf_append_auth(struct tpm_buf *buf, u32 session_handle,
^
security/keys/trusted-keys/trusted_tpm2.c:435:17: error: expected parameter declarator
tpm_buf_append(&buf, blob, blob_len);
^
security/keys/trusted-keys/trusted_tpm2.c:435:17: error: expected ')'
security/keys/trusted-keys/trusted_tpm2.c:435:16: note: to match this '('
tpm_buf_append(&buf, blob, blob_len);
^
security/keys/trusted-keys/trusted_tpm2.c:435:2: warning: declaration specifier missing, defaulting to 'int'
tpm_buf_append(&buf, blob, blob_len);
^
int
security/keys/trusted-keys/trusted_tpm2.c:435:16: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
tpm_buf_append(&buf, blob, blob_len);
^
void
>> security/keys/trusted-keys/trusted_tpm2.c:435:2: error: conflicting types for 'tpm_buf_append'
tpm_buf_append(&buf, blob, blob_len);
^
include/linux/tpm.h:361:20: note: previous definition is here
static inline void tpm_buf_append(struct tpm_buf *buf,
^
>> security/keys/trusted-keys/trusted_tpm2.c:437:2: error: expected identifier or '('
if (buf.flags & TPM_BUF_OVERFLOW) {
^
security/keys/trusted-keys/trusted_tpm2.c:442:2: warning: declaration specifier missing, defaulting to 'int'
rc = tpm_transmit_cmd(chip, &buf, 4, "loading blob");
^
int
>> security/keys/trusted-keys/trusted_tpm2.c:442:24: error: use of undeclared identifier 'chip'
rc = tpm_transmit_cmd(chip, &buf, 4, "loading blob");
^
>> security/keys/trusted-keys/trusted_tpm2.c:442:31: error: use of undeclared identifier 'buf'
rc = tpm_transmit_cmd(chip, &buf, 4, "loading blob");
^
security/keys/trusted-keys/trusted_tpm2.c:443:2: error: expected identifier or '('
if (!rc)
^
security/keys/trusted-keys/trusted_tpm2.c:447:1: warning: declaration specifier missing, defaulting to 'int'
out:
^
int
>> security/keys/trusted-keys/trusted_tpm2.c:447:4: error: expected ';' after top level declarator
out:
^
;
fatal error: too many errors emitted, stopping now [-ferror-limit=]
6 warnings and 20 errors generated.
vim +/out +426 security/keys/trusted-keys/trusted_tpm2.c
346
347 /**
348 * tpm2_load_cmd() - execute a TPM2_Load command
349 *
350 * @chip: TPM chip to use
351 * @payload: the key data in clear and encrypted form
352 * @options: authentication values and other options
353 * @blob_handle: returned blob handle
354 *
355 * Return: 0 on success.
356 * -E2BIG on wrong payload size.
357 * -EPERM on tpm error status.
358 * < 0 error from tpm_send.
359 */
360 static int tpm2_load_cmd(struct tpm_chip *chip,
361 struct trusted_key_payload *payload,
362 struct trusted_key_options *options,
363 u32 *blob_handle)
364 {
365 struct tpm_buf buf;
366 unsigned int private_len;
367 unsigned int public_len;
368 unsigned int blob_len;
369 u8 *blob, *pub;
370 int rc;
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
380 /* new format carries keyhandle but old format doesn't */
381 if (!options->keyhandle) {
382 rc = -EINVAL;
> 383 goto err;
384 }
385
386 /* must be big enough for at least the two be16 size counts */
387 if (payload->blob_len < 4) {
388 rc = -EINVAL;
389 goto err;
390 }
391
392 private_len = get_unaligned_be16(blob);
393
394 /* must be big enough for following public_len */
395 if (private_len + 2 + 2 > (payload->blob_len)) {
396 rc = -E2BIG;
397 goto err;
398 }
399
400 public_len = get_unaligned_be16(blob + 2 + private_len);
401
402 if (private_len + 2 + public_len + 2 > payload->blob_len) {
403 rc = -E2BIG;
404 goto err;
405 }
406
407 pub = blob + 2 + private_len + 2;
408 /* key attributes are always at offset 4 */
409 attrs = get_unaligned_be32(pub + 4);
410
411 if ((attrs & (TPM2_OA_FIXED_TPM | TPM2_OA_FIXED_PARENT)) ==
412 (TPM2_OA_FIXED_TPM | TPM2_OA_FIXED_PARENT))
413 payload->migratable = 0;
414 else
415 payload->migratable = 1;
416
417 blob_len = private_len + public_len + 4;
418
419 if (blob_len > payload->blob_len) {
420 rc = -E2BIG;
421 goto err;
422 }
423
> 424 if (tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD) != 0)
425 rc = -ENOMEM;
> 426 goto out;
427 }
> 428 tpm_buf_append_u32(&buf, options->keyhandle);
> 429 tpm2_buf_append_auth(&buf, TPM2_RS_PW,
430 NULL /* nonce */, 0,
431 0 /* session_attributes */,
432 options->keyauth /* hmac */,
433 TPM_DIGEST_SIZE);
434
> 435 tpm_buf_append(&buf, blob, blob_len);
436
> 437 if (buf.flags & TPM_BUF_OVERFLOW) {
438 rc = -E2BIG;
439 goto out;
440 }
441
> 442 rc = tpm_transmit_cmd(chip, &buf, 4, "loading blob");
> 443 if (!rc)
444 *blob_handle = be32_to_cpup(
445 (__be32 *) &buf.data[TPM_HEADER_SIZE]);
446
> 447 out:
448 tpm_buf_destroy(&buf);
449 err:
450 if (blob != payload->blob)
451 kfree(blob);
452
453 if (rc > 0)
454 rc = -EPERM;
455
456 return rc;
457 }
458
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Thu, 2021-06-10 at 10:49 +0100, Dhiraj Shah wrote: > tpm2_key_decode allocates memory which is stored in blob and it's not > freed. > > Signed-off-by: Dhiraj Shah <find.dhiraj@gmail.com> > --- > security/keys/trusted-keys/trusted_tpm2.c | 41 +++++++++++++++---- > ---- > 1 file changed, 27 insertions(+), 14 deletions(-) > > diff --git a/security/keys/trusted-keys/trusted_tpm2.c > b/security/keys/trusted-keys/trusted_tpm2.c > index 0165da386289..52dd43bb8cdb 100644 > --- a/security/keys/trusted-keys/trusted_tpm2.c > +++ b/security/keys/trusted-keys/trusted_tpm2.c > @@ -378,22 +378,31 @@ static int tpm2_load_cmd(struct tpm_chip *chip, > } > > /* new format carries keyhandle but old format doesn't */ > - if (!options->keyhandle) > - return -EINVAL; > + if (!options->keyhandle) { > + rc = -EINVAL; > + goto err; > + } This one is unnecessary ... for the old format there's nothing to free. > /* must be big enough for at least the two be16 size counts */ > - if (payload->blob_len < 4) > - return -EINVAL; > + if (payload->blob_len < 4) { > + rc = -EINVAL; > + goto err; > + } > > private_len = get_unaligned_be16(blob); > > /* must be big enough for following public_len */ > - if (private_len + 2 + 2 > (payload->blob_len)) > - return -E2BIG; > + if (private_len + 2 + 2 > (payload->blob_len)) { > + rc = -E2BIG; > + goto err; > + } > > public_len = get_unaligned_be16(blob + 2 + private_len); > - if (private_len + 2 + public_len + 2 > payload->blob_len) > - return -E2BIG; > + > + if (private_len + 2 + public_len + 2 > payload->blob_len) { > + rc = -E2BIG; > + goto err; > + } > > pub = blob + 2 + private_len + 2; > /* key attributes are always at offset 4 */ > @@ -406,13 +415,16 @@ static int tpm2_load_cmd(struct tpm_chip *chip, > payload->migratable = 1; > > blob_len = private_len + public_len + 4; > - if (blob_len > payload->blob_len) > - return -E2BIG; > > - rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD); > - if (rc) > - return rc; > + if (blob_len > payload->blob_len) { > + rc = -E2BIG; > + goto err; > + } > > + if (tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD) != 0) You didn't compile this, did you? There's no opening brace here ... James
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c index 0165da386289..52dd43bb8cdb 100644 --- a/security/keys/trusted-keys/trusted_tpm2.c +++ b/security/keys/trusted-keys/trusted_tpm2.c @@ -378,22 +378,31 @@ static int tpm2_load_cmd(struct tpm_chip *chip, } /* new format carries keyhandle but old format doesn't */ - if (!options->keyhandle) - return -EINVAL; + if (!options->keyhandle) { + rc = -EINVAL; + goto err; + } /* must be big enough for at least the two be16 size counts */ - if (payload->blob_len < 4) - return -EINVAL; + if (payload->blob_len < 4) { + rc = -EINVAL; + goto err; + } private_len = get_unaligned_be16(blob); /* must be big enough for following public_len */ - if (private_len + 2 + 2 > (payload->blob_len)) - return -E2BIG; + if (private_len + 2 + 2 > (payload->blob_len)) { + rc = -E2BIG; + goto err; + } public_len = get_unaligned_be16(blob + 2 + private_len); - if (private_len + 2 + public_len + 2 > payload->blob_len) - return -E2BIG; + + if (private_len + 2 + public_len + 2 > payload->blob_len) { + rc = -E2BIG; + goto err; + } pub = blob + 2 + private_len + 2; /* key attributes are always at offset 4 */ @@ -406,13 +415,16 @@ static int tpm2_load_cmd(struct tpm_chip *chip, payload->migratable = 1; blob_len = private_len + public_len + 4; - if (blob_len > payload->blob_len) - return -E2BIG; - rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD); - if (rc) - return rc; + if (blob_len > payload->blob_len) { + rc = -E2BIG; + goto err; + } + if (tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD) != 0) + rc = -ENOMEM; + goto out; + } tpm_buf_append_u32(&buf, options->keyhandle); tpm2_buf_append_auth(&buf, TPM2_RS_PW, NULL /* nonce */, 0, @@ -433,9 +445,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip, (__be32 *) &buf.data[TPM_HEADER_SIZE]); out: + tpm_buf_destroy(&buf); +err: if (blob != payload->blob) kfree(blob); - tpm_buf_destroy(&buf); if (rc > 0) rc = -EPERM;
tpm2_key_decode allocates memory which is stored in blob and it's not freed. Signed-off-by: Dhiraj Shah <find.dhiraj@gmail.com> --- security/keys/trusted-keys/trusted_tpm2.c | 41 +++++++++++++++-------- 1 file changed, 27 insertions(+), 14 deletions(-)