Message ID | 20230727-sound-xen-v1-1-89dd161351f1@google.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:918b:0:b0:3e4:2afc:c1 with SMTP id s11csp55122vqg; Thu, 27 Jul 2023 15:51:54 -0700 (PDT) X-Google-Smtp-Source: APBJJlFeav07XUEoCtqILdmfHozNBbKWwupa9R8eI7FyhspxTR/xE5s7WhEq5yDiDYkyLv6UwgSO X-Received: by 2002:a17:903:2686:b0:1b8:6987:de84 with SMTP id jf6-20020a170903268600b001b86987de84mr704854plb.48.1690498314597; Thu, 27 Jul 2023 15:51:54 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1690498314; cv=none; d=google.com; s=arc-20160816; b=MudYRswDFWmcZvik6I0B6gOkKclN70k+l4YeQJJXGbELWepgUQCGX2GmoHeQcdHfLa qzm58449ZLmFKOfY3E3mQGDco8pgCGo7B6VcYVOtCr/E7yPd6UjKSvjvHKQIrvJ1HtCh ACQv9fUXpmIKPvTXoqFcLHRwc1pATYDbYUPQvIohWF2BI4EekyBKFXWFGtZKQEc/BJil O86lEJxHPrI+DwCXL0eBcvG0odwjT8PTyrZs7hXinlPyp8l9Wh+pvTVu1Y3c09uaUhdA WlJCnS5Y/lTbNUWV84eTRTmp+h85fY/2OwmqqJKe9ieb2mtnvKKIKpb6NUaGJVPNpDz9 J92A== 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=+IrV9GMRSdZPt30c7KcILm3qep9zKx/623+6CGNnSZI=; fh=fx4/YJ4BUsAqvaVP8sFw/EFUV9viCiTo0pgz6rCJC20=; b=Sjo6MUm6oGrfdLPV92FtpghDgkI/Zv1gJMCef16tjnZhLkao7whVmAjTk3lOt9H1gx ueDw/LccIRvTEdTqauU8F7WPre5qjDr3+J+Gty9iCvZvLxpAUTIMSwTabd6Gy5Wx9exV 821CAkkAzk9OP7xjNHYOlLCMSb7yz4wItjQPrVkwQr3DVTh4JVOfzQzR5iOT0AlR0R/V 8GPbMHtWEGVPYC5ioTptC/LkrfsX6YY1iePozigsBTJSG5b4vOpwrDQeNcYOy58SNoUy vbiHX1V61RVW+kaG+E3ndca0Lh+RqGsnMBMM76tPOojnrVip6VvCKMlNBiTJnbOdfpCi 894w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20221208 header.b=YRH0Rand; 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 kt6-20020a170903088600b001baff05d890si1830203plb.313.2023.07.27.15.51.42; Thu, 27 Jul 2023 15:51:54 -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=YRH0Rand; 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 S232665AbjG0Vyc (ORCPT <rfc822;kloczko.tomasz@gmail.com> + 99 others); Thu, 27 Jul 2023 17:54:32 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33122 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232656AbjG0VyL (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 27 Jul 2023 17:54:11 -0400 Received: from mail-yb1-xb4a.google.com (mail-yb1-xb4a.google.com [IPv6:2607:f8b0:4864:20::b4a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5F45F423B for <linux-kernel@vger.kernel.org>; Thu, 27 Jul 2023 14:53:35 -0700 (PDT) Received: by mail-yb1-xb4a.google.com with SMTP id 3f1490d57ef6-d13e11bb9ecso1253963276.0 for <linux-kernel@vger.kernel.org>; Thu, 27 Jul 2023 14:53:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1690494814; x=1691099614; h=cc:to:from:subject:message-id:mime-version:date:from:to:cc:subject :date:message-id:reply-to; bh=+IrV9GMRSdZPt30c7KcILm3qep9zKx/623+6CGNnSZI=; b=YRH0RandNbmWOsNbix1juf2j32ITK9Uvj67kgr3R72979X637zKHH8KZguOPLopzPy fjkQbcWwTfMo1Hqb4p+zc/kFemN21RJ6rTyJtcMQ/oEKwjHKIFtmzqeJZgnBmRq/79FF Q1T9puk7xzNEdjIV/igXmV4UFIlev2+Onajgaupn8iFIWwD+LDMaU7Rqh6Y/qZ8WNZcQ EbYgsmXlOOtwiPgFmIy/5TsvUTsSImKFRVPRiBPAtXKPd4wWKkCURJYYWadAtcEjnXsQ ZCgI9IcqtCntKz8lcvSlVTtTRikw2cRxyKs3v6SBQpDYBNM+kCas8jJKyEbxblWbQEXw lu9A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690494814; x=1691099614; h=cc:to:from:subject:message-id:mime-version:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=+IrV9GMRSdZPt30c7KcILm3qep9zKx/623+6CGNnSZI=; b=cdMWgx+QPQpHizCjUdM/feE5LXH40ST0zJu0MmvzKMO67DvkYYQuDtWTTyW10K3h7Y VHj6IehJoD5m3GFUCIaOxP9JypzeksrSLU/L+cKM8S8ijvp7VJH7sUuPYlhS6RMPvYlY VEiEah0l9pQDNEXUyCnV0AWRCOxVLEG+Z8aO0H2OKKnp1BSzztvzx59FDxeLZiuU/6AV L/OLGsDmvVAk/iaRyCY2Gh5nD0H/GvlgHwxqWwGon2jEPtehpdjQGc/RLakghFvrUW2S LbwyN5j6rxKheMvrOxoDyK1X7y4ngVNQkUZUsmIiZQqpIIZHR7z6ymmZatmLwsKEtoTz ZAcg== X-Gm-Message-State: ABy/qLbyjaZqGvDCTEbdgp6NZzJRQAosd0Y9dEdkUCUWhkoLWFZE/wR+ ArYRdVKpSmbXZQwX7TF/Bgo2Hfn/cmvcFxZF/w== X-Received: from jstitt-linux1.c.googlers.com ([fda3:e722:ac3:cc00:2b:ff92:c0a8:23b5]) (user=justinstitt job=sendgmr) by 2002:a25:ab13:0:b0:cfe:74cf:e61a with SMTP id u19-20020a25ab13000000b00cfe74cfe61amr4397ybi.6.1690494813865; Thu, 27 Jul 2023 14:53:33 -0700 (PDT) Date: Thu, 27 Jul 2023 21:53:24 +0000 Mime-Version: 1.0 X-B4-Tracking: v=1; b=H4sIAFPnwmQC/6tWKk4tykwtVrJSqFYqSi3LLM7MzwNyDHUUlJIzE vPSU3UzU4B8JSMDI2MDcyNz3eL80rwU3YrUPF1jS4tkS0sj8yTjFAsloPqCotS0zAqwWdGxtbU AfC3LdVsAAAA= X-Developer-Key: i=justinstitt@google.com; a=ed25519; pk=tC3hNkJQTpNX/gLKxTNQKDmiQl6QjBNCGKJINqAdJsE= X-Developer-Signature: v=1; a=ed25519-sha256; t=1690494812; l=3106; i=justinstitt@google.com; s=20230717; h=from:subject:message-id; bh=2DV18A5LAIbHOz8+4mF5Dz/H6ijYF+f0DqJHiw+O6Z8=; b=vopO06Wm0zYSnDpTC9r5I+EsMsCZYRVtZWxLb5Dfivdpz4dFnQc4/MROOoOi/9uBOTysUrbZO B8mJVXUSQDcCy59jXa+eGsPlPBt2oonSrkqDfkKPfBHRnmLmfyW1zgm X-Mailer: b4 0.12.3 Message-ID: <20230727-sound-xen-v1-1-89dd161351f1@google.com> Subject: [PATCH] ALSA: xen-front: refactor deprecated strncpy From: Justin Stitt <justinstitt@google.com> To: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>, Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com> Cc: xen-devel@lists.xenproject.org, alsa-devel@alsa-project.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,T_SCC_BODY_TEXT_LINE, USER_IN_DEF_DKIM_WL autolearn=ham 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: 1772615960624842889 X-GMAIL-MSGID: 1772615960624842889 |
Series |
ALSA: xen-front: refactor deprecated strncpy
|
|
Commit Message
Justin Stitt
July 27, 2023, 9:53 p.m. UTC
`strncpy` is deprecated for use on NUL-terminated destination strings [1].
A suitable replacement is `strscpy` [2] due to the fact that it
guarantees NUL-termination on its destination buffer argument which is
_not_ always the case for `strncpy`!
It should be noted that, in this case, the destination buffer has a
length strictly greater than the source string. Moreover, the source
string is NUL-terminated (and so is the destination) which means there
was no real bug happening here. Nonetheless, this patch would get us one
step closer to eliminating the `strncpy` API in the kernel, as its use
is too ambiguous. We need to favor less ambiguous replacements such as:
strscpy, strscpy_pad, strtomem and strtomem_pad (amongst others).
Technically, my patch yields subtly different behavior. The original
implementation with `strncpy` would fill the entire destination buffer
with null bytes [3] while `strscpy` will leave the junk, uninitialized
bytes trailing after the _mandatory_ NUL-termination. So, if somehow
`pcm->name` or `card->driver/shortname/longname` require this
NUL-padding behavior then `strscpy_pad` should be used. My
interpretation, though, is that the aforementioned fields are just fine
as NUL-terminated strings. Please correct my assumptions if needed and
I'll send in a v2.
[1]: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
[2]: manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html
[3]: https://linux.die.net/man/3/strncpy
Link: https://github.com/KSPP/linux/issues/90
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
sound/xen/xen_snd_front_alsa.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
---
base-commit: 57012c57536f8814dec92e74197ee96c3498d24e
change-id: 20230727-sound-xen-398c9927b3d8
Best regards,
--
Justin Stitt <justinstitt@google.com>
Comments
On Thu, Jul 27, 2023 at 09:53:24PM +0000, Justin Stitt wrote: > Technically, my patch yields subtly different behavior. The original > implementation with `strncpy` would fill the entire destination buffer > with null bytes [3] while `strscpy` will leave the junk, uninitialized > bytes trailing after the _mandatory_ NUL-termination. So, if somehow > `pcm->name` or `card->driver/shortname/longname` require this > NUL-padding behavior then `strscpy_pad` should be used. My > interpretation, though, is that the aforementioned fields are just fine > as NUL-terminated strings. Please correct my assumptions if needed and > I'll send in a v2. "uninitialized bytes" => "leak of sensitive information" => "security hole" One hopes the unitialized bytes don't contain sensitive information, but that is the start of the chain. One can hope the VM on the other end is friendly, but that isn't something to rely on. I'm not in charge of any of the appropriate subsystems, I just happened to randomly look at this as message on a mailing list I'm on. Could be the maintainers will find this acceptable.
On Thu, Jul 27, 2023 at 09:46:36PM -0700, Elliott Mitchell wrote: > On Thu, Jul 27, 2023 at 09:53:24PM +0000, Justin Stitt wrote: > > Technically, my patch yields subtly different behavior. The original > > implementation with `strncpy` would fill the entire destination buffer > > with null bytes [3] while `strscpy` will leave the junk, uninitialized > > bytes trailing after the _mandatory_ NUL-termination. So, if somehow > > `pcm->name` or `card->driver/shortname/longname` require this > > NUL-padding behavior then `strscpy_pad` should be used. My > > interpretation, though, is that the aforementioned fields are just fine > > as NUL-terminated strings. Please correct my assumptions if needed and > > I'll send in a v2. > > "uninitialized bytes" => "leak of sensitive information" => "security hole" For xen_snd_front_alsa_init(), "card" is already zero-initialized in snd_card_new(). For new_pcm_instance(), "pcm" is already zero-initialized in _snd_pcm_new(). So things look good to me! Reviewed-by: Kees Cook <keescook@chromium.org>
On Thu, 27 Jul 2023 23:53:24 +0200, Justin Stitt wrote: > > `strncpy` is deprecated for use on NUL-terminated destination strings [1]. > > A suitable replacement is `strscpy` [2] due to the fact that it > guarantees NUL-termination on its destination buffer argument which is > _not_ always the case for `strncpy`! > > It should be noted that, in this case, the destination buffer has a > length strictly greater than the source string. Moreover, the source > string is NUL-terminated (and so is the destination) which means there > was no real bug happening here. Nonetheless, this patch would get us one > step closer to eliminating the `strncpy` API in the kernel, as its use > is too ambiguous. We need to favor less ambiguous replacements such as: > strscpy, strscpy_pad, strtomem and strtomem_pad (amongst others). > > Technically, my patch yields subtly different behavior. The original > implementation with `strncpy` would fill the entire destination buffer > with null bytes [3] while `strscpy` will leave the junk, uninitialized > bytes trailing after the _mandatory_ NUL-termination. So, if somehow > `pcm->name` or `card->driver/shortname/longname` require this > NUL-padding behavior then `strscpy_pad` should be used. My > interpretation, though, is that the aforementioned fields are just fine > as NUL-terminated strings. Please correct my assumptions if needed and > I'll send in a v2. > > [1]: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings > [2]: manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html > [3]: https://linux.die.net/man/3/strncpy > > Link: https://github.com/KSPP/linux/issues/90 > Signed-off-by: Justin Stitt <justinstitt@google.com> Applied now. Thanks. Takashi
diff --git a/sound/xen/xen_snd_front_alsa.c b/sound/xen/xen_snd_front_alsa.c index db917453a473..7a3dfce97c15 100644 --- a/sound/xen/xen_snd_front_alsa.c +++ b/sound/xen/xen_snd_front_alsa.c @@ -783,7 +783,7 @@ static int new_pcm_instance(struct xen_snd_front_card_info *card_info, pcm->info_flags = 0; /* we want to handle all PCM operations in non-atomic context */ pcm->nonatomic = true; - strncpy(pcm->name, "Virtual card PCM", sizeof(pcm->name)); + strscpy(pcm->name, "Virtual card PCM", sizeof(pcm->name)); if (instance_cfg->num_streams_pb) snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, @@ -835,9 +835,9 @@ int xen_snd_front_alsa_init(struct xen_snd_front_info *front_info) goto fail; } - strncpy(card->driver, XENSND_DRIVER_NAME, sizeof(card->driver)); - strncpy(card->shortname, cfg->name_short, sizeof(card->shortname)); - strncpy(card->longname, cfg->name_long, sizeof(card->longname)); + strscpy(card->driver, XENSND_DRIVER_NAME, sizeof(card->driver)); + strscpy(card->shortname, cfg->name_short, sizeof(card->shortname)); + strscpy(card->longname, cfg->name_long, sizeof(card->longname)); ret = snd_card_register(card); if (ret < 0)