crash_dump: Don't log dm-crypt key bytes in read_key_from_user_keying
Subsystems (4)
PATCH 1/1 crash_dump: Don't log dm-crypt key bytes in read_key_from_user_keying
When debug logging is enabled, read_key_from_user_keying() logs the first 8 bytes of the key payload and partially exposes the dm-crypt key. Stop logging any key bytes.
--- kernel/crash_dump_dm_crypt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/crash_dump_dm_crypt.c +2 -2
@@ -168,8 +168,8 @@ static int read_key_from_user_keying(struct dm_crypt_key *dm_key)memcpy (dm_key->data, ukp->data, ukp->datalen); dm_key->key_size = ukp->datalen; -kexec_dprintk ("Get dm crypt key (size=%u) %s: %8ph\n" , dm_key->key_size, - dm_key->key_desc, dm_key->data); +kexec_dprintk ("Get dm crypt key (size=%u) %s\n" , dm_key->key_size, + dm_key->key_desc); out:up_read (&key->sem);
Signature · 2 lines
Thorsten Blum <[email protected]> GPG: 1D60 735E 8AEF 3BE4 73B6 9D84 7336 78FD 8DFE EAD4
Quoted 16 lines · Thorsten Blum
When debug logging is enabled,
read_key_from_user_keying()logs the first 8 bytes of the key payload and partially exposes the dm-crypt key. Stop logging any key bytes.Fixes 479e58549b0f ("crash_dump: store dm crypt keys in kdump reserved memory")Signed-off-by Thorsten Blum <[email protected]>--- kernel/crash_dump_dm_crypt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)kernel/crash_dump_dm_crypt.c +0
-0
kernel/crash_dump_dm_crypt.c static int read_key_from_user_keying(struct dm_crypt_key *dm_key) · line 171
Quoted 6 lines
kernel/crash_dump_dm_crypt.c +1
-2@@ -168,8 +168,8 @@ static int read_key_from_user_keying(struct dm_crypt_key *dm_key)memcpy (dm_key->data, ukp->data, ukp->datalen); dm_key->key_size = ukp->datalen; -kexec_dprintk ("Get dm crypt key (size=%u) %s: %8ph\n" , dm_key->key_size, - dm_key->key_desc, dm_key->data); + kexec_dprintk("Get dm crypt key (size=%u) %s\n", dm_key->key_size,
Make sense to me.
The kexec_dprintk() is only for debug printing. We can remove above line or change it to pr_debug() if security is worried.
Coiby, what do you think?
Thanks
Baoquan
Quoted 26 lines · Baoquan He
Quoted 16 lines · Thorsten Blum
When debug logging is enabled,
read_key_from_user_keying()logs the first 8 bytes of the key payload and partially exposes the dm-crypt key. Stop logging any key bytes.Fixes 479e58549b0f ("crash_dump: store dm crypt keys in kdump reserved memory")Signed-off-by Thorsten Blum <[email protected]>--- kernel/crash_dump_dm_crypt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)kernel/crash_dump_dm_crypt.c +0
-0kernel/crash_dump_dm_crypt.c static int read_key_from_user_keying(struct dm_crypt_key *dm_key) · line 171
Quoted 6 lines
kernel/crash_dump_dm_crypt.c +1
-2@@ -168,8 +168,8 @@ static int read_key_from_user_keying(struct dm_crypt_key *dm_key)memcpy (dm_key->data, ukp->data, ukp->datalen); dm_key->key_size = ukp->datalen; -kexec_dprintk ("Get dm crypt key (size=%u) %s: %8ph\n" , dm_key->key_size, - dm_key->key_desc, dm_key->data); + kexec_dprintk("Get dm crypt key (size=%u) %s\n", dm_key->key_size,Make sense to me.
Quoted 4 lines
The
kexec_dprintk()is only for debug printing. We can remove above line or change it topr_debug()if security is worried.Coiby, what do you think?
I think we can assume a key can be reliably read thus no need to print the first 8 bytes to check it. So this patch looks good to me. And thanks to Thorsten for suggesting multiple improvements on kernel/crash_dump_dm_crypt.c!
Quoted 4 lines
Thanks
Baoquan
Signature · 2 lines
Best regards, Coiby