| Message ID | 20190802150733.1972-1-roberto.sassu@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] KEYS: trusted: allow module init if TPM is inactive or deactivated | expand |
On 2019-08-02 17:07:33, Roberto Sassu wrote: > Commit c78719203fc6 ("KEYS: trusted: allow trusted.ko to initialize w/o a > TPM") allows the trusted module to be loaded even a TPM is not found to ^ if > avoid module dependency problems. > > However, trusted module initialization can still fail if the TPM is > inactive or deactivated. This patch ignores tpm_get_random() errors in > init_digests() and returns -EFAULT in pcrlock() if the TPM didn't return > random data. > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> The code changes look correct to me. Reviewed-by: Tyler Hicks <tyhicks@canonical.com> For whoever takes this patch through their tree, I think that adding the following Fixes tag would be useful (as well as cc'ing stable): Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") I think it is also worth leaving a short note, in the commit message, for backporters that commit 782779b60faa ("tpm: Actually fail on TPM errors during "get random"") should be included with any backports of this patch. Thanks! Tyler > --- > security/keys/trusted.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/security/keys/trusted.c b/security/keys/trusted.c > index 9a94672e7adc..34f04ffcf2e5 100644 > --- a/security/keys/trusted.c > +++ b/security/keys/trusted.c > @@ -389,6 +389,10 @@ static int pcrlock(const int pcrnum) > if (!capable(CAP_SYS_ADMIN)) > return -EPERM; > > + /* This happens if the TPM didn't return random data */ > + if (!digests) > + return -EFAULT; > + > return tpm_pcr_extend(chip, pcrnum, digests) ? -EINVAL : 0; > } > > @@ -1233,10 +1237,8 @@ static int __init init_digests(void) > int i; > > ret = tpm_get_random(chip, digest, TPM_MAX_DIGEST_SIZE); > - if (ret < 0) > - return ret; > - if (ret < TPM_MAX_DIGEST_SIZE) > - return -EFAULT; > + if (ret < 0 || ret < TPM_MAX_DIGEST_SIZE) > + return 0; > > digests = kcalloc(chip->nr_allocated_banks, sizeof(*digests), > GFP_KERNEL); > -- > 2.17.1 >
On 8/2/2019 5:30 PM, Tyler Hicks wrote: > On 2019-08-02 17:07:33, Roberto Sassu wrote: >> Commit c78719203fc6 ("KEYS: trusted: allow trusted.ko to initialize w/o a >> TPM") allows the trusted module to be loaded even a TPM is not found to > ^ if > >> avoid module dependency problems. >> >> However, trusted module initialization can still fail if the TPM is >> inactive or deactivated. This patch ignores tpm_get_random() errors in >> init_digests() and returns -EFAULT in pcrlock() if the TPM didn't return >> random data. >> >> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > > The code changes look correct to me. > > Reviewed-by: Tyler Hicks <tyhicks@canonical.com> > > For whoever takes this patch through their tree, I think that adding the > following Fixes tag would be useful (as well as cc'ing stable): > > Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") > > I think it is also worth leaving a short note, in the commit message, > for backporters that commit 782779b60faa ("tpm: Actually fail on TPM > errors during "get random"") should be included with any backports of > this patch. Right, thanks. I wait for Jarkko's comments and I add both the Fixes tag and the short note in the next version of the patch. Roberto
On 8/2/2019 5:34 PM, Roberto Sassu wrote: > On 8/2/2019 5:30 PM, Tyler Hicks wrote: >> On 2019-08-02 17:07:33, Roberto Sassu wrote: >>> Commit c78719203fc6 ("KEYS: trusted: allow trusted.ko to initialize >>> w/o a >>> TPM") allows the trusted module to be loaded even a TPM is not found to >> ^ if >> >>> avoid module dependency problems. >>> >>> However, trusted module initialization can still fail if the TPM is >>> inactive or deactivated. This patch ignores tpm_get_random() errors in >>> init_digests() and returns -EFAULT in pcrlock() if the TPM didn't return >>> random data. >>> >>> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> >> >> The code changes look correct to me. >> >> Reviewed-by: Tyler Hicks <tyhicks@canonical.com> >> >> For whoever takes this patch through their tree, I think that adding the >> following Fixes tag would be useful (as well as cc'ing stable): >> >> Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip >> structure...") >> >> I think it is also worth leaving a short note, in the commit message, >> for backporters that commit 782779b60faa ("tpm: Actually fail on TPM >> errors during "get random"") should be included with any backports of >> this patch. > > Right, thanks. I wait for Jarkko's comments and I add both the Fixes tag > and the short note in the next version of the patch. Uhm, I was thinking that maybe it is not necessary to mention commit 782779b60faa. This patch would still return 0 even if that commit is not backported (TPM_ERR_DISABLED < TPM_MAX_DIGEST_SIZE). Roberto
On 2019-08-02 18:11:09, Roberto Sassu wrote: > On 8/2/2019 5:34 PM, Roberto Sassu wrote: > > On 8/2/2019 5:30 PM, Tyler Hicks wrote: > > > On 2019-08-02 17:07:33, Roberto Sassu wrote: > > > > Commit c78719203fc6 ("KEYS: trusted: allow trusted.ko to > > > > initialize w/o a > > > > TPM") allows the trusted module to be loaded even a TPM is not found to > > > ^ if > > > > > > > avoid module dependency problems. > > > > > > > > However, trusted module initialization can still fail if the TPM is > > > > inactive or deactivated. This patch ignores tpm_get_random() errors in > > > > init_digests() and returns -EFAULT in pcrlock() if the TPM didn't return > > > > random data. > > > > > > > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > > > > > > The code changes look correct to me. > > > > > > Reviewed-by: Tyler Hicks <tyhicks@canonical.com> > > > > > > For whoever takes this patch through their tree, I think that adding the > > > following Fixes tag would be useful (as well as cc'ing stable): > > > > > > Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip > > > structure...") > > > > > > I think it is also worth leaving a short note, in the commit message, > > > for backporters that commit 782779b60faa ("tpm: Actually fail on TPM > > > errors during "get random"") should be included with any backports of > > > this patch. > > > > Right, thanks. I wait for Jarkko's comments and I add both the Fixes tag > > and the short note in the next version of the patch. > > Uhm, I was thinking that maybe it is not necessary to mention commit > 782779b60faa. This patch would still return 0 even if that commit is not > backported (TPM_ERR_DISABLED < TPM_MAX_DIGEST_SIZE). The commit message for 782779b60faa shows 379 being returned when attempting to get random: [ 18.092103] tpm tpm0: A TPM error (379) occurred attempting get random I don't know enough about TPM chips to know how common that is... Tyler > > Roberto > > -- > HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 > Managing Director: Li Peng, Li Jian, Shi Yanli
diff --git a/security/keys/trusted.c b/security/keys/trusted.c index 9a94672e7adc..34f04ffcf2e5 100644 --- a/security/keys/trusted.c +++ b/security/keys/trusted.c @@ -389,6 +389,10 @@ static int pcrlock(const int pcrnum) if (!capable(CAP_SYS_ADMIN)) return -EPERM; + /* This happens if the TPM didn't return random data */ + if (!digests) + return -EFAULT; + return tpm_pcr_extend(chip, pcrnum, digests) ? -EINVAL : 0; } @@ -1233,10 +1237,8 @@ static int __init init_digests(void) int i; ret = tpm_get_random(chip, digest, TPM_MAX_DIGEST_SIZE); - if (ret < 0) - return ret; - if (ret < TPM_MAX_DIGEST_SIZE) - return -EFAULT; + if (ret < 0 || ret < TPM_MAX_DIGEST_SIZE) + return 0; digests = kcalloc(chip->nr_allocated_banks, sizeof(*digests), GFP_KERNEL);
Commit c78719203fc6 ("KEYS: trusted: allow trusted.ko to initialize w/o a TPM") allows the trusted module to be loaded even a TPM is not found to avoid module dependency problems. However, trusted module initialization can still fail if the TPM is inactive or deactivated. This patch ignores tpm_get_random() errors in init_digests() and returns -EFAULT in pcrlock() if the TPM didn't return random data. Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> --- security/keys/trusted.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)