Skip to contentSkip to conversation

security/integrity/digsig.c:74:1: warning: 'shash_digest' uses dynamic stack

PATCH In review Fengguang Wu 3 participants 3 messages Sep 13, 2012 Kernel janitors

lore
Share & tools
lore

Fengguang Wu

author
#0 lore raw

PATCH 47/64 security/integrity/digsig.c:74:1: warning: 'shash_digest' uses dynamic stack

Hi Dmitry,

I'm not sure if the warned 'dynamic stack allocation' is relevant, but as an FYI:

tree:   git://git.kernel.org/pub/scm/linux/kernel/git/kasatkin/linux-digsig.git working
head:   ca88a6aa648a167b6f8e5666518249635510b0b0
commit: c6a28f9ee92fe96c5a7c4e5989eb7ae06b545469 [47/64] integrity: added digest calculation function
config: s390-allyesconfig

All error/warnings:

security/integrity/digsig.c: In function 'shash_digest':
security/integrity/digsig.c:74:1: warning: 'shash_digest' uses dynamic stack allocation [enabled by default]

vim +74 security/integrity/digsig.c

    64			char ctx[crypto_shash_descsize(tfm)];
    65		} desc;
    66	
    67		desc.shash.tfm = tfm;
    68		desc.shash.flags = 0;
    69	
    70		rc = crypto_shash_digest(&desc.shash, data, len, digest);
    71		if (!rc)
    72			rc = crypto_shash_digestsize(tfm);
    73		return rc;
Quoted 1 line

74 }

75
76 /**
77 * integrity_calc_digest() - digest calculation
78 * @algo: digest algo id (enum digest_algo)
79 * @data: data
80 * @len: length of the data
81 * @digest: digest
82 * @return: digest length on success, -EINVAL otherwise
83 *
84 * Calculates digest of the data.

---
0-DAY kernel build testing backend Open Source Technology Centre
Fengguang Wu <[email protected]> Intel Corporation

#1 lore raw

Re: [digsig:working 47/64] security/integrity/digsig.c:74:1: warning: 'shash_digest' uses dynamic st

Hi,

This is acceptable case for crypto stuff....

- Dmitry

Quoted 41 lines · Fengguang Wu

Hi Dmitry,

I'm not sure if the warned 'dynamic stack allocation' is relevant, but as an FYI:

tree:   git://git.kernel.org/pub/scm/linux/kernel/git/kasatkin/linux-digsig.git working
head:   ca88a6aa648a167b6f8e5666518249635510b0b0
commit: c6a28f9ee92fe96c5a7c4e5989eb7ae06b545469 [47/64] integrity: added digest calculation function
config: s390-allyesconfig

All error/warnings:

security/integrity/digsig.c: In function 'shash_digest':
security/integrity/digsig.c:74:1: warning: 'shash_digest' uses dynamic stack allocation [enabled by default]

vim +74 security/integrity/digsig.c

    64                  char ctx[crypto_shash_descsize(tfm)];
    65          } desc;
    66
    67          desc.shash.tfm = tfm;
    68          desc.shash.flags = 0;
    69
    70          rc = crypto_shash_digest(&desc.shash, data, len, digest);
    71          if (!rc)
    72                  rc = crypto_shash_digestsize(tfm);
    73          return rc;
Quoted 1 line

74 }

75
76 /**
77 * integrity_calc_digest() - digest calculation
78 * @algo: digest algo id (enum digest_algo)
79 * @data: data
80 * @len: length of the data
81 * @digest: digest
82 * @return: digest length on success, -EINVAL otherwise
83 *
84 * Calculates digest of the data.

---
0-DAY kernel build testing backend Open Source Technology Centre
Fengguang Wu <[email protected]> Intel Corporation

#2 lore raw

Re: [digsig:working 47/64] security/integrity/digsig.c:74:1: warning: 'shash_digest' uses dynamic st

In reply to Fengguang Wu
Quoted 10 lines · Fengguang Wu

Hi Dmitry,

I'm not sure if the warned 'dynamic stack allocation' is relevant, but as an FYI:

tree:   git://git.kernel.org/pub/scm/linux/kernel/git/kasatkin/linux-digsig.git working
head:   ca88a6aa648a167b6f8e5666518249635510b0b0
commit: c6a28f9ee92fe96c5a7c4e5989eb7ae06b545469 [47/64] integrity: added digest calculation function
config: s390-allyesconfig
Quoted 7 lines

All error/warnings:

security/integrity/digsig.c: In function 'shash_digest':
security/integrity/digsig.c:74:1: warning: 'shash_digest' uses dynamic stack allocation [enabled by default]

vim +74 security/integrity/digsig.c

    64			char ctx[crypto_shash_descsize(tfm)];
                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^

Where it wouldn't be relavent is if the size of the can be made to overflow the 8k kernel's stack. We've seen places where this could be controlled by the user before.

Also it's not ok inside a for loop. On some arches the dynamic allocations aren't freed at the end of the loop, only at the end of the function. So inside a for loop the stack keeps getting bigger and bigger until it overflows the 8k stack. (Doesn't happen on x86).

regards,
dan carpenter