lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 18 Oct 2018 10:12:03 +0530 From: Sai Prakash Ranjan <saiprakash.ranjan@...eaurora.org> To: Kees Cook <keescook@...omium.org>, Joel Fernandes <joel@...lfernandes.org> Cc: linux-kernel@...r.kernel.org Subject: Re: [PATCH] pstore: Refactor compression initialization On 10/18/2018 7:00 AM, Joel Fernandes wrote: On Wed, Oct 17, 2018 at 02:41:24PM -0700, Kees Cook wrote: >> With compression initialization now separated from pstore_register(), >> there is no longer a good reason to do compression method selection >> during fs init. Instead, merge everything together into the late init. >> Additionally cleans up the reporting to be more clear. >> >> Signed-off-by: Kees Cook <keescook@...omium.org> >> --- >> This patch goes on top of Joel's. I'll have both in linux-next shortly... >> --- >> fs/pstore/inode.c | 2 -- >> fs/pstore/internal.h | 3 -- >> fs/pstore/platform.c | 65 ++++++++++++++++++++++++++++---------------- >> 3 files changed, 41 insertions(+), 29 deletions(-) >> >> diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c >> index 5fcb845b9fec..d814723fb27d 100644 >> --- a/fs/pstore/inode.c >> +++ b/fs/pstore/inode.c >> @@ -486,8 +486,6 @@ static int __init init_pstore_fs(void) >> { >> int err; >> >> - pstore_choose_compression(); >> - >> /* Create a convenient mount point for people to access pstore */ >> err = sysfs_create_mount_point(fs_kobj, "pstore"); >> if (err) >> diff --git a/fs/pstore/internal.h b/fs/pstore/internal.h >> index fb767e28aeb2..c029314478fa 100644 >> --- a/fs/pstore/internal.h >> +++ b/fs/pstore/internal.h >> @@ -37,7 +37,4 @@ extern bool pstore_is_mounted(void); >> extern void pstore_record_init(struct pstore_record *record, >> struct pstore_info *psi); >> >> -/* Called during module_init() */ >> -extern void __init pstore_choose_compression(void); >> - >> #endif >> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c >> index f09066db2d4d..e4737a5fa68c 100644 >> --- a/fs/pstore/platform.c >> +++ b/fs/pstore/platform.c >> @@ -274,36 +274,56 @@ static int pstore_decompress(void *in, void *out, >> >> static void allocate_buf_for_compression(void) >> { >> + struct crypto_comp *ctx; >> + int size; >> + char *buf; >> + >> + /* Skip if not built-in or compression backend not selected yet. */ >> if (!IS_ENABLED(CONFIG_PSTORE_COMPRESS) || !zbackend) >> return; >> >> + /* Skip if no pstore backend yet or compression init already done. */ >> + if (!psinfo || tfm) >> + return; >> + >> if (!crypto_has_comp(zbackend->name, 0, 0)) { >> - pr_err("No %s compression\n", zbackend->name); >> + pr_err("Unknown compression: %s\n", zbackend->name); >> return; >> } >> >> - big_oops_buf_sz = zbackend->zbufsize(psinfo->bufsize); >> - if (big_oops_buf_sz <= 0) >> + size = zbackend->zbufsize(psinfo->bufsize); >> + if (size <= 0) { >> + pr_err("Invalid compression size for %s: %d\n", >> + zbackend->name, size); >> return; >> + } >> >> - big_oops_buf = kmalloc(big_oops_buf_sz, GFP_KERNEL); >> - if (!big_oops_buf) { >> - pr_err("allocate compression buffer error!\n"); >> + buf = kmalloc(size, GFP_KERNEL); >> + if (!buf) { >> + pr_err("Failed %d byte compression buffer allocation for: %s\n", >> + size, zbackend->name); >> return; >> } >> >> - tfm = crypto_alloc_comp(zbackend->name, 0, 0); >> - if (IS_ERR_OR_NULL(tfm)) { >> - kfree(big_oops_buf); >> - big_oops_buf = NULL; >> - pr_err("crypto_alloc_comp() failed!\n"); >> + ctx = crypto_alloc_comp(zbackend->name, 0, 0); >> + if (IS_ERR_OR_NULL(ctx)) { >> + kfree(buf); >> + pr_err("crypto_alloc_comp('%s') failed: %ld\n", zbackend->name, >> + PTR_ERR(ctx)); >> return; >> } >> + >> + /* A non-NULL big_oops_buf indicates compression is available. */ >> + tfm = ctx; >> + big_oops_buf_sz = size; >> + big_oops_buf = buf; >> + >> + pr_info("Using compression: %s\n", zbackend->name); >> } >> >> static void free_buf_for_compression(void) >> { >> - if (IS_ENABLED(CONFIG_PSTORE_COMPRESS) && !IS_ERR_OR_NULL(tfm)) >> + if (IS_ENABLED(CONFIG_PSTORE_COMPRESS) && tfm) >> crypto_free_comp(tfm); >> kfree(big_oops_buf); >> big_oops_buf = NULL; >> @@ -764,31 +784,28 @@ static void pstore_timefunc(struct timer_list *unused) >> jiffies + msecs_to_jiffies(pstore_update_ms)); >> } >> >> -void __init pstore_choose_compression(void) >> +static int __init pstore_compression_late_init(void) >> { >> const struct pstore_zbackend *step; >> >> if (!compress) >> - return; >> + return 0; >> >> + /* Locate desired compression method. */ >> for (step = zbackends; step->name; step++) { >> if (!strcmp(compress, step->name)) { >> zbackend = step; >> - pr_info("using %s compression\n", zbackend->name); >> - return; >> + break; >> } >> } >> -} >> >> -static int __init pstore_compression_late_init(void) >> -{ >> /* >> - * Check if any pstore backends registered earlier but did not allocate >> - * for compression because crypto was not ready, if so then initialize >> - * compression. >> + * Check if any pstore backends registered earlier but did not >> + * initialize compression because crypto was not ready. If so, >> + * then initialize compression now. >> */ >> - if (psinfo && !tfm) >> - allocate_buf_for_compression(); >> + allocate_buf_for_compression(); > > We can also get rid of the 'zbackend' global variable since choosing the > compression backend and allocating the buffers are done at the same time? > > Otherwise looks good to me, > > Reviewed-by: Joel Fernandes (Google) <joel@...lfernandes.org> > Tested this on top of Joel's patch, works fine on getting early crash pstore dmesg logs, thanks. Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@...eaurora.org> -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Powered by blists - more mailing lists