linux-kernel - [PATCH -next]pstore/zone: Preallocate zone buffer for psz_kmsg_write

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-Id: <DE6A3E0D-275B-4719-850A-41850749311A@gmail.com>
Date:   Wed, 16 Nov 2022 00:49:33 +0800
From:   黄秋钧 <hqjagain@...il.com>
To:     keescook@...omium.org
Cc:     WeiXiong Liao <gmpy.liaowx@...il.com>, anton@...msg.org,
        ccross@...roid.com, tony.luck@...el.com,
        LKML <linux-kernel@...r.kernel.org>
Subject: [PATCH -next]pstore/zone: Preallocate zone buffer for 
 psz_kmsg_write_record

The case found when triggering a panic_on_oom, pstore fails to dump
kmsg.
Partly fixed by: commit 99b3b837855b ("pstore/zone: Use GFP_ATOMIC to
allocate zone buffer"). After the patch, it also fails sometimes. As I use
64k as kmsg_size, it's hard to get 4-order free pages sometimes at this
case. So it's better to preallocate the buffer.

Signed-off-by: Qiujun Huang <hqjagain@...il.com>
---
 fs/pstore/zone.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/fs/pstore/zone.c b/fs/pstore/zone.c
index 2770746bb7aa..86e47dbc9d53 100644
--- a/fs/pstore/zone.c
+++ b/fs/pstore/zone.c
@@ -136,6 +136,7 @@ struct psz_context {
 	struct mutex pstore_zone_info_lock;
 	struct pstore_zone_info *pstore_zone_info;
 	struct pstore_info pstore;
+	struct psz_buffer *kmsg_buf_swap;
 };
 static struct psz_context pstore_zone_cxt;
 
@@ -758,14 +759,11 @@ static inline int notrace psz_kmsg_write_record(struct psz_context *cxt,
 		if (unlikely(!zone))
 			return -ENOSPC;
 
-		/* avoid destroying old data, allocate a new one */
+		/* avoid destroying old data, use the swap buffer */
 		len = zone->buffer_size + sizeof(*zone->buffer);
 		zone->oldbuf = zone->buffer;
-		zone->buffer = kzalloc(len, GFP_ATOMIC);
-		if (!zone->buffer) {
-			zone->buffer = zone->oldbuf;
-			return -ENOMEM;
-		}
+		zone->buffer = cxt->kmsg_buf_swap;
+		memset(zone->buffer, 0, len);
 		zone->buffer->sig = zone->oldbuf->sig;
 
 		pr_debug("write %s to zone id %d\n", zone->name, zonenum);
@@ -776,15 +774,14 @@ static inline int notrace psz_kmsg_write_record(struct psz_context *cxt,
 		if (likely(!ret || ret != -ENOMSG)) {
 			cxt->kmsg_write_cnt = zonenum + 1;
 			cxt->kmsg_write_cnt %= cxt->kmsg_max_cnt;
-			/* no need to try next zone, free last zone buffer */
-			kfree(zone->oldbuf);
+			/* no need to try next zone, put last one to swap buffer */
+			cxt->kmsg_buf_swap = zone->oldbuf;
 			zone->oldbuf = NULL;
 			return ret;
 		}
 
 		pr_debug("zone %u may be broken, try next dmesg zone\n",
 				zonenum);
-		kfree(zone->buffer);
 		zone->buffer = zone->oldbuf;
 		zone->oldbuf = NULL;
 	}
@@ -1373,6 +1370,12 @@ int register_pstore_zone(struct pstore_zone_info *info)
 			err = -ENOMEM;
 			goto fail_free;
 		}
+
+		cxt->kmsg_buf_swap = kzalloc(info->kmsg_size, GFP_KERNEL);
+		if (!cxt->kmsg_buf_swap) {
+			err = -ENOMEM;
+			goto fail_free;
+		}
 	}
 	cxt->pstore.data = cxt;
 
@@ -1411,6 +1414,8 @@ int register_pstore_zone(struct pstore_zone_info *info)
 	return 0;
 
 fail_free:
+	kfree(cxt->kmsg_buf_swap);
+	cxt->kmsg_buf_swap = NULL;
 	kfree(cxt->pstore.buf);
 	cxt->pstore.buf = NULL;
 	cxt->pstore.bufsize = 0;
@@ -1445,6 +1450,9 @@ void unregister_pstore_zone(struct pstore_zone_info *info)
 	flush_delayed_work(&psz_cleaner);
 
 	/* Clean up allocations. */
+	kfree(cxt->kmsg_buf_swap);
+	cxt->kmsg_buf_swap = NULL;
+
 	kfree(cxt->pstore.buf);
 	cxt->pstore.buf = NULL;
 	cxt->pstore.bufsize = 0;
-- 
2.30.2