From e4f0a7ec586b7644107839f5394fb685cf1aadcc Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Tue, 26 Apr 2022 19:56:18 +0200 Subject: [PATCH 1/2] pstore: migrate to crypto acomp interface The crypto 'compress' interface is deprecated, so before adding new features, migrate to the acomp interface. Note that we are only using synchronous implementations of acomp, so we don't have to deal with asynchronous completion. Signed-off-by: Ard Biesheuvel Signed-off-by: Kees Cook --- fs/pstore/platform.c | 63 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 51 insertions(+), 12 deletions(-) diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index e26162f102ff..4d7fc1c06a6c 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -28,11 +28,14 @@ #include #include #include +#include #include #include #include #include +#include + #include "internal.h" /* @@ -90,7 +93,8 @@ module_param(compress, charp, 0444); MODULE_PARM_DESC(compress, "compression to use"); /* Compression parameters */ -static struct crypto_comp *tfm; +static struct crypto_acomp *tfm; +static struct acomp_req *creq; struct pstore_zbackend { int (*zbufsize)(size_t size); @@ -268,12 +272,21 @@ static const struct pstore_zbackend zbackends[] = { static int pstore_compress(const void *in, void *out, unsigned int inlen, unsigned int outlen) { + struct scatterlist src, dst; int ret; if (!IS_ENABLED(CONFIG_PSTORE_COMPRESS)) return -EINVAL; - ret = crypto_comp_compress(tfm, in, inlen, out, &outlen); + sg_init_table(&src, 1); + sg_set_buf(&src, in, inlen); + + sg_init_table(&dst, 1); + sg_set_buf(&dst, out, outlen); + + acomp_request_set_params(creq, &src, &dst, inlen, outlen); + + ret = crypto_acomp_compress(creq); if (ret) { pr_err("crypto_comp_compress failed, ret = %d!\n", ret); return ret; @@ -284,7 +297,7 @@ static int pstore_compress(const void *in, void *out, static void allocate_buf_for_compression(void) { - struct crypto_comp *ctx; + struct crypto_acomp *acomp; int size; char *buf; @@ -296,7 +309,7 @@ static void allocate_buf_for_compression(void) if (!psinfo || tfm) return; - if (!crypto_has_comp(zbackend->name, 0, 0)) { + if (!crypto_has_acomp(zbackend->name, 0, CRYPTO_ALG_ASYNC)) { pr_err("Unknown compression: %s\n", zbackend->name); return; } @@ -315,16 +328,24 @@ static void allocate_buf_for_compression(void) return; } - ctx = crypto_alloc_comp(zbackend->name, 0, 0); - if (IS_ERR_OR_NULL(ctx)) { + acomp = crypto_alloc_acomp(zbackend->name, 0, CRYPTO_ALG_ASYNC); + if (IS_ERR_OR_NULL(acomp)) { kfree(buf); pr_err("crypto_alloc_comp('%s') failed: %ld\n", zbackend->name, - PTR_ERR(ctx)); + PTR_ERR(acomp)); + return; + } + + creq = acomp_request_alloc(acomp); + if (!creq) { + crypto_free_acomp(acomp); + kfree(buf); + pr_err("acomp_request_alloc('%s') failed\n", zbackend->name); return; } /* A non-NULL big_oops_buf indicates compression is available. */ - tfm = ctx; + tfm = acomp; big_oops_buf_sz = size; big_oops_buf = buf; @@ -334,7 +355,8 @@ static void allocate_buf_for_compression(void) static void free_buf_for_compression(void) { if (IS_ENABLED(CONFIG_PSTORE_COMPRESS) && tfm) { - crypto_free_comp(tfm); + acomp_request_free(creq); + crypto_free_acomp(tfm); tfm = NULL; } kfree(big_oops_buf); @@ -671,6 +693,8 @@ static void decompress_record(struct pstore_record *record) int ret; int unzipped_len; char *unzipped, *workspace; + struct acomp_req *dreq; + struct scatterlist src, dst; if (!IS_ENABLED(CONFIG_PSTORE_COMPRESS) || !record->compressed) return; @@ -694,16 +718,30 @@ static void decompress_record(struct pstore_record *record) if (!workspace) return; + dreq = acomp_request_alloc(tfm); + if (!dreq) { + kfree(workspace); + return; + } + + sg_init_table(&src, 1); + sg_set_buf(&src, record->buf, record->size); + + sg_init_table(&dst, 1); + sg_set_buf(&dst, workspace, unzipped_len); + + acomp_request_set_params(dreq, &src, &dst, record->size, unzipped_len); + /* After decompression "unzipped_len" is almost certainly smaller. */ - ret = crypto_comp_decompress(tfm, record->buf, record->size, - workspace, &unzipped_len); + ret = crypto_acomp_decompress(dreq); if (ret) { - pr_err("crypto_comp_decompress failed, ret = %d!\n", ret); + pr_err("crypto_acomp_decompress failed, ret = %d!\n", ret); kfree(workspace); return; } /* Append ECC notice to decompressed buffer. */ + unzipped_len = dreq->dlen; memcpy(workspace + unzipped_len, record->buf + record->size, record->ecc_notice_size); @@ -711,6 +749,7 @@ static void decompress_record(struct pstore_record *record) unzipped = kmemdup(workspace, unzipped_len + record->ecc_notice_size, GFP_KERNEL); kfree(workspace); + acomp_request_free(dreq); if (!unzipped) return; From 2c09d1443b9b8b6e25bfb2acf51ad442cf9b314e Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Thu, 23 Jun 2022 16:40:13 +0300 Subject: [PATCH 2/2] pstore/zone: cleanup "rcnt" type The info->read() function returns ssize_t. That means that info->read() either returns either negative error codes or a positive number representing the bytes read. The "rcnt" variable should be declared as ssize_t as well. Most places do this correctly but psz_kmsg_recover_meta() needed to be fixed. This code casts the "rcnt" to int. That is unnecessary when "rcnt" is already signed. It's also slightly wrong because if info->read() returned a very high (more than INT_MAX) number of bytes then this might treat that as an error. This bug cannot happen in real life, so it doesn't affect run time, but static checkers correctly complain that it is wrong. fs/pstore/zone.c:366 psz_kmsg_recover_data() warn: casting 'rcnt' truncates high bits Signed-off-by: Dan Carpenter Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/YrRtPSFHDVJzV6d+@kili --- fs/pstore/zone.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/fs/pstore/zone.c b/fs/pstore/zone.c index 7c8f8feac6c3..017d0d4ad329 100644 --- a/fs/pstore/zone.c +++ b/fs/pstore/zone.c @@ -363,7 +363,7 @@ static int psz_kmsg_recover_data(struct psz_context *cxt) rcnt = info->read((char *)buf, zone->buffer_size + sizeof(*buf), zone->off); if (rcnt != zone->buffer_size + sizeof(*buf)) - return (int)rcnt < 0 ? (int)rcnt : -EIO; + return rcnt < 0 ? rcnt : -EIO; } return 0; } @@ -372,7 +372,7 @@ static int psz_kmsg_recover_meta(struct psz_context *cxt) { struct pstore_zone_info *info = cxt->pstore_zone_info; struct pstore_zone *zone; - size_t rcnt, len; + ssize_t rcnt, len; struct psz_buffer *buf; struct psz_kmsg_header *hdr; struct timespec64 time = { }; @@ -400,7 +400,7 @@ static int psz_kmsg_recover_meta(struct psz_context *cxt) continue; } else if (rcnt != len) { pr_err("read %s with id %lu failed\n", zone->name, i); - return (int)rcnt < 0 ? (int)rcnt : -EIO; + return rcnt < 0 ? rcnt : -EIO; } if (buf->sig != zone->buffer->sig) { @@ -502,7 +502,7 @@ static int psz_recover_zone(struct psz_context *cxt, struct pstore_zone *zone) rcnt = info->read((char *)&tmpbuf, len, zone->off); if (rcnt != len) { pr_debug("read zone %s failed\n", zone->name); - return (int)rcnt < 0 ? (int)rcnt : -EIO; + return rcnt < 0 ? rcnt : -EIO; } if (tmpbuf.sig != zone->buffer->sig) { @@ -544,7 +544,7 @@ static int psz_recover_zone(struct psz_context *cxt, struct pstore_zone *zone) rcnt = info->read(buf, len - start, off + start); if (rcnt != len - start) { pr_err("read zone %s failed\n", zone->name); - ret = (int)rcnt < 0 ? (int)rcnt : -EIO; + ret = rcnt < 0 ? rcnt : -EIO; goto free_oldbuf; } @@ -552,7 +552,7 @@ static int psz_recover_zone(struct psz_context *cxt, struct pstore_zone *zone) rcnt = info->read(buf + len - start, start, off); if (rcnt != start) { pr_err("read zone %s failed\n", zone->name); - ret = (int)rcnt < 0 ? (int)rcnt : -EIO; + ret = rcnt < 0 ? rcnt : -EIO; goto free_oldbuf; }