[PATCH] EDAC/thunderx: Replace strlcat() with snprintf() and seq_buf

Ian Bridges <[email protected]>
Newsgroups org.kernel.vger.linux-edac,org.kernel.vger.linux-hardening,org.kernel.vger.linux-kernel
Message-ID <akYJUdY1hVABUQH6@dev>
In preparation for removing the strlcat() API[1], replace its uses in
the error message construction of the ThunderX OCX and L2C threaded
IRQ handlers.

The OCX link and L2C handlers append a single decoded-register string,
so the whole message can be produced with one snprintf(). The OCX com
handler accumulates a variable number of fragments in a per-lane loop,
so use a struct seq_buf, which tracks the current write position and
remaining space internally. seq_buf_str() keeps the message
NUL-terminated for edac_device_handle_ce().

Link: https://github.com/KSPP/linux/issues/370 [1]
Signed-off-by: Ian Bridges <[email protected]>
---
The seq_buf conversion introduces one behavioral difference on
overflow. strlcat() copied as much of a fragment as would fit, but
seq_buf_puts() is all-or-nothing. A message longer than
OCX_MESSAGE_SIZE therefore truncates at a fragment boundary instead of
mid-fragment. The message was truncated either way.

I don't have ThunderX hardware, so I tested the patch with:
 - arm64 cross-build at W=1 with no warnings. Applies cleanly on
   v7.2-rc1.
 - Module load/unload in an arm64 QEMU guest.
 - A userspace harness comparing old and new message construction;
   outputs matched across ~500k register patterns.

 drivers/edac/thunderx_edac.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/edac/thunderx_edac.c b/drivers/edac/thunderx_edac.c
index 75c04dfc3962..5210ca18473d 100644
--- a/drivers/edac/thunderx_edac.c
+++ b/drivers/edac/thunderx_edac.c
@@ -20,6 +20,7 @@
 #include <linux/atomic.h>
 #include <linux/bitfield.h>
 #include <linux/circ_buf.h>
+#include <linux/seq_buf.h>
 
 #include <asm/page.h>
 
@@ -1108,6 +1109,7 @@ static irqreturn_t thunderx_ocx_com_threaded_isr(int irq, void *irq_id)
 	int lane;
 	char *msg;
 	char *other;
+	struct seq_buf sb;
 
 	msg = kmalloc(OCX_MESSAGE_SIZE, GFP_KERNEL);
 	other = kmalloc(OCX_OTHER_SIZE, GFP_KERNEL);
@@ -1121,13 +1123,14 @@ static irqreturn_t thunderx_ocx_com_threaded_isr(int irq, void *irq_id)
 				ARRAY_SIZE(ocx->com_err_ctx));
 		ctx = &ocx->com_err_ctx[tail];
 
-		snprintf(msg, OCX_MESSAGE_SIZE, "%s: OCX_COM_INT: %016llx",
-			ocx->edac_dev->ctl_name, ctx->reg_com_int);
+		seq_buf_init(&sb, msg, OCX_MESSAGE_SIZE);
+		seq_buf_printf(&sb, "%s: OCX_COM_INT: %016llx",
+			       ocx->edac_dev->ctl_name, ctx->reg_com_int);
 
 		decode_register(other, OCX_OTHER_SIZE,
 				ocx_com_errors, ctx->reg_com_int);
 
-		strlcat(msg, other, OCX_MESSAGE_SIZE);
+		seq_buf_puts(&sb, other);
 
 		for (lane = 0; lane < OCX_RX_LANES; lane++)
 			if (ctx->reg_com_int & BIT(lane)) {
@@ -1136,16 +1139,17 @@ static irqreturn_t thunderx_ocx_com_threaded_isr(int irq, void *irq_id)
 					 lane, ctx->reg_lane_int[lane],
 					 lane, ctx->reg_lane_stat11[lane]);
 
-				strlcat(msg, other, OCX_MESSAGE_SIZE);
+				seq_buf_puts(&sb, other);
 
 				decode_register(other, OCX_OTHER_SIZE,
 						ocx_lane_errors,
 						ctx->reg_lane_int[lane]);
-				strlcat(msg, other, OCX_MESSAGE_SIZE);
+				seq_buf_puts(&sb, other);
 			}
 
 		if (ctx->reg_com_int & OCX_COM_INT_CE)
-			edac_device_handle_ce(ocx->edac_dev, 0, 0, msg);
+			edac_device_handle_ce(ocx->edac_dev, 0, 0,
+					      seq_buf_str(&sb));
 
 		ocx->com_ring_tail++;
 	}
@@ -1203,15 +1207,13 @@ static irqreturn_t thunderx_ocx_lnk_threaded_isr(int irq, void *irq_id)
 
 		ctx = &ocx->link_err_ctx[tail];
 
-		snprintf(msg, OCX_MESSAGE_SIZE,
-			 "%s: OCX_COM_LINK_INT[%d]: %016llx",
-			 ocx->edac_dev->ctl_name,
-			 ctx->link, ctx->reg_com_link_int);
-
 		decode_register(other, OCX_OTHER_SIZE,
 				ocx_com_link_errors, ctx->reg_com_link_int);
 
-		strlcat(msg, other, OCX_MESSAGE_SIZE);
+		snprintf(msg, OCX_MESSAGE_SIZE,
+			 "%s: OCX_COM_LINK_INT[%d]: %016llx%s",
+			 ocx->edac_dev->ctl_name,
+			 ctx->link, ctx->reg_com_link_int, other);
 
 		if (ctx->reg_com_link_int & OCX_COM_LINK_INT_UE)
 			edac_device_handle_ue(ocx->edac_dev, 0, 0, msg);
@@ -1882,14 +1884,12 @@ static irqreturn_t thunderx_l2c_threaded_isr(int irq, void *irq_id)
 
 	while (CIRC_CNT(l2c->ring_head, l2c->ring_tail,
 			ARRAY_SIZE(l2c->err_ctx))) {
-		snprintf(msg, L2C_MESSAGE_SIZE,
-			 "%s: %s: %016llx, %s: %016llx",
-			 l2c->edac_dev->ctl_name, reg_int_name, ctx->reg_int,
-			 ctx->reg_ext_name, ctx->reg_ext);
-
 		decode_register(other, L2C_OTHER_SIZE, l2_errors, ctx->reg_int);
 
-		strlcat(msg, other, L2C_MESSAGE_SIZE);
+		snprintf(msg, L2C_MESSAGE_SIZE,
+			 "%s: %s: %016llx, %s: %016llx%s",
+			 l2c->edac_dev->ctl_name, reg_int_name, ctx->reg_int,
+			 ctx->reg_ext_name, ctx->reg_ext, other);
 
 		if (ctx->reg_int & mask_ue)
 			edac_device_handle_ue(l2c->edac_dev, 0, 0, msg);
-- 
2.47.3
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.