Message ID | 20230807-arch-um-v1-1-86dbbfb59709@google.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:c44e:0:b0:3f2:4152:657d with SMTP id w14csp1725650vqr; Mon, 7 Aug 2023 14:40:28 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHMkhKWCJEMZ1fpIQNjyWB5FWpn6erXTE6/03Sj7p8PXOYnlbXT4tgTuQn296zgsKT70Ztm X-Received: by 2002:a17:906:54:b0:99b:44aa:fae0 with SMTP id 20-20020a170906005400b0099b44aafae0mr8873247ejg.21.1691444427718; Mon, 07 Aug 2023 14:40:27 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1691444427; cv=none; d=google.com; s=arc-20160816; b=QNVRCQ8fpYpQg7dQMaMbAqI3Veb8hCoVfX+jpK4Sg3D65si88FKmzuRqpYRk/CpkY3 rbOaS01QkWeBn0/ouirM/9qqxkxuVIlRAkDmw95sgM0sr19wmz9MiwmeVHx2xw7LZSmR CWsVEIiiHfoDZPGRZSA1rHndc1+YKE5nJjpEjz3NkzTIVy/SfHX+NEkK9AJ+8YH7ZtCI mw7qq584iAixK6mCei8CkJgKcG5L9lkfkTtsDwQMgejphEbunebSAQiPkKpHuIV7JG7U aalpYxuMHRPZTqfC+hu5nke3wvNoWjztJGESGk4EKfTfEsXlib1dmsRKxfDokkFnqunE jVwg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:from:subject:message-id:mime-version:date :dkim-signature; bh=2mfCIyPm2aO7x0iXfdEGub/+bSKhcssAHTeROw2F55I=; fh=5CjBLHHLBpRUG43U/juo4qKf9zk9/H/LW2OLn0k0Pbk=; b=VEzNbly3ncgCSmaHLnJsVp/49hnTNU+RL7MKzby5PZ2W82Alc4V3dZm4BH6lmqB7pL EssrugM78PbZ/3Vd7rLctuT1Viq51hTGIvoRVwmTbZAlSqOarMs2FRaf8B9jdCcUYFQu 2zGIeHjbeZ2h7/KTJioVOBA69dWHLnuClSpwy/pUjdfFDjnH1bTso7cnEzrttmJLTJA+ 8n7qADLMU80R7mB82pzZupVk5N4UIyY2I0liY927Pav+qpwJ76TXJMmHQDEjQqzYZJ6o tJww+DT597ljnsa+btr1vWcOh5LYb3SzKGkLDY65C3bQm3ccZhFw9mLas7WQv52/yD6S fmpQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20221208 header.b=iXvXoonI; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id xa17-20020a170907b9d100b0099b7211b2c2si5754022ejc.706.2023.08.07.14.40.03; Mon, 07 Aug 2023 14:40:27 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20221208 header.b=iXvXoonI; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229631AbjHGVSK (ORCPT <rfc822;aaronkmseo@gmail.com> + 99 others); Mon, 7 Aug 2023 17:18:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52578 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229996AbjHGVSI (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 7 Aug 2023 17:18:08 -0400 Received: from mail-oa1-x49.google.com (mail-oa1-x49.google.com [IPv6:2001:4860:4864:20::49]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 255EEE68 for <linux-kernel@vger.kernel.org>; Mon, 7 Aug 2023 14:18:07 -0700 (PDT) Received: by mail-oa1-x49.google.com with SMTP id 586e51a60fabf-1bf94d4d2f4so6503337fac.1 for <linux-kernel@vger.kernel.org>; Mon, 07 Aug 2023 14:18:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1691443085; x=1692047885; h=cc:to:from:subject:message-id:mime-version:date:from:to:cc:subject :date:message-id:reply-to; bh=2mfCIyPm2aO7x0iXfdEGub/+bSKhcssAHTeROw2F55I=; b=iXvXoonIYBdhMFviyOkoygmsJIyTGeBeq4/AOnBxytLBvQQhjalGqFzg7pCH7KQxfF eKRi5WxgelqBpHePErOT2Bt5zN88BKI9miy34Avnyz48BfPkNp3qp1t/NvRGeLfMSMBu ZKa+3pwQi71GXW9fVgOgw5fIB079W+CRsJHTp+GUGaJsCoY+4M1QuK58KCYVKIgKNQhZ MDzIdOWmfwnJvoSd7P4iHZp6p3yo4EwP6d7k43C+57isSu04UsZzVLz9k1QVhs9iHeMp JDzFnnEmAS8cBGV9g1gv2aE9flzX+CV9jCKD2QZUmxpSkE8tujbKDIBT7w8/Lgv1ixP2 1iOg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691443085; x=1692047885; h=cc:to:from:subject:message-id:mime-version:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=2mfCIyPm2aO7x0iXfdEGub/+bSKhcssAHTeROw2F55I=; b=e/OkIZZFovgapgdV1o+hx2igl+Dj+eoscXiMdX9182gOyTVS3/03ylLY7T5/DHwmvJ DnuGMFpTC1K09/uYwkGk/TqRv+uppfF5dYpRxMli1owjiEt8TSZ0QetaPBVXbzokwhrS Zf94IZkBQeAAC3cjUQGEDo75pgj1kyt/0UJU87gwS1k/uQYe/UzGbZ4ckPz8fz5y6thy bFuJnHWASPwWecf4kxQTrRu8r6F6CL+DINZeAqJvxMFOWTiuoiwn1GeCZxfiBRgU7h+9 hX/jDJE2ST6PE/bIuNK4G8b4as7zPyWYJZDDvaFguOW4+05wp6/uRmexcE0Tc++0UolM pc5A== X-Gm-Message-State: AOJu0YwWFrM23nGjuejJu4Gfl9scjIMfA83WACyoohATS0/sguHQhHGK 4+GwmZS9sgldN+boLMzNdRcSGHuDSAJDij62xg== X-Received: from jstitt-linux1.c.googlers.com ([fda3:e722:ac3:cc00:2b:ff92:c0a8:23b5]) (user=justinstitt job=sendgmr) by 2002:a05:6870:3a0f:b0:1bf:d7b9:9960 with SMTP id du15-20020a0568703a0f00b001bfd7b99960mr7527734oab.2.1691443085255; Mon, 07 Aug 2023 14:18:05 -0700 (PDT) Date: Mon, 07 Aug 2023 21:17:56 +0000 Mime-Version: 1.0 X-B4-Tracking: v=1; b=H4sIAINf0WQC/6tWKk4tykwtVrJSqFYqSi3LLM7MzwNyDHUUlJIzE vPSU3UzU4B8JSMDI2MDCwNz3cSi5Azd0lxd49Q0IxMTQ2MTI/NUJaDqgqLUtMwKsEnRsbW1AHb 5zV9ZAAAA X-Developer-Key: i=justinstitt@google.com; a=ed25519; pk=tC3hNkJQTpNX/gLKxTNQKDmiQl6QjBNCGKJINqAdJsE= X-Developer-Signature: v=1; a=ed25519-sha256; t=1691443084; l=1886; i=justinstitt@google.com; s=20230717; h=from:subject:message-id; bh=29NvDKooXX0ph89b44s4wsreDRVLQpWCZn8WSLtPy8Y=; b=E9ZtbxM3R6Zqci4MF+SDmmhn3TxUgdCPU66xW4h3e6fKqBfhSZq9iL/ienwg97ORyGw56fXX+ 9sD/afEHAV+BT4wvqLe0apqEsuO1EjSbbcyG+J/cbR93lW9E0n3XX51 X-Mailer: b4 0.12.3 Message-ID: <20230807-arch-um-v1-1-86dbbfb59709@google.com> Subject: [PATCH] um: refactor deprecated strncpy to strtomem From: Justin Stitt <justinstitt@google.com> To: Richard Weinberger <richard@nod.at>, Anton Ivanov <anton.ivanov@cambridgegreys.com>, Johannes Berg <johannes@sipsolutions.net> Cc: linux-hardening@vger.kernel.org, linux-um@lists.infradead.org, linux-kernel@vger.kernel.org, Kees Cook <keescook@chromium.org>, Justin Stitt <justinstitt@google.com> Content-Type: text/plain; charset="utf-8" X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1773608031866585530 X-GMAIL-MSGID: 1773608031866585530 |
Series |
um: refactor deprecated strncpy to strtomem
|
|
Commit Message
Justin Stitt
Aug. 7, 2023, 9:17 p.m. UTC
Use `strtomem` here since `console_buf` is not expected to be
NUL-terminated. We should probably also just use `MCONSOLE_MAX_DATA`
instead of using `ARRAY_SIZE()` for every iteration of the loop.
Also mark char buffer as `__nonstring` as per Kees' suggestion here [1]
Finally, follow checkpatch's recommendation of using `min_t` over `min`
Link: https://github.com/KSPP/linux/issues/90 [1]
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
Notes:
I only build tested this patch.
---
arch/um/drivers/mconsole_kern.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
---
base-commit: c1a515d3c0270628df8ae5f5118ba859b85464a2
change-id: 20230807-arch-um-3ef24413427e
Best regards,
--
Justin Stitt <justinstitt@google.com>
Comments
On Mon, Aug 7, 2023 at 2:18 PM Justin Stitt <justinstitt@google.com> wrote: > > Use `strtomem` here since `console_buf` is not expected to be > NUL-terminated. We should probably also just use `MCONSOLE_MAX_DATA` > instead of using `ARRAY_SIZE()` for every iteration of the loop. > Is this change necessary? I have a general preference for ARRAY_SIZE, because a change in size is less likely to be overlooked (unless that goes against the coding standard). > Also mark char buffer as `__nonstring` as per Kees' suggestion here [1] > > Finally, follow checkpatch's recommendation of using `min_t` over `min` > > Link: https://github.com/KSPP/linux/issues/90 [1] > Cc: linux-hardening@vger.kernel.org > Signed-off-by: Justin Stitt <justinstitt@google.com> > --- > Notes: > I only build tested this patch. > --- > arch/um/drivers/mconsole_kern.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/arch/um/drivers/mconsole_kern.c b/arch/um/drivers/mconsole_kern.c > index 5026e7b9adfe..fd4c024202ae 100644 > --- a/arch/um/drivers/mconsole_kern.c > +++ b/arch/um/drivers/mconsole_kern.c > @@ -4,6 +4,7 @@ > * Copyright (C) 2001 - 2008 Jeff Dike (jdike@{addtoit,linux.intel}.com) > */ > > +#include "linux/compiler_attributes.h" nit: Should this include be in angle brackets? #include <linux/compiler_attributes.h> > #include <linux/console.h> > #include <linux/ctype.h> > #include <linux/string.h> > @@ -554,7 +555,7 @@ struct mconsole_output { > > static DEFINE_SPINLOCK(client_lock); > static LIST_HEAD(clients); > -static char console_buf[MCONSOLE_MAX_DATA]; > +static char console_buf[MCONSOLE_MAX_DATA] __nonstring; > > static void console_write(struct console *console, const char *string, > unsigned int len) > @@ -566,8 +567,8 @@ static void console_write(struct console *console, const char *string, > return; > > while (len > 0) { > - n = min((size_t) len, ARRAY_SIZE(console_buf)); > - strncpy(console_buf, string, n); > + n = min_t(size_t, len, MCONSOLE_MAX_DATA); > + strtomem(console_buf, string); > string += n; > len -= n; > > > --- > base-commit: c1a515d3c0270628df8ae5f5118ba859b85464a2 > change-id: 20230807-arch-um-3ef24413427e > > Best regards, > -- > Justin Stitt <justinstitt@google.com> >
On Mon, Aug 07, 2023 at 03:36:55PM -0700, Bill Wendling wrote: > On Mon, Aug 7, 2023 at 2:18 PM Justin Stitt <justinstitt@google.com> wrote: > > > > Use `strtomem` here since `console_buf` is not expected to be > > NUL-terminated. We should probably also just use `MCONSOLE_MAX_DATA` How is it known that console_buf is not a C-string? > > instead of using `ARRAY_SIZE()` for every iteration of the loop. > > > Is this change necessary? I have a general preference for ARRAY_SIZE, > because a change in size is less likely to be overlooked (unless that > goes against the coding standard). I would prefer this stay either ARRAY_SIZE or sizeof, as it keeps it tied to the variable in question. > > > Also mark char buffer as `__nonstring` as per Kees' suggestion here [1] > > > > Finally, follow checkpatch's recommendation of using `min_t` over `min` > > > > Link: https://github.com/KSPP/linux/issues/90 [1] > > Cc: linux-hardening@vger.kernel.org > > Signed-off-by: Justin Stitt <justinstitt@google.com> > > --- > > Notes: > > I only build tested this patch. > > --- > > arch/um/drivers/mconsole_kern.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/arch/um/drivers/mconsole_kern.c b/arch/um/drivers/mconsole_kern.c > > index 5026e7b9adfe..fd4c024202ae 100644 > > --- a/arch/um/drivers/mconsole_kern.c > > +++ b/arch/um/drivers/mconsole_kern.c > > @@ -4,6 +4,7 @@ > > * Copyright (C) 2001 - 2008 Jeff Dike (jdike@{addtoit,linux.intel}.com) > > */ > > > > +#include "linux/compiler_attributes.h" > > nit: Should this include be in angle brackets? > > #include <linux/compiler_attributes.h> True, though this shouldn't need to be included at all. What was missing? > > > #include <linux/console.h> > > #include <linux/ctype.h> > > #include <linux/string.h> > > @@ -554,7 +555,7 @@ struct mconsole_output { > > > > static DEFINE_SPINLOCK(client_lock); > > static LIST_HEAD(clients); > > -static char console_buf[MCONSOLE_MAX_DATA]; > > +static char console_buf[MCONSOLE_MAX_DATA] __nonstring; > > > > static void console_write(struct console *console, const char *string, > > unsigned int len) > > @@ -566,8 +567,8 @@ static void console_write(struct console *console, const char *string, > > return; > > > > while (len > 0) { > > - n = min((size_t) len, ARRAY_SIZE(console_buf)); > > - strncpy(console_buf, string, n); > > + n = min_t(size_t, len, MCONSOLE_MAX_DATA); > > + strtomem(console_buf, string); > > string += n; > > len -= n; > > > > > > --- > > base-commit: c1a515d3c0270628df8ae5f5118ba859b85464a2 > > change-id: 20230807-arch-um-3ef24413427e > > > > Best regards, > > -- > > Justin Stitt <justinstitt@google.com> > >
On Mon, Aug 7, 2023 at 4:40 PM Kees Cook <keescook@chromium.org> wrote: > > On Mon, Aug 07, 2023 at 03:36:55PM -0700, Bill Wendling wrote: > > On Mon, Aug 7, 2023 at 2:18 PM Justin Stitt <justinstitt@google.com> wrote: > > > > > > Use `strtomem` here since `console_buf` is not expected to be > > > NUL-terminated. We should probably also just use `MCONSOLE_MAX_DATA` > > How is it known that console_buf is not a C-string? There are a few clues that led me to believe console_buf was not a C-string: 1) `n = min((size_t) len, ARRAY_SIZE(console_buf));` means that `n` can be equal to size of buffer which is then subsequently filled entirely by the `strncpy` invocation. 2) console_buf looks to be a circular buffer wherein once it's filled it starts again from the beginning. 3) ARRAY_SIZE is used on it as opposed to strlen or something like that (but not sure if ARRAY_SIZE is also used on C-strings to be fair) 4) It has `buf` in its name which I loosely associate with non C-strings char buffers. All in all, it looks to be a non C-string but honestly it's hard to tell, especially since if it IS a C-string the previous implementation had some bugs with strncpy I believe. > > > > instead of using `ARRAY_SIZE()` for every iteration of the loop. > > > > > Is this change necessary? I have a general preference for ARRAY_SIZE, > > because a change in size is less likely to be overlooked (unless that > > goes against the coding standard). > > I would prefer this stay either ARRAY_SIZE or sizeof, as it keeps it > tied to the variable in question. > > > > > > Also mark char buffer as `__nonstring` as per Kees' suggestion here [1] > > > > > > Finally, follow checkpatch's recommendation of using `min_t` over `min` > > > > > > Link: https://github.com/KSPP/linux/issues/90 [1] > > > Cc: linux-hardening@vger.kernel.org > > > Signed-off-by: Justin Stitt <justinstitt@google.com> > > > --- > > > Notes: > > > I only build tested this patch. > > > --- > > > arch/um/drivers/mconsole_kern.c | 7 ++++--- > > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > > > diff --git a/arch/um/drivers/mconsole_kern.c b/arch/um/drivers/mconsole_kern.c > > > index 5026e7b9adfe..fd4c024202ae 100644 > > > --- a/arch/um/drivers/mconsole_kern.c > > > +++ b/arch/um/drivers/mconsole_kern.c > > > @@ -4,6 +4,7 @@ > > > * Copyright (C) 2001 - 2008 Jeff Dike (jdike@{addtoit,linux.intel}.com) > > > */ > > > > > > +#include "linux/compiler_attributes.h" > > > > nit: Should this include be in angle brackets? > > > > #include <linux/compiler_attributes.h> > > True, though this shouldn't need to be included at all. What was > missing? > > > > > > #include <linux/console.h> > > > #include <linux/ctype.h> > > > #include <linux/string.h> > > > @@ -554,7 +555,7 @@ struct mconsole_output { > > > > > > static DEFINE_SPINLOCK(client_lock); > > > static LIST_HEAD(clients); > > > -static char console_buf[MCONSOLE_MAX_DATA]; > > > +static char console_buf[MCONSOLE_MAX_DATA] __nonstring; > > > > > > static void console_write(struct console *console, const char *string, > > > unsigned int len) > > > @@ -566,8 +567,8 @@ static void console_write(struct console *console, const char *string, > > > return; > > > > > > while (len > 0) { > > > - n = min((size_t) len, ARRAY_SIZE(console_buf)); > > > - strncpy(console_buf, string, n); > > > + n = min_t(size_t, len, MCONSOLE_MAX_DATA); > > > + strtomem(console_buf, string); > > > string += n; > > > len -= n; > > > > > > > > > --- > > > base-commit: c1a515d3c0270628df8ae5f5118ba859b85464a2 > > > change-id: 20230807-arch-um-3ef24413427e > > > > > > Best regards, > > > -- > > > Justin Stitt <justinstitt@google.com> > > > > > -- > Kees Cook
On Tue, Aug 08, 2023 at 10:28:57AM -0700, Justin Stitt wrote: > On Mon, Aug 7, 2023 at 4:40 PM Kees Cook <keescook@chromium.org> wrote: > > > > On Mon, Aug 07, 2023 at 03:36:55PM -0700, Bill Wendling wrote: > > > On Mon, Aug 7, 2023 at 2:18 PM Justin Stitt <justinstitt@google.com> wrote: > > > > > > > > Use `strtomem` here since `console_buf` is not expected to be > > > > NUL-terminated. We should probably also just use `MCONSOLE_MAX_DATA` > > > > How is it known that console_buf is not a C-string? > There are a few clues that led me to believe console_buf was not a C-string: > 1) `n = min((size_t) len, ARRAY_SIZE(console_buf));` means that `n` > can be equal to size of buffer which is then subsequently filled > entirely by the `strncpy` invocation. > 2) console_buf looks to be a circular buffer wherein once it's filled > it starts again from the beginning. > 3) ARRAY_SIZE is used on it as opposed to strlen or something like > that (but not sure if ARRAY_SIZE is also used on C-strings to be fair) > 4) It has `buf` in its name which I loosely associate with non > C-strings char buffers. > > All in all, it looks to be a non C-string but honestly it's hard to > tell, especially since if it IS a C-string the previous implementation > had some bugs with strncpy I believe. I took a look at this. It's only used in that one function, and it's always working from the start, and uses at max ARRAY_SIZE(console_buf), as you mentioned. Then it's passed to mconsole_reply_len() with "len", so we can examine that: do { ... len = MIN(total, MCONSOLE_MAX_DATA - 1); ... memcpy(reply.data, str, len); reply.data[len] = '\0'; total -= len; ... } while (total > 0); It's sending it as NUL-terminated, possibly breaking it up. If this used the whole MCONSOLE_MAX_DATA, it would send MCONSOLE_MAX_DATA - 1 bytes followed by NUL, and then 1 byte, followed by NUL. :P Anyway, point being, yes, it appears that console_buf is a nonstring. But it's a weird one... In your v2 patch, you use strtomem(), which is probably close, but in looking at the implementation details here, I'm starting to think that strtomem() needs to return the number of bytes copied. Initially I was thinking it could actually just be replaced with memcpy(), but there are some side-effects going on in the original code. First: static void console_write(..., const char *string, unsigned int len) ... n = min((size_t) len, ARRAY_SIZE(console_buf)); strncpy(console_buf, string, n); The 'n' is being passed in, so it's possible that "string" has NUL-bytes in it. (Though I would assume, unlikely -- I haven't tracked down the callers of console_write() here...) That means that strncpy() will stop the copy at the first NUL and then NUL pad the rest to 'n', and that's what gets passed down to mconsole_reply_len() (which is specifically using memcpy and will carry those NUL bytes forward). So we should not lose the NUL-padding behavior. Additionally, when "len" is smaller than MCONSOLE_MAX_DATA, the padding will only go up to "len", not MCONSOLE_MAX_DATA. So there's a weird behavioral difference here where the new code is doing more work, but, okay fine. I find this original code to be rather confused about whether it is dealing with C-strings or byte arrays. :| So now I wanted to find the callers of struct console::write. My Coccinelle script: @found@ struct console *CON; @@ * CON->write(...) show hits in kernel/printk/printk.c, which is dealing with a NUL-terminated string constructed by vsnprintf(): console_emit_next_record Technically there could be NUL bytes in such a string, but almost everything else expects to be dealing with C-strings here. This looks really fragile. So, I'm back around to thinking this should just be memcpy(): - strncpy(console_buf, string, n); + memcpy(console_buf, string, n);
diff --git a/arch/um/drivers/mconsole_kern.c b/arch/um/drivers/mconsole_kern.c index 5026e7b9adfe..fd4c024202ae 100644 --- a/arch/um/drivers/mconsole_kern.c +++ b/arch/um/drivers/mconsole_kern.c @@ -4,6 +4,7 @@ * Copyright (C) 2001 - 2008 Jeff Dike (jdike@{addtoit,linux.intel}.com) */ +#include "linux/compiler_attributes.h" #include <linux/console.h> #include <linux/ctype.h> #include <linux/string.h> @@ -554,7 +555,7 @@ struct mconsole_output { static DEFINE_SPINLOCK(client_lock); static LIST_HEAD(clients); -static char console_buf[MCONSOLE_MAX_DATA]; +static char console_buf[MCONSOLE_MAX_DATA] __nonstring; static void console_write(struct console *console, const char *string, unsigned int len) @@ -566,8 +567,8 @@ static void console_write(struct console *console, const char *string, return; while (len > 0) { - n = min((size_t) len, ARRAY_SIZE(console_buf)); - strncpy(console_buf, string, n); + n = min_t(size_t, len, MCONSOLE_MAX_DATA); + strtomem(console_buf, string); string += n; len -= n;