From: Bernard Zhao <[email protected]>
[ 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 <[email protected]>
[trivial commit msg fixup]
Signed-off-by: Rob Clark <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
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;
}
--
2.25.1
From: Krishna Manikandan <[email protected]>
[ 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 <[email protected]>
Signed-off-by: Rob Clark <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
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;
}
--
2.25.1
From: Marek Szyprowski <[email protected]>
[ Upstream commit b9c633882de4601015625f9136f248e9abca8a7a ]
Propagate the proper error codes from the called functions instead of
unconditionally returning 0.
Reported-by: kbuild test robot <[email protected]>
Signed-off-by: Marek Szyprowski <[email protected]>
Merge conflict so merged it manually.
Signed-off-by: Inki Dae <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
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;
}
/*
--
2.25.1
From: "Hyeongseok.Kim" <[email protected]>
[ 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 <[email protected]>
Reviewed-by: Sungjong Seo <[email protected]>
Signed-off-by: Namjae Jeon <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
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;
+ }
}
}
--
2.25.1
From: Anson Huang <[email protected]>
[ 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 <[email protected]>
Signed-off-by: Daniel Lezcano <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Sasha Levin <[email protected]>
---
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)
--
2.25.1
From: Srinivas Pandruvada <[email protected]>
[ Upstream commit 1e05daca83bb42cde569f75f3bd7c8828b1ef30f ]
Add DPTF battery participant ACPI ID for platforms based on the Intel
TigerLake SoC.
Signed-off-by: Srinivas Pandruvada <[email protected]>
[ rjw: Changelog ]
Signed-off-by: Rafael J. Wysocki <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
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);
--
2.25.1
From: Angelo Dureghello <[email protected]>
[ 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 <[email protected]>
Signed-off-by: Mike Rapoport <[email protected]>
Signed-off-by: Greg Ungerer <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
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);
--
2.25.1
From: Mike Rapoport <[email protected]>
[ 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 <[email protected]>
Signed-off-by: Greg Ungerer <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
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];
--
2.25.1
From: Hyunchul Lee <[email protected]>
[ 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 <[email protected]>
Acked-by: Sungjong Seo <[email protected]>
Signed-off-by: Namjae Jeon <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
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)
--
2.25.1
From: Navid Emamdoost <[email protected]>
[ 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 <[email protected]>
Signed-off-by: Inki Dae <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
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);
--
2.25.1
From: Hou Tao <[email protected]>
[ Upstream commit bfe373f608cf81b7626dfeb904001b0e867c5110 ]
Else there may be magic numbers in /sys/kernel/debug/block/*/state.
Signed-off-by: Hou Tao <[email protected]>
Reviewed-by: Bart Van Assche <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
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 */
--
2.25.1
From: Petteri Aimonen <[email protected]>
[ 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 <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=207979
Link: https://lkml.kernel.org/r/[email protected]
Signed-off-by: Sasha Levin <[email protected]>
---
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);
--
2.25.1
From: Maurizio Drocco <[email protected]>
[ 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 <[email protected]>
Reviewed-by: Bruno Meneguele <[email protected]>
Tested-by: Bruno Meneguele <[email protected]> (TPM 1.2, TPM 2.0)
Signed-off-by: Mimi Zohar <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
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;
--
2.25.1
Hi Sasha,
On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote:
> From: Maurizio Drocco <[email protected]>
>
> [ 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 <[email protected]>
>>
>> [ 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!
--
Thanks,
Sasha
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 <[email protected]>
> >>
> >> [ 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 <[email protected]>
>> >>
>> >> [ 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.
--
Thanks,
Sasha
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 <[email protected]>
> >> >>
> >> >> [ 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 <[email protected]>
>> >> >>
>> >> >> [ 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!
--
Thanks,
Sasha
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 <[email protected]>
> > >>
> > >> [ 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 <[email protected]>
> > > >>
> > > >> [ 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 <[email protected]>
> > > > > >
> > > > > > [ 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 <[email protected]>
> > > > > > >
> > > > > > > [ 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 <[email protected]>
> > > > >>
> > > > >> [ 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 <[email protected]>
> > > > > >>
> > > > > >> [ 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 <[email protected]>
> > > > > > >>
> > > > > > >> [ 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