From: Bernard Zhao bernard@vivo.com
[ Upstream commit 177d3819633cd520e3f95df541a04644aab4c657 ]
In function msm_submitqueue_create, the queue is a local variable, in return -EINVAL branch, queue didn`t add to ctx`s list yet, and also didn`t kfree, this maybe bring in potential memleak.
Signed-off-by: Bernard Zhao bernard@vivo.com [trivial commit msg fixup] Signed-off-by: Rob Clark robdclark@chromium.org Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/gpu/drm/msm/msm_submitqueue.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/msm_submitqueue.c b/drivers/gpu/drm/msm/msm_submitqueue.c index 001fbf537440a..a1d94be7883a0 100644 --- a/drivers/gpu/drm/msm/msm_submitqueue.c +++ b/drivers/gpu/drm/msm/msm_submitqueue.c @@ -71,8 +71,10 @@ int msm_submitqueue_create(struct drm_device *drm, struct msm_file_private *ctx, queue->flags = flags;
if (priv->gpu) { - if (prio >= priv->gpu->nr_rings) + if (prio >= priv->gpu->nr_rings) { + kfree(queue); return -EINVAL; + }
queue->prio = prio; }
From: Krishna Manikandan mkrishn@codeaurora.org
[ Upstream commit 2e7ec6b5297157efabb50e5f82adc628cf90296c ]
In the current implementation, mutex initialization for encoder mutex locks are done during encoder setup. This can lead to scenarios where the lock is used before it is initialized. Move mutex_init to dpu_encoder_init to avoid this.
Signed-off-by: Krishna Manikandan mkrishn@codeaurora.org Signed-off-by: Rob Clark robdclark@chromium.org Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index a1b79ee2bd9d5..aa5409f06e3f9 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -2126,7 +2126,6 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc,
dpu_enc = to_dpu_encoder_virt(enc);
- mutex_init(&dpu_enc->enc_lock); ret = dpu_encoder_setup_display(dpu_enc, dpu_kms, disp_info); if (ret) goto fail; @@ -2141,7 +2140,6 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc, 0);
- mutex_init(&dpu_enc->rc_lock); INIT_DELAYED_WORK(&dpu_enc->delayed_off_work, dpu_encoder_off_work); dpu_enc->idle_timeout = IDLE_TIMEOUT; @@ -2186,6 +2184,8 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev,
spin_lock_init(&dpu_enc->enc_spinlock); dpu_enc->enabled = false; + mutex_init(&dpu_enc->enc_lock); + mutex_init(&dpu_enc->rc_lock);
return &dpu_enc->base; }
From: Maurizio Drocco maurizio.drocco@ibm.com
[ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ]
Registers 8-9 are used to store measurements of the kernel and its command line (e.g., grub2 bootloader with tpm module enabled). IMA should include them in the boot aggregate. Registers 8-9 should be only included in non-SHA1 digests to avoid ambiguity.
Signed-off-by: Maurizio Drocco maurizio.drocco@ibm.com Reviewed-by: Bruno Meneguele bmeneg@redhat.com Tested-by: Bruno Meneguele bmeneg@redhat.com (TPM 1.2, TPM 2.0) Signed-off-by: Mimi Zohar zohar@linux.ibm.com Signed-off-by: Sasha Levin sashal@kernel.org --- security/integrity/ima/ima.h | 2 +- security/integrity/ima/ima_crypto.c | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 495e28bd488e6..844a55225ede0 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -30,7 +30,7 @@
enum ima_show_type { IMA_SHOW_BINARY, IMA_SHOW_BINARY_NO_FIELD_LEN, IMA_SHOW_BINARY_OLD_STRING_FMT, IMA_SHOW_ASCII }; -enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 }; +enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
/* digest size for IMA, fits SHA1 or MD5 */ #define IMA_DIGEST_SIZE SHA1_DIGEST_SIZE diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index fb27174806ba4..e0738d1d143d7 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -682,13 +682,26 @@ static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id, if (rc != 0) return rc;
- /* cumulative sha1 over tpm registers 0-7 */ + /* cumulative digest over TPM registers 0-7 */ for (i = TPM_PCR0; i < TPM_PCR8; i++) { ima_pcrread(i, &d); /* now accumulate with current aggregate */ rc = crypto_shash_update(shash, d.digest, crypto_shash_digestsize(tfm)); } + /* + * Extend cumulative digest over TPM registers 8-9, which contain + * measurement for the kernel command line (reg. 8) and image (reg. 9) + * in a typical PCR allocation. Registers 8-9 are only included in + * non-SHA1 boot_aggregate digests to avoid ambiguity. + */ + if (alg_id != TPM_ALG_SHA1) { + for (i = TPM_PCR8; i < TPM_PCR10; i++) { + ima_pcrread(i, &d); + rc = crypto_shash_update(shash, d.digest, + crypto_shash_digestsize(tfm)); + } + } if (!rc) crypto_shash_final(shash, digest); return rc;
From: Marek Szyprowski m.szyprowski@samsung.com
[ Upstream commit b9c633882de4601015625f9136f248e9abca8a7a ]
Propagate the proper error codes from the called functions instead of unconditionally returning 0.
Reported-by: kbuild test robot lkp@intel.com Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com Merge conflict so merged it manually. Signed-off-by: Inki Dae inki.dae@samsung.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/gpu/drm/exynos/exynos_drm_dma.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dma.c b/drivers/gpu/drm/exynos/exynos_drm_dma.c index 619f81435c1b2..58b89ec11b0eb 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dma.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dma.c @@ -61,7 +61,7 @@ static int drm_iommu_attach_device(struct drm_device *drm_dev, struct device *subdrv_dev, void **dma_priv) { struct exynos_drm_private *priv = drm_dev->dev_private; - int ret; + int ret = 0;
if (get_dma_ops(priv->dma_dev) != get_dma_ops(subdrv_dev)) { DRM_DEV_ERROR(subdrv_dev, "Device %s lacks support for IOMMU\n", @@ -92,7 +92,7 @@ static int drm_iommu_attach_device(struct drm_device *drm_dev, if (ret) clear_dma_max_seg_size(subdrv_dev);
- return 0; + return ret; }
/*
From: Navid Emamdoost navid.emamdoost@gmail.com
[ Upstream commit d4f5a095daf0d25f0b385e1ef26338608433a4c5 ]
in mic_pre_enable, pm_runtime_get_sync is called which increments the counter even in case of failure, leading to incorrect ref count. In case of failure, decrement the ref count before returning.
Signed-off-by: Navid Emamdoost navid.emamdoost@gmail.com Signed-off-by: Inki Dae inki.dae@samsung.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/gpu/drm/exynos/exynos_drm_mic.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c b/drivers/gpu/drm/exynos/exynos_drm_mic.c index f41d75923557a..004110c5ded42 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_mic.c +++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c @@ -269,8 +269,10 @@ static void mic_pre_enable(struct drm_bridge *bridge) goto unlock;
ret = pm_runtime_get_sync(mic->dev); - if (ret < 0) + if (ret < 0) { + pm_runtime_put_noidle(mic->dev); goto unlock; + }
mic_set_path(mic, 1);
From: Petteri Aimonen jpa@git.mail.kapsi.fi
[ Upstream commit 7ad816762f9bf89e940e618ea40c43138b479e10 ]
Previously, kernel floating point code would run with the MXCSR control register value last set by userland code by the thread that was active on the CPU core just before kernel call. This could affect calculation results if rounding mode was changed, or a crash if a FPU/SIMD exception was unmasked.
Restore MXCSR to the kernel's default value.
[ bp: Carve out from a bigger patch by Petteri, add feature check, add FNINIT call too (amluto). ]
Signed-off-by: Petteri Aimonen jpa@git.mail.kapsi.fi Signed-off-by: Borislav Petkov bp@suse.de Link: https://bugzilla.kernel.org/show_bug.cgi?id=207979 Link: https://lkml.kernel.org/r/20200624114646.28953-2-bp@alien8.de Signed-off-by: Sasha Levin sashal@kernel.org --- arch/x86/include/asm/fpu/internal.h | 5 +++++ arch/x86/kernel/fpu/core.c | 6 ++++++ 2 files changed, 11 insertions(+)
diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h index 44c48e34d7994..00eac7f1529b0 100644 --- a/arch/x86/include/asm/fpu/internal.h +++ b/arch/x86/include/asm/fpu/internal.h @@ -619,6 +619,11 @@ static inline void switch_fpu_finish(struct fpu *new_fpu) * MXCSR and XCR definitions: */
+static inline void ldmxcsr(u32 mxcsr) +{ + asm volatile("ldmxcsr %0" :: "m" (mxcsr)); +} + extern unsigned int mxcsr_feature_mask;
#define XCR_XFEATURE_ENABLED_MASK 0x00000000 diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c index 12c70840980e4..cd8839027f66d 100644 --- a/arch/x86/kernel/fpu/core.c +++ b/arch/x86/kernel/fpu/core.c @@ -101,6 +101,12 @@ void kernel_fpu_begin(void) copy_fpregs_to_fpstate(¤t->thread.fpu); } __cpu_invalidate_fpregs_state(); + + if (boot_cpu_has(X86_FEATURE_XMM)) + ldmxcsr(MXCSR_DEFAULT); + + if (boot_cpu_has(X86_FEATURE_FPU)) + asm volatile ("fninit"); } EXPORT_SYMBOL_GPL(kernel_fpu_begin);
From: "Hyeongseok.Kim" Hyeongseok@gmail.com
[ Upstream commit 4ba6ccd695f5ed3ae851e59b443b757bbe4557fe ]
Some fsck tool complain that padding part of the FileName field is not set to the value 0000h. So let's maintain filesystem cleaner, as exfat's spec. recommendation.
Signed-off-by: Hyeongseok.Kim Hyeongseok@gmail.com Reviewed-by: Sungjong Seo sj1557.seo@samsung.com Signed-off-by: Namjae Jeon namjae.jeon@samsung.com Signed-off-by: Sasha Levin sashal@kernel.org --- fs/exfat/dir.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c index 4b91afb0f0515..349ca0c282c2c 100644 --- a/fs/exfat/dir.c +++ b/fs/exfat/dir.c @@ -430,10 +430,12 @@ static void exfat_init_name_entry(struct exfat_dentry *ep, ep->dentry.name.flags = 0x0;
for (i = 0; i < EXFAT_FILE_NAME_LEN; i++) { - ep->dentry.name.unicode_0_14[i] = cpu_to_le16(*uniname); - if (*uniname == 0x0) - break; - uniname++; + if (*uniname != 0x0) { + ep->dentry.name.unicode_0_14[i] = cpu_to_le16(*uniname); + uniname++; + } else { + ep->dentry.name.unicode_0_14[i] = 0x0; + } } }
From: Hyunchul Lee hyc.lee@gmail.com
[ Upstream commit a0271a15cf2cf907ea5b0f2ba611123f1b7935ec ]
We need to commit dirty metadata and pages to disk before remounting exfat as read-only.
This fixes a failure in xfstests generic/452
generic/452 does the following: cp something <exfat>/ mount -o remount,ro <exfat>
the <exfat>/something is corrupted. because while exfat is remounted as read-only, exfat doesn't have a chance to commit metadata and vfs invalidates page caches in a block device.
Signed-off-by: Hyunchul Lee hyc.lee@gmail.com Acked-by: Sungjong Seo sj1557.seo@samsung.com Signed-off-by: Namjae Jeon namjae.jeon@samsung.com Signed-off-by: Sasha Levin sashal@kernel.org --- fs/exfat/super.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/fs/exfat/super.c b/fs/exfat/super.c index c1b1ed306a485..e879801533980 100644 --- a/fs/exfat/super.c +++ b/fs/exfat/super.c @@ -637,10 +637,20 @@ static void exfat_free(struct fs_context *fc) } }
+static int exfat_reconfigure(struct fs_context *fc) +{ + fc->sb_flags |= SB_NODIRATIME; + + /* volume flag will be updated in exfat_sync_fs */ + sync_filesystem(fc->root->d_sb); + return 0; +} + static const struct fs_context_operations exfat_context_ops = { .parse_param = exfat_parse_param, .get_tree = exfat_get_tree, .free = exfat_free, + .reconfigure = exfat_reconfigure, };
static int exfat_init_fs_context(struct fs_context *fc)
From: Anson Huang Anson.Huang@nxp.com
[ Upstream commit b45fd13be340e4ed0a2a9673ba299eb2a71ba829 ]
After finishing using cpu node got from of_get_cpu_node(), of_node_put() needs to be called.
Signed-off-by: Anson Huang Anson.Huang@nxp.com Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org Link: https://lore.kernel.org/r/1585232945-23368-1-git-send-email-Anson.Huang@nxp.... Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/thermal/imx_thermal.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c index e761c9b422179..1b84ea674edb7 100644 --- a/drivers/thermal/imx_thermal.c +++ b/drivers/thermal/imx_thermal.c @@ -649,7 +649,7 @@ MODULE_DEVICE_TABLE(of, of_imx_thermal_match); static int imx_thermal_register_legacy_cooling(struct imx_thermal_data *data) { struct device_node *np; - int ret; + int ret = 0;
data->policy = cpufreq_cpu_get(0); if (!data->policy) { @@ -664,11 +664,12 @@ static int imx_thermal_register_legacy_cooling(struct imx_thermal_data *data) if (IS_ERR(data->cdev)) { ret = PTR_ERR(data->cdev); cpufreq_cpu_put(data->policy); - return ret; } }
- return 0; + of_node_put(np); + + return ret; }
static void imx_thermal_unregister_legacy_cooling(struct imx_thermal_data *data)
From: Srinivas Pandruvada srinivas.pandruvada@linux.intel.com
[ Upstream commit 1e05daca83bb42cde569f75f3bd7c8828b1ef30f ]
Add DPTF battery participant ACPI ID for platforms based on the Intel TigerLake SoC.
Signed-off-by: Srinivas Pandruvada srinivas.pandruvada@linux.intel.com [ rjw: Changelog ] Signed-off-by: Rafael J. Wysocki rafael.j.wysocki@intel.com Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/acpi/dptf/dptf_power.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/acpi/dptf/dptf_power.c b/drivers/acpi/dptf/dptf_power.c index e4e8b75d39f09..8b42f529047e9 100644 --- a/drivers/acpi/dptf/dptf_power.c +++ b/drivers/acpi/dptf/dptf_power.c @@ -99,6 +99,7 @@ static int dptf_power_remove(struct platform_device *pdev) static const struct acpi_device_id int3407_device_ids[] = { {"INT3407", 0}, {"INTC1047", 0}, + {"INTC1050", 0}, {"", 0}, }; MODULE_DEVICE_TABLE(acpi, int3407_device_ids);
From: Hou Tao houtao1@huawei.com
[ Upstream commit bfe373f608cf81b7626dfeb904001b0e867c5110 ]
Else there may be magic numbers in /sys/kernel/debug/block/*/state.
Signed-off-by: Hou Tao houtao1@huawei.com Reviewed-by: Bart Van Assche bvanassche@acm.org Signed-off-by: Jens Axboe axboe@kernel.dk Signed-off-by: Sasha Levin sashal@kernel.org --- block/blk-mq-debugfs.c | 3 +++ include/linux/blkdev.h | 1 + 2 files changed, 4 insertions(+)
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c index b3f2ba483992d..121f4c1e0697b 100644 --- a/block/blk-mq-debugfs.c +++ b/block/blk-mq-debugfs.c @@ -125,6 +125,9 @@ static const char *const blk_queue_flag_name[] = { QUEUE_FLAG_NAME(REGISTERED), QUEUE_FLAG_NAME(SCSI_PASSTHROUGH), QUEUE_FLAG_NAME(QUIESCED), + QUEUE_FLAG_NAME(PCI_P2PDMA), + QUEUE_FLAG_NAME(ZONE_RESETALL), + QUEUE_FLAG_NAME(RQ_ALLOC_TIME), }; #undef QUEUE_FLAG_NAME
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 32868fbedc9e9..02809e4dd661e 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -585,6 +585,7 @@ struct request_queue { u64 write_hints[BLK_MAX_WRITE_HINTS]; };
+/* Keep blk_queue_flag_name[] in sync with the definitions below */ #define QUEUE_FLAG_STOPPED 0 /* queue is stopped */ #define QUEUE_FLAG_DYING 1 /* queue being torn down */ #define QUEUE_FLAG_NOMERGES 3 /* disable merge attempts */
From: Mike Rapoport rppt@linux.ibm.com
[ Upstream commit d63bd8c81d8ab64db506ffde569cc8ff197516e2 ]
The m68k nommu setup code didn't register the beginning of the physical memory with memblock because it was anyway occupied by the kernel. However, commit fa3354e4ea39 ("mm: free_area_init: use maximal zone PFNs rather than zone sizes") changed zones initialization to use memblock.memory to detect the zone extents and this caused inconsistency between zone PFNs and the actual PFNs:
BUG: Bad page state in process swapper pfn:20165 page:41fe0ca0 refcount:0 mapcount:1 mapping:00000000 index:0x0 flags: 0x0() raw: 00000000 00000100 00000122 00000000 00000000 00000000 00000000 00000000 page dumped because: nonzero mapcount CPU: 0 PID: 1 Comm: swapper Not tainted 5.8.0-rc1-00001-g3a38f8a60c65-dirty #1 Stack from 404c9ebc: 404c9ebc 4029ab28 4029ab28 40088470 41fe0ca0 40299e21 40299df1 404ba2a4 00020165 00000000 41fd2c10 402c7ba0 41fd2c04 40088504 41fe0ca0 40299e21 00000000 40088a12 41fe0ca0 41fe0ca4 0000020a 00000000 00000001 402ca000 00000000 41fe0ca0 41fd2c10 41fd2c10 00000000 00000000 402b2388 00000001 400a0934 40091056 404c9f44 404c9f44 40088db4 402c7ba0 00000001 41fd2c04 41fe0ca0 41fd2000 41fe0ca0 40089e02 4026ecf4 40089e4e 41fe0ca0 ffffffff Call Trace: [<40088470>] 0x40088470 [<40088504>] 0x40088504 [<40088a12>] 0x40088a12 [<402ca000>] 0x402ca000 [<400a0934>] 0x400a0934
Adjust the memory registration with memblock to include the beginning of the physical memory and make sure that the area occupied by the kernel is marked as reserved.
Signed-off-by: Mike Rapoport rppt@linux.ibm.com Signed-off-by: Greg Ungerer gerg@linux-m68k.org Signed-off-by: Sasha Levin sashal@kernel.org --- arch/m68k/kernel/setup_no.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/m68k/kernel/setup_no.c b/arch/m68k/kernel/setup_no.c index a63483de7a422..5dacba392c74e 100644 --- a/arch/m68k/kernel/setup_no.c +++ b/arch/m68k/kernel/setup_no.c @@ -139,7 +139,8 @@ void __init setup_arch(char **cmdline_p) pr_debug("MEMORY -> ROMFS=0x%p-0x%06lx MEM=0x%06lx-0x%06lx\n ", __bss_stop, memory_start, memory_start, memory_end);
- memblock_add(memory_start, memory_end - memory_start); + memblock_add(_rambase, memory_end - _rambase); + memblock_reserve(_rambase, memory_start - _rambase);
/* Keep a copy of command line */ *cmdline_p = &command_line[0];
From: Angelo Dureghello angelo.dureghello@timesys.com
[ Upstream commit c43e55796dd4d13f4855971a4d7970ce2cd94db4 ]
After pulling 5.7.0 (linux-next merge), mcf5441x mmu boot was hanging silently.
memblock_add() seems not appropriate, since using MAX_NUMNODES as node id, while memblock_add_node() sets up memory for node id 0.
Signed-off-by: Angelo Dureghello angelo.dureghello@timesys.com Signed-off-by: Mike Rapoport rppt@linux.ibm.com Signed-off-by: Greg Ungerer gerg@linux-m68k.org Signed-off-by: Sasha Levin sashal@kernel.org --- arch/m68k/mm/mcfmmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/m68k/mm/mcfmmu.c b/arch/m68k/mm/mcfmmu.c index 0ea375607767c..2c57f46facc0e 100644 --- a/arch/m68k/mm/mcfmmu.c +++ b/arch/m68k/mm/mcfmmu.c @@ -178,7 +178,7 @@ void __init cf_bootmem_alloc(void) m68k_memory[0].addr = _rambase; m68k_memory[0].size = _ramend - _rambase;
- memblock_add(m68k_memory[0].addr, m68k_memory[0].size); + memblock_add_node(m68k_memory[0].addr, m68k_memory[0].size, 0);
/* compute total pages in system */ num_pages = PFN_DOWN(_ramend - _rambase);
Hi Sasha,
On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote:
From: Maurizio Drocco maurizio.drocco@ibm.com
[ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ]
Registers 8-9 are used to store measurements of the kernel and its command line (e.g., grub2 bootloader with tpm module enabled). IMA should include them in the boot aggregate. Registers 8-9 should be only included in non-SHA1 digests to avoid ambiguity.
Prior to Linux 5.8, the SHA1 template data hashes were padded before being extended into the TPM. Â Support for calculating and extending the per TPM bank template data digests is only being upstreamed in Linux 5.8.
How will attestation servers know whether to include PCRs 8 & 9 in the the boot_aggregate calculation? Â Now, there is a direct relationship between the template data SHA1 padded digest not including PCRs 8 & 9, and the new per TPM bank template data digest including them.
Mimi
On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote:
Hi Sasha,
On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote:
From: Maurizio Drocco maurizio.drocco@ibm.com
[ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ]
Registers 8-9 are used to store measurements of the kernel and its command line (e.g., grub2 bootloader with tpm module enabled). IMA should include them in the boot aggregate. Registers 8-9 should be only included in non-SHA1 digests to avoid ambiguity.
Prior to Linux 5.8, the SHA1 template data hashes were padded before being extended into the TPM. Â Support for calculating and extending the per TPM bank template data digests is only being upstreamed in Linux 5.8.
How will attestation servers know whether to include PCRs 8 & 9 in the the boot_aggregate calculation? Â Now, there is a direct relationship between the template data SHA1 padded digest not including PCRs 8 & 9, and the new per TPM bank template data digest including them.
Got it, I'll drop it then, thank you!
Hi Sasha,
On Wed, 2020-07-08 at 21:27 -0400, Sasha Levin wrote:
On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote:
Hi Sasha,
On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote:
From: Maurizio Drocco maurizio.drocco@ibm.com
[ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ]
Registers 8-9 are used to store measurements of the kernel and its command line (e.g., grub2 bootloader with tpm module enabled). IMA should include them in the boot aggregate. Registers 8-9 should be only included in non-SHA1 digests to avoid ambiguity.
Prior to Linux 5.8, the SHA1 template data hashes were padded before being extended into the TPM. Support for calculating and extending the per TPM bank template data digests is only being upstreamed in Linux 5.8.
How will attestation servers know whether to include PCRs 8 & 9 in the the boot_aggregate calculation? Now, there is a direct relationship between the template data SHA1 padded digest not including PCRs 8 & 9, and the new per TPM bank template data digest including them.
Got it, I'll drop it then, thank you!
After re-thinking this over, I realized that the attestation server can verify the "boot_aggregate" based on the quoted PCRs without knowing whether padded SHA1 hashes or per TPM bank hash values were extended into the TPM[1], but non-SHA1 boot aggregate values [2] should always include PCRs 8 & 9.
Any place commit 6f1a1d103b48 was backported [2], this commit 20c59ce010f8 ("ima: extend boot_aggregate with kernel measurements") should be backported as well.
thanks,
Mimi
[1] commit 1ea973df6e21 ("ima: Calculate and extend PCR with digests in ima_template_entry") [2] commit 6f1a1d103b48 ("ima: Switch to ima_hash_algo for boot aggregate")
On Sun, Nov 29, 2020 at 08:17:38AM -0500, Mimi Zohar wrote:
Hi Sasha,
On Wed, 2020-07-08 at 21:27 -0400, Sasha Levin wrote:
On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote:
Hi Sasha,
On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote:
From: Maurizio Drocco maurizio.drocco@ibm.com
[ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ]
Registers 8-9 are used to store measurements of the kernel and its command line (e.g., grub2 bootloader with tpm module enabled). IMA should include them in the boot aggregate. Registers 8-9 should be only included in non-SHA1 digests to avoid ambiguity.
Prior to Linux 5.8, the SHA1 template data hashes were padded before being extended into the TPM. Support for calculating and extending the per TPM bank template data digests is only being upstreamed in Linux 5.8.
How will attestation servers know whether to include PCRs 8 & 9 in the the boot_aggregate calculation? Now, there is a direct relationship between the template data SHA1 padded digest not including PCRs 8 & 9, and the new per TPM bank template data digest including them.
Got it, I'll drop it then, thank you!
After re-thinking this over, I realized that the attestation server can verify the "boot_aggregate" based on the quoted PCRs without knowing whether padded SHA1 hashes or per TPM bank hash values were extended into the TPM[1], but non-SHA1 boot aggregate values [2] should always include PCRs 8 & 9.
Any place commit 6f1a1d103b48 was backported [2], this commit 20c59ce010f8 ("ima: extend boot_aggregate with kernel measurements") should be backported as well.
Which kernels should it apply to? 5.7 is EOL now, so I looked at 5.4 but it doesn't apply cleanly there.
On Mon, 2020-11-30 at 19:21 -0500, Sasha Levin wrote:
On Sun, Nov 29, 2020 at 08:17:38AM -0500, Mimi Zohar wrote:
Hi Sasha,
On Wed, 2020-07-08 at 21:27 -0400, Sasha Levin wrote:
On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote:
Hi Sasha,
On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote:
From: Maurizio Drocco maurizio.drocco@ibm.com
[ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ]
Registers 8-9 are used to store measurements of the kernel and its command line (e.g., grub2 bootloader with tpm module enabled). IMA should include them in the boot aggregate. Registers 8-9 should be only included in non-SHA1 digests to avoid ambiguity.
Prior to Linux 5.8, the SHA1 template data hashes were padded before being extended into the TPM. Support for calculating and extending the per TPM bank template data digests is only being upstreamed in Linux 5.8.
How will attestation servers know whether to include PCRs 8 & 9 in the the boot_aggregate calculation? Now, there is a direct relationship between the template data SHA1 padded digest not including PCRs 8 & 9, and the new per TPM bank template data digest including them.
Got it, I'll drop it then, thank you!
After re-thinking this over, I realized that the attestation server can verify the "boot_aggregate" based on the quoted PCRs without knowing whether padded SHA1 hashes or per TPM bank hash values were extended into the TPM[1], but non-SHA1 boot aggregate values [2] should always include PCRs 8 & 9.
Any place commit 6f1a1d103b48 was backported [2], this commit 20c59ce010f8 ("ima: extend boot_aggregate with kernel measurements") should be backported as well.
Which kernels should it apply to? 5.7 is EOL now, so I looked at 5.4 but it doesn't apply cleanly there.
For 5.4, both "git cherry-pick" and "git am --3way" for 20c59ce010f8 seem to work.
thanks,
Mimi
On Mon, Nov 30, 2020 at 10:13:02PM -0500, Mimi Zohar wrote:
On Mon, 2020-11-30 at 19:21 -0500, Sasha Levin wrote:
On Sun, Nov 29, 2020 at 08:17:38AM -0500, Mimi Zohar wrote:
Hi Sasha,
On Wed, 2020-07-08 at 21:27 -0400, Sasha Levin wrote:
On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote:
Hi Sasha,
On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote:
From: Maurizio Drocco maurizio.drocco@ibm.com
[ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ]
Registers 8-9 are used to store measurements of the kernel and its command line (e.g., grub2 bootloader with tpm module enabled). IMA should include them in the boot aggregate. Registers 8-9 should be only included in non-SHA1 digests to avoid ambiguity.
Prior to Linux 5.8, the SHA1 template data hashes were padded before being extended into the TPM. Support for calculating and extending the per TPM bank template data digests is only being upstreamed in Linux 5.8.
How will attestation servers know whether to include PCRs 8 & 9 in the the boot_aggregate calculation? Now, there is a direct relationship between the template data SHA1 padded digest not including PCRs 8 & 9, and the new per TPM bank template data digest including them.
Got it, I'll drop it then, thank you!
After re-thinking this over, I realized that the attestation server can verify the "boot_aggregate" based on the quoted PCRs without knowing whether padded SHA1 hashes or per TPM bank hash values were extended into the TPM[1], but non-SHA1 boot aggregate values [2] should always include PCRs 8 & 9.
Any place commit 6f1a1d103b48 was backported [2], this commit 20c59ce010f8 ("ima: extend boot_aggregate with kernel measurements") should be backported as well.
Which kernels should it apply to? 5.7 is EOL now, so I looked at 5.4 but it doesn't apply cleanly there.
For 5.4, both "git cherry-pick" and "git am --3way" for 20c59ce010f8 seem to work.
You're right, I've grabbed it too. Thanks!
On 2020-11-29 08:17:38, Mimi Zohar wrote:
Hi Sasha,
On Wed, 2020-07-08 at 21:27 -0400, Sasha Levin wrote:
On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote:
Hi Sasha,
On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote:
From: Maurizio Drocco maurizio.drocco@ibm.com
[ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ]
Registers 8-9 are used to store measurements of the kernel and its command line (e.g., grub2 bootloader with tpm module enabled). IMA should include them in the boot aggregate. Registers 8-9 should be only included in non-SHA1 digests to avoid ambiguity.
Prior to Linux 5.8, the SHA1 template data hashes were padded before being extended into the TPM. Support for calculating and extending the per TPM bank template data digests is only being upstreamed in Linux 5.8.
How will attestation servers know whether to include PCRs 8 & 9 in the the boot_aggregate calculation? Now, there is a direct relationship between the template data SHA1 padded digest not including PCRs 8 & 9, and the new per TPM bank template data digest including them.
Got it, I'll drop it then, thank you!
After re-thinking this over, I realized that the attestation server can verify the "boot_aggregate" based on the quoted PCRs without knowing whether padded SHA1 hashes or per TPM bank hash values were extended into the TPM[1], but non-SHA1 boot aggregate values [2] should always include PCRs 8 & 9.
I'm still not clear on how an attestation server would know to include PCRs 8 and 9 after this change came through a stable kernel update. It doesn't seem like something appropriate for stable since it requires code changes to attestation servers to handle the change.
I know this has already been released in some stable releases, so I'm too late, but perhaps I'm missing something.
Tyler
Any place commit 6f1a1d103b48 was backported [2], this commit 20c59ce010f8 ("ima: extend boot_aggregate with kernel measurements") should be backported as well.
thanks,
Mimi
[1] commit 1ea973df6e21 ("ima: Calculate and extend PCR with digests in ima_template_entry") [2] commit 6f1a1d103b48 ("ima: Switch to ima_hash_algo for boot aggregate")
On Thu, 2020-12-10 at 21:10 -0600, Tyler Hicks wrote:
On 2020-11-29 08:17:38, Mimi Zohar wrote:
Hi Sasha,
On Wed, 2020-07-08 at 21:27 -0400, Sasha Levin wrote:
On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote:
Hi Sasha,
On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote:
From: Maurizio Drocco maurizio.drocco@ibm.com
[ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ]
Registers 8-9 are used to store measurements of the kernel and its command line (e.g., grub2 bootloader with tpm module enabled). IMA should include them in the boot aggregate. Registers 8-9 should be only included in non-SHA1 digests to avoid ambiguity.
Prior to Linux 5.8, the SHA1 template data hashes were padded before being extended into the TPM. Support for calculating and extending the per TPM bank template data digests is only being upstreamed in Linux 5.8.
How will attestation servers know whether to include PCRs 8 & 9 in the the boot_aggregate calculation? Now, there is a direct relationship between the template data SHA1 padded digest not including PCRs 8 & 9, and the new per TPM bank template data digest including them.
Got it, I'll drop it then, thank you!
After re-thinking this over, I realized that the attestation server can verify the "boot_aggregate" based on the quoted PCRs without knowing whether padded SHA1 hashes or per TPM bank hash values were extended into the TPM[1], but non-SHA1 boot aggregate values [2] should always include PCRs 8 & 9.
I'm still not clear on how an attestation server would know to include PCRs 8 and 9 after this change came through a stable kernel update. It doesn't seem like something appropriate for stable since it requires code changes to attestation servers to handle the change.
I know this has already been released in some stable releases, so I'm too late, but perhaps I'm missing something.
The point of adding PCRs 8 & 9 only to non-SHA1 boot_aggregate values was to avoid affecting existing attestation servers. The intention was when attestation servers added support for the non-sha1 boot_aggregate values, they'd also include PCRs 8 & 9. The existing SHA1 boot_aggregate value remains PCRs 0 - 7.
To prevent this or something similar from happening again, what should have been the proper way of including PCRs 8 & 9?
thanks,
Mimi
On Fri, 2020-12-11 at 06:01 -0500, Mimi Zohar wrote:
On Thu, 2020-12-10 at 21:10 -0600, Tyler Hicks wrote:
On 2020-11-29 08:17:38, Mimi Zohar wrote:
Hi Sasha,
On Wed, 2020-07-08 at 21:27 -0400, Sasha Levin wrote:
On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote:
Hi Sasha,
On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote:
From: Maurizio Drocco maurizio.drocco@ibm.com
[ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ]
Registers 8-9 are used to store measurements of the kernel and its command line (e.g., grub2 bootloader with tpm module enabled). IMA should include them in the boot aggregate. Registers 8-9 should be only included in non- SHA1 digests to avoid ambiguity.
Prior to Linux 5.8, the SHA1 template data hashes were padded before being extended into the TPM. Support for calculating and extending the per TPM bank template data digests is only being upstreamed in Linux 5.8.
How will attestation servers know whether to include PCRs 8 & 9 in the the boot_aggregate calculation? Now, there is a direct relationship between the template data SHA1 padded digest not including PCRs 8 & 9, and the new per TPM bank template data digest including them.
Got it, I'll drop it then, thank you!
After re-thinking this over, I realized that the attestation server can verify the "boot_aggregate" based on the quoted PCRs without knowing whether padded SHA1 hashes or per TPM bank hash values were extended into the TPM[1], but non-SHA1 boot aggregate values [2] should always include PCRs 8 & 9.
I'm still not clear on how an attestation server would know to include PCRs 8 and 9 after this change came through a stable kernel update. It doesn't seem like something appropriate for stable since it requires code changes to attestation servers to handle the change.
I know this has already been released in some stable releases, so I'm too late, but perhaps I'm missing something.
The point of adding PCRs 8 & 9 only to non-SHA1 boot_aggregate values was to avoid affecting existing attestation servers. The intention was when attestation servers added support for the non-sha1 boot_aggregate values, they'd also include PCRs 8 & 9. The existing SHA1 boot_aggregate value remains PCRs 0 - 7.
To prevent this or something similar from happening again, what should have been the proper way of including PCRs 8 & 9?
Just to be pragmatic: this is going to happen again. Shim is already measuring the Mok variables through PCR 14, so if we want an accurate boot aggregate, we're going to have to include PCR 14 as well (or persuade shim to measure through a PCR we're already including, which isn't impossible since I think shim should be measuring the Mok variables using the EV_EFI_VARIABLE_DRIVER_CONFIG event and, since it affects secure boot policy, that does argue it should be measured through PCR 7).
James
On Fri, 2020-12-11 at 09:46 -0800, James Bottomley wrote:
On Fri, 2020-12-11 at 06:01 -0500, Mimi Zohar wrote:
On Thu, 2020-12-10 at 21:10 -0600, Tyler Hicks wrote:
On 2020-11-29 08:17:38, Mimi Zohar wrote:
Hi Sasha,
On Wed, 2020-07-08 at 21:27 -0400, Sasha Levin wrote:
On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote:
Hi Sasha,
On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote: > From: Maurizio Drocco maurizio.drocco@ibm.com > > [ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c > ] > > Registers 8-9 are used to store measurements of the kernel > and its command line (e.g., grub2 bootloader with tpm > module enabled). IMA should include them in the boot > aggregate. Registers 8-9 should be only included in non- > SHA1 digests to avoid ambiguity.
Prior to Linux 5.8, the SHA1 template data hashes were padded before being extended into the TPM. Support for calculating and extending the per TPM bank template data digests is only being upstreamed in Linux 5.8.
How will attestation servers know whether to include PCRs 8 & 9 in the the boot_aggregate calculation? Now, there is a direct relationship between the template data SHA1 padded digest not including PCRs 8 & 9, and the new per TPM bank template data digest including them.
Got it, I'll drop it then, thank you!
After re-thinking this over, I realized that the attestation server can verify the "boot_aggregate" based on the quoted PCRs without knowing whether padded SHA1 hashes or per TPM bank hash values were extended into the TPM[1], but non-SHA1 boot aggregate values [2] should always include PCRs 8 & 9.
I'm still not clear on how an attestation server would know to include PCRs 8 and 9 after this change came through a stable kernel update. It doesn't seem like something appropriate for stable since it requires code changes to attestation servers to handle the change.
I know this has already been released in some stable releases, so I'm too late, but perhaps I'm missing something.
The point of adding PCRs 8 & 9 only to non-SHA1 boot_aggregate values was to avoid affecting existing attestation servers. The intention was when attestation servers added support for the non-sha1 boot_aggregate values, they'd also include PCRs 8 & 9. The existing SHA1 boot_aggregate value remains PCRs 0 - 7.
To prevent this or something similar from happening again, what should have been the proper way of including PCRs 8 & 9?
Just to be pragmatic: this is going to happen again. Shim is already measuring the Mok variables through PCR 14, so if we want an accurate boot aggregate, we're going to have to include PCR 14 as well (or persuade shim to measure through a PCR we're already including, which isn't impossible since I think shim should be measuring the Mok variables using the EV_EFI_VARIABLE_DRIVER_CONFIG event and, since it affects secure boot policy, that does argue it should be measured through PCR 7).
Ok. Going forward, it sounds like we need to define a new "boot_aggregate" record. One that contains a version number and PCR mask.
Mimi
On 2020-12-11 06:01:54, Mimi Zohar wrote:
On Thu, 2020-12-10 at 21:10 -0600, Tyler Hicks wrote:
On 2020-11-29 08:17:38, Mimi Zohar wrote:
Hi Sasha,
On Wed, 2020-07-08 at 21:27 -0400, Sasha Levin wrote:
On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote:
Hi Sasha,
On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote:
From: Maurizio Drocco maurizio.drocco@ibm.com
[ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ]
Registers 8-9 are used to store measurements of the kernel and its command line (e.g., grub2 bootloader with tpm module enabled). IMA should include them in the boot aggregate. Registers 8-9 should be only included in non-SHA1 digests to avoid ambiguity.
Prior to Linux 5.8, the SHA1 template data hashes were padded before being extended into the TPM. Support for calculating and extending the per TPM bank template data digests is only being upstreamed in Linux 5.8.
How will attestation servers know whether to include PCRs 8 & 9 in the the boot_aggregate calculation? Now, there is a direct relationship between the template data SHA1 padded digest not including PCRs 8 & 9, and the new per TPM bank template data digest including them.
Got it, I'll drop it then, thank you!
After re-thinking this over, I realized that the attestation server can verify the "boot_aggregate" based on the quoted PCRs without knowing whether padded SHA1 hashes or per TPM bank hash values were extended into the TPM[1], but non-SHA1 boot aggregate values [2] should always include PCRs 8 & 9.
I'm still not clear on how an attestation server would know to include PCRs 8 and 9 after this change came through a stable kernel update. It doesn't seem like something appropriate for stable since it requires code changes to attestation servers to handle the change.
I know this has already been released in some stable releases, so I'm too late, but perhaps I'm missing something.
The point of adding PCRs 8 & 9 only to non-SHA1 boot_aggregate values was to avoid affecting existing attestation servers. The intention was when attestation servers added support for the non-sha1 boot_aggregate values, they'd also include PCRs 8 & 9. The existing SHA1 boot_aggregate value remains PCRs 0 - 7.
AFAIK, there's nothing that prevents the non-SHA1 TPM 2.0 PCR banks from being used even before v5.8, albeit with zero padded SHA1 digests. Existing attestation servers that already support that configuration are broken by this stable backport.
To prevent this or something similar from happening again, what should have been the proper way of including PCRs 8 & 9?
I don't think that commits like 6f1a1d103b48 ("ima: Switch to ima_hash_algo for boot aggregate") and 20c59ce010f8 ("ima: extend boot_aggregate with kernel measurements") should be backported to stable.
Including PCRs 8 and 9 definitely makes sense to include in the boot_aggregate value but limiting such a change to "starting in 5.8", rather than "starting in 5.8 and 5.4.82", is the safer approach when attestation server modifications are required.
Tyler
thanks,
Mimi
On 12/12/2020 9:22 PM, Mimi Zohar wrote:
Ok. Going forward, it sounds like we need to define a new "boot_aggregate" record. One that contains a version number and PCR mask.
Just BTW, there is a TCG standard for a TPM 2.0 PCR mask that works well.
There is also a standard for an event log version number. It is the first event of a TPM 2.0 event log. It is strange.
One useful field, though, is a mapping between the algorithm ID (e.g., sha256 is 0x000b) and the digest size (e.g., 32 bytes). This permits a parser to parse a log even when it encounters an unknown digest algorithm.
On Mon, 2020-12-28 at 14:28 -0500, Ken Goldman wrote:
On 12/12/2020 9:22 PM, Mimi Zohar wrote:
Ok. Going forward, it sounds like we need to define a new "boot_aggregate" record. One that contains a version number and PCR mask.
Just BTW, there is a TCG standard for a TPM 2.0 PCR mask that works well.
Sounds good.
There is also a standard for an event log version number. It is the first event of a TPM 2.0 event log. It is strange.
Ok
One useful field, though, is a mapping between the algorithm ID (e.g., sha256 is 0x000b) and the digest size (e.g., 32 bytes). This permits a parser to parse a log even when it encounters an unknown digest algorithm.
The template data is prefixed with the template data length. The problem is verifying the boot_aggregate, not parsing the log.
thanks,
Mimi
On 2020-12-14 10:42:24, Tyler Hicks wrote:
On 2020-12-11 06:01:54, Mimi Zohar wrote:
On Thu, 2020-12-10 at 21:10 -0600, Tyler Hicks wrote:
On 2020-11-29 08:17:38, Mimi Zohar wrote:
Hi Sasha,
On Wed, 2020-07-08 at 21:27 -0400, Sasha Levin wrote:
On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote:
Hi Sasha,
On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote: > From: Maurizio Drocco maurizio.drocco@ibm.com > > [ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ] > > Registers 8-9 are used to store measurements of the kernel and its > command line (e.g., grub2 bootloader with tpm module enabled). IMA > should include them in the boot aggregate. Registers 8-9 should be > only included in non-SHA1 digests to avoid ambiguity.
Prior to Linux 5.8, the SHA1 template data hashes were padded before being extended into the TPM. Support for calculating and extending the per TPM bank template data digests is only being upstreamed in Linux 5.8.
How will attestation servers know whether to include PCRs 8 & 9 in the the boot_aggregate calculation? Now, there is a direct relationship between the template data SHA1 padded digest not including PCRs 8 & 9, and the new per TPM bank template data digest including them.
Got it, I'll drop it then, thank you!
After re-thinking this over, I realized that the attestation server can verify the "boot_aggregate" based on the quoted PCRs without knowing whether padded SHA1 hashes or per TPM bank hash values were extended into the TPM[1], but non-SHA1 boot aggregate values [2] should always include PCRs 8 & 9.
I'm still not clear on how an attestation server would know to include PCRs 8 and 9 after this change came through a stable kernel update. It doesn't seem like something appropriate for stable since it requires code changes to attestation servers to handle the change.
I know this has already been released in some stable releases, so I'm too late, but perhaps I'm missing something.
The point of adding PCRs 8 & 9 only to non-SHA1 boot_aggregate values was to avoid affecting existing attestation servers. The intention was when attestation servers added support for the non-sha1 boot_aggregate values, they'd also include PCRs 8 & 9. The existing SHA1 boot_aggregate value remains PCRs 0 - 7.
AFAIK, there's nothing that prevents the non-SHA1 TPM 2.0 PCR banks from being used even before v5.8, albeit with zero padded SHA1 digests. Existing attestation servers that already support that configuration are broken by this stable backport.
To wrap up this thread, I think the last thing to address is if this commit should be reverted from stable kernels? Do you have any thoughts about that, Mimi?
Tyler
To prevent this or something similar from happening again, what should have been the proper way of including PCRs 8 & 9?
I don't think that commits like 6f1a1d103b48 ("ima: Switch to ima_hash_algo for boot aggregate") and 20c59ce010f8 ("ima: extend boot_aggregate with kernel measurements") should be backported to stable.
Including PCRs 8 and 9 definitely makes sense to include in the boot_aggregate value but limiting such a change to "starting in 5.8", rather than "starting in 5.8 and 5.4.82", is the safer approach when attestation server modifications are required.
Tyler
thanks,
Mimi
Hi Tyler,
On Tue, 2021-01-12 at 09:35 -0600, Tyler Hicks wrote:
On 2020-12-14 10:42:24, Tyler Hicks wrote:
On 2020-12-11 06:01:54, Mimi Zohar wrote:
On Thu, 2020-12-10 at 21:10 -0600, Tyler Hicks wrote:
On 2020-11-29 08:17:38, Mimi Zohar wrote:
Hi Sasha,
On Wed, 2020-07-08 at 21:27 -0400, Sasha Levin wrote:
On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote: >Hi Sasha, > >On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote: >> From: Maurizio Drocco maurizio.drocco@ibm.com >> >> [ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ] >> >> Registers 8-9 are used to store measurements of the kernel and its >> command line (e.g., grub2 bootloader with tpm module enabled). IMA >> should include them in the boot aggregate. Registers 8-9 should be >> only included in non-SHA1 digests to avoid ambiguity. > >Prior to Linux 5.8, the SHA1 template data hashes were padded before >being extended into the TPM. Support for calculating and extending >the per TPM bank template data digests is only being upstreamed in >Linux 5.8. > >How will attestation servers know whether to include PCRs 8 & 9 in the >the boot_aggregate calculation? Now, there is a direct relationship >between the template data SHA1 padded digest not including PCRs 8 & 9, >and the new per TPM bank template data digest including them.
Got it, I'll drop it then, thank you!
After re-thinking this over, I realized that the attestation server can verify the "boot_aggregate" based on the quoted PCRs without knowing whether padded SHA1 hashes or per TPM bank hash values were extended into the TPM[1], but non-SHA1 boot aggregate values [2] should always include PCRs 8 & 9.
I'm still not clear on how an attestation server would know to include PCRs 8 and 9 after this change came through a stable kernel update. It doesn't seem like something appropriate for stable since it requires code changes to attestation servers to handle the change.
I know this has already been released in some stable releases, so I'm too late, but perhaps I'm missing something.
The point of adding PCRs 8 & 9 only to non-SHA1 boot_aggregate values was to avoid affecting existing attestation servers. The intention was when attestation servers added support for the non-sha1 boot_aggregate values, they'd also include PCRs 8 & 9. The existing SHA1 boot_aggregate value remains PCRs 0 - 7.
AFAIK, there's nothing that prevents the non-SHA1 TPM 2.0 PCR banks from being used even before v5.8, albeit with zero padded SHA1 digests. Existing attestation servers that already support that configuration are broken by this stable backport.
To wrap up this thread, I think the last thing to address is if this commit should be reverted from stable kernels? Do you have any thoughts about that, Mimi?
To prevent this or something similar from happening again, what should have been the proper way of including PCRs 8 & 9?
I don't think that commits like 6f1a1d103b48 ("ima: Switch to ima_hash_algo for boot aggregate") and 20c59ce010f8 ("ima: extend boot_aggregate with kernel measurements") should be backported to stable.
Including PCRs 8 and 9 definitely makes sense to include in the boot_aggregate value but limiting such a change to "starting in 5.8", rather than "starting in 5.8 and 5.4.82", is the safer approach when attestation server modifications are required.
As I recall, commit 6f1a1d103b48 ("ima: Switch to ima_hash_algo for boot aggregate") was backported to address TPMs without SHA1 support, as reported by Jerry.
Mimi
linux-stable-mirror@lists.linaro.org
-
James Bottomley -
Ken Goldman -
Mimi Zohar -
Sasha Levin -
Tyler Hicks