From patchwork Thu Oct 26 17:07:28 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 13437748 Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A92C2381C4 for ; Thu, 26 Oct 2023 17:07:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="jDsHeG0e" Received: from mail-pf1-x435.google.com (mail-pf1-x435.google.com [IPv6:2607:f8b0:4864:20::435]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 17A9F1B9 for ; Thu, 26 Oct 2023 10:07:33 -0700 (PDT) Received: by mail-pf1-x435.google.com with SMTP id d2e1a72fcca58-6be0277c05bso1078246b3a.0 for ; Thu, 26 Oct 2023 10:07:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1698340052; x=1698944852; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=b6uwa1NQ3b5So6ZqC1jDfFnZs3E6+OcPbQsril5qkDE=; b=jDsHeG0eV/PVWEpkwiV7KyjG3d+bbr4szNp1BE2YWFuVFrl0VmhMjLk34b7Iv+u0LV K7WPas9ItsasMtIs28unrbP6gVrc1EWDMDW+Royy9L8iAr2LO47ivfyXsjTHB4DGgGbB DMUPhDoShJr3hBckPhO3t7K54woFOBkAB6S24= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698340052; x=1698944852; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=b6uwa1NQ3b5So6ZqC1jDfFnZs3E6+OcPbQsril5qkDE=; b=qcOEe/pO4Vt8UsDg5wBXto9MwIrldbuY16QHcOYH30AlUPMXoQdjjEmMlspxq4RaYo R6zbfHqKRLUSoT5fFDcHXRsXaXcFvzEs4d5v3QXZ8A4kqvA2ZwUWmLGDOzUB6vpr1zgZ nVYFAxd8UbGGkmryYm8TXVS2hzVabKakD+9t4hs3IpFVDiAiFpWCG3oLi0TKf1791B2d L6fAottND/NhB/fviO0cbnXDgZjDPZgoeU6tn7/9jucmIPcfTZTd18a4Z80zLznQOeE/ /gKPvs8GCd0tvViD4vtEKXAC/Dt3OlAhGXQWzGR9Xs01NXjDNygjr+LPJtg7RwMm2//L 8Gcw== X-Gm-Message-State: AOJu0YyJ4b0JmBRMYC9IkP7qKGPOVXLskCTp5RQ1uWv41qFIBD0bUc2D YKV2dqkoDl3ph3karhmldGo/Iw== X-Google-Smtp-Source: AGHT+IE7hpmLQqEysEfdJXUltg5QC3Oo8kQXtpDePhBsrIx9j6UHm4qiSQH81VcLVMZ/nIfcqpCBIA== X-Received: by 2002:a05:6a20:3d2a:b0:15d:684d:f514 with SMTP id y42-20020a056a203d2a00b0015d684df514mr537837pzi.6.1698340052455; Thu, 26 Oct 2023 10:07:32 -0700 (PDT) Received: from www.outflux.net (198-0-35-241-static.hfc.comcastbusiness.net. [198.0.35.241]) by smtp.gmail.com with ESMTPSA id z16-20020a656110000000b005b929dc2d25sm1351597pgu.10.2023.10.26.10.07.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 26 Oct 2023 10:07:32 -0700 (PDT) From: Kees Cook To: Steven Rostedt Cc: Kees Cook , "Matthew Wilcox (Oracle)" , Christoph Hellwig , Justin Stitt , Kent Overstreet , Petr Mladek , Andy Shevchenko , Rasmus Villemoes , Sergey Senozhatsky , Masami Hiramatsu , Greg Kroah-Hartman , Arnd Bergmann , Jonathan Corbet , Yun Zhou , Jacob Keller , Zhen Lei , linux-trace-kernel@vger.kernel.org, Yosry Ahmed , linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org Subject: [PATCH] seq_buf: Introduce DECLARE_SEQ_BUF and seq_buf_cstr() Date: Thu, 26 Oct 2023 10:07:28 -0700 Message-Id: <20231026170722.work.638-kees@kernel.org> X-Mailer: git-send-email 2.34.1 Precedence: bulk X-Mailing-List: linux-hardening@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=4488; i=keescook@chromium.org; h=from:subject:message-id; bh=VSeNn7/RMcdVPleKpkEjvHM85UsfX0np5UVWhRNzd8U=; b=owEBbQKS/ZANAwAKAYly9N/cbcAmAcsmYgBlOpzQ5loxdyuUGtxqQcAlDxcpVgVq53LwrCQag bnMFdQY2dKJAjMEAAEKAB0WIQSlw/aPIp3WD3I+bhOJcvTf3G3AJgUCZTqc0AAKCRCJcvTf3G3A JrXtD/9QYO6GmmwzvTM0xFkPaw27QP97gXGJGIxv631wEy5F56f10I8PqEzoILEJzIU9s/s/vaJ 60Oxo5TGL1aMtHG3kadZSJnyOV+zj244PintWD18FXWAS/OhBKlPEYYGoWdCC+6xSQePk+m1o6H 3H/U94d2dodigsjd4qCFswPoK/Ukdmt6Ks9zp5aUMHcH2IOLCHx47xag1gQ508t/+9xy43Ox8HP bk0diEv0qqOKGBOnQv+cl9PhK3ciQnK3W9Fri1XapmlpesIlz/IxV3bVk+S6Y9MnYvlUi0zrLEN UgJbiBxkVgsMSbNig4uQWkUh9mj/LTlPBvDKm1/nHNSu3KZb0ialmZVZE9CxEAgj0W3JL84IZfT 8dEGsOw62OfrkVAuLnZc0k8K33Vhxj4QO4kLFK//Oml7I0lUdx3Abw85jgO/NJQds9CByzUnx0M 51YrrF4h+edaHU1h+Qrqh5kIbOHVB5DocI4mDSrN0mZH/1E/Z6XrGxkhIo/qocyEUj00DIY5x+V a4lK+sascEFpR25yMi3gIYEtSL+TE10P1zGfoBpNQYpIxKSDRiFfj7Vew7TLfQuGd+qrFqdG2/w xpHdSXIUUXpgvoFOqoWGRz3qdfuOq8+qMkHIPjnnA3Rso7zZBeUDHhRfwxu9n9+JqRtL5gy5Xg8 juLsepS NEfQ04kA== X-Developer-Key: i=keescook@chromium.org; a=openpgp; fpr=A5C3F68F229DD60F723E6E138972F4DFDC6DC026 Solve two ergonomic issues with struct seq_buf: 1) Too much boilerplate is required to initialize: struct seq_buf s; char buf[32]; seq_buf_init(s, buf, sizeof(buf)); Instead, we can build this directly on the stack. Provide DECLARE_SEQ_BUF() macro to do this: DECLARE_SEQ_BUF(s, 32); 2) %NUL termination is fragile and requires 2 steps to get a valid C String (and is a layering violation exposing the "internals" of seq_buf): seq_buf_terminate(s); do_something(s->buffer); Instead, we can just return s->buffer direction after terminating it in refactored seq_buf_terminate(), now known as seq_buf_cstr(): do_soemthing(seq_buf_cstr(s)); Cc: Steven Rostedt Cc: "Matthew Wilcox (Oracle)" Cc: Christoph Hellwig Cc: Justin Stitt Cc: Kent Overstreet Cc: Petr Mladek Cc: Andy Shevchenko Cc: Rasmus Villemoes Cc: Sergey Senozhatsky Cc: Masami Hiramatsu Cc: Greg Kroah-Hartman Cc: Arnd Bergmann Cc: Jonathan Corbet Cc: Yun Zhou Cc: Jacob Keller Cc: Zhen Lei Cc: linux-trace-kernel@vger.kernel.org Signed-off-by: Kees Cook --- include/linux/seq_buf.h | 19 +++++++++++++++---- kernel/trace/trace.c | 11 +---------- lib/seq_buf.c | 4 +--- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h index 8483e4b2d0d2..8896b830eb3d 100644 --- a/include/linux/seq_buf.h +++ b/include/linux/seq_buf.h @@ -21,9 +21,16 @@ struct seq_buf { size_t len; }; +#define DECLARE_SEQ_BUF(NAME, SIZE) \ + char __ ## NAME ## _buffer[SIZE] = ""; \ + struct seq_buf NAME = { .buffer = &__ ## NAME ## _buffer, \ + .size = SIZE } + static inline void seq_buf_clear(struct seq_buf *s) { s->len = 0; + if (s->size) + s->buffer[0] = '\0'; } static inline void @@ -69,8 +76,8 @@ static inline unsigned int seq_buf_used(struct seq_buf *s) } /** - * seq_buf_terminate - Make sure buffer is nul terminated - * @s: the seq_buf descriptor to terminate. + * seq_buf_cstr - get %NUL-terminated C string from seq_buf + * @s: the seq_buf handle * * This makes sure that the buffer in @s is nul terminated and * safe to read as a string. @@ -81,16 +88,20 @@ static inline unsigned int seq_buf_used(struct seq_buf *s) * * After this function is called, s->buffer is safe to use * in string operations. + * + * Returns @s->buf after making sure it is terminated. */ -static inline void seq_buf_terminate(struct seq_buf *s) +static inline char *seq_buf_cstr(struct seq_buf *s) { if (WARN_ON(s->size == 0)) - return; + return ""; if (seq_buf_buffer_left(s)) s->buffer[s->len] = 0; else s->buffer[s->size - 1] = 0; + + return s->buffer; } /** diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index d629065c2383..d83f36dc4bf8 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -3828,15 +3828,6 @@ static bool trace_safe_str(struct trace_iterator *iter, const char *str, return false; } -static const char *show_buffer(struct trace_seq *s) -{ - struct seq_buf *seq = &s->seq; - - seq_buf_terminate(seq); - - return seq->buffer; -} - static DEFINE_STATIC_KEY_FALSE(trace_no_verify); static int test_can_verify_check(const char *fmt, ...) @@ -3976,7 +3967,7 @@ void trace_check_vprintf(struct trace_iterator *iter, const char *fmt, */ if (WARN_ONCE(!trace_safe_str(iter, str, star, len), "fmt: '%s' current_buffer: '%s'", - fmt, show_buffer(&iter->seq))) { + fmt, seq_buf_cstr(&iter->seq.seq))) { int ret; /* Try to safely read the string */ diff --git a/lib/seq_buf.c b/lib/seq_buf.c index b7477aefff53..165caed5a74e 100644 --- a/lib/seq_buf.c +++ b/lib/seq_buf.c @@ -109,9 +109,7 @@ void seq_buf_do_printk(struct seq_buf *s, const char *lvl) if (s->size == 0 || s->len == 0) return; - seq_buf_terminate(s); - - start = s->buffer; + start = seq_buf_cstr(s); while ((lf = strchr(start, '\n'))) { int len = lf - start + 1;