From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3F5C0C001DB for ; Mon, 7 Aug 2023 23:40:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229578AbjHGXkA (ORCPT ); Mon, 7 Aug 2023 19:40:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34522 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230141AbjHGXj7 (ORCPT ); Mon, 7 Aug 2023 19:39:59 -0400 Received: from mail-pl1-x62d.google.com (mail-pl1-x62d.google.com [IPv6:2607:f8b0:4864:20::62d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A4CC91986 for ; Mon, 7 Aug 2023 16:39:58 -0700 (PDT) Received: by mail-pl1-x62d.google.com with SMTP id d9443c01a7336-1bba54f7eefso39963025ad.1 for ; Mon, 07 Aug 2023 16:39:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1691451598; x=1692056398; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=0l+sCpHwJioTYF/wfHRNJdNAUkK21XwVbEV3jke4+es=; b=dXsftWzX5+Hu4twrL1n/Mo54tTI+wzGtnBu1CLPrmVntA19h9YdrWv34e6mRkH7DdU 1969sylXKdmjK0xJ4TqcjxyND+jT9oEGt+HDYRRKu/Cw3IOnJt/we8cjsnh3UH3SZGN1 5WfC0t1j0f5wDUm0oCa6ZXsUGiS5NcpQEHUu4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691451598; x=1692056398; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=0l+sCpHwJioTYF/wfHRNJdNAUkK21XwVbEV3jke4+es=; b=GRoHNk+RXtxNv2rRKZZKLz0MRbxEYlc6B+7Pz1If5R3K0EGQUiYFGHgr48j+zC0oEo D/HmPhhzQEekAGVA8ec7qmdtiCalw0Pmb6yjQLNI4nvljgniEkagpDH9n5bUmzLfYsLc wisU+H85uWlpEKSClVQRy2lCeR8+TjcMQRl4Q7DMju6SkHuqL4bDspxpZyvolwJ0F3II RBnlHCpATs3aCWedFf+aUM+mTr6m02pHfx+HXdzZxMRvyfr3h4I5Ec8WY6ri99jI0wAR 9FMLihvqK33YSz5XpWbnrNjmS+fhmDPPpC5a4c8rixMJkG6fi9QbZQdPNb8LzxSgKETX /bjw== X-Gm-Message-State: AOJu0YxpGDUHX3Y5/ZPh5Khin0klqpG05ZDf/lq20eAMnl8y+Pbe/x5Z sTn1Zc0LhihYEzQThTJE93nOfg== X-Google-Smtp-Source: AGHT+IGdMQVeL0uHk5iGceyUJ4IMfMXBfpY5lj7Xb2/D1CXzdymvrVtvYnilTL43ah4FuOYOA8JvkQ== X-Received: by 2002:a17:902:ec8e:b0:1bb:cd5a:ba53 with SMTP id x14-20020a170902ec8e00b001bbcd5aba53mr10052640plg.14.1691451598065; Mon, 07 Aug 2023 16:39:58 -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 s17-20020a170902ea1100b001b552309aedsm7252666plg.192.2023.08.07.16.39.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 07 Aug 2023 16:39:57 -0700 (PDT) Date: Mon, 7 Aug 2023 16:39:56 -0700 From: Kees Cook To: Bill Wendling Cc: Justin Stitt , Richard Weinberger , Anton Ivanov , Johannes Berg , linux-hardening@vger.kernel.org, linux-um@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] um: refactor deprecated strncpy to strtomem Message-ID: <202308071636.AF290F0@keescook> References: <20230807-arch-um-v1-1-86dbbfb59709@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-hardening@vger.kernel.org On Mon, Aug 07, 2023 at 03:36:55PM -0700, Bill Wendling wrote: > On Mon, Aug 7, 2023 at 2:18 PM Justin Stitt 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 > > --- > > 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 True, though this shouldn't need to be included at all. What was missing? > > > #include > > #include > > #include > > @@ -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 > > -- Kees Cook