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 lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id AB620C38142 for ; Fri, 20 Jan 2023 00:52:35 +0000 (UTC) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4NygvP5ryzz3fK7 for ; Fri, 20 Jan 2023 11:52:33 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=russell.cc header.i=@russell.cc header.a=rsa-sha256 header.s=fm1 header.b=mHrv+P2K; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=messagingengine.com header.i=@messagingengine.com header.a=rsa-sha256 header.s=fm3 header.b=mpy5pc4Y; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=russell.cc (client-ip=66.111.4.26; helo=out2-smtp.messagingengine.com; envelope-from=ruscur@russell.cc; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=russell.cc header.i=@russell.cc header.a=rsa-sha256 header.s=fm1 header.b=mHrv+P2K; dkim=pass (2048-bit key; unprotected) header.d=messagingengine.com header.i=@messagingengine.com header.a=rsa-sha256 header.s=fm3 header.b=mpy5pc4Y; dkim-atps=neutral Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4NygtJ6Yk4z3cCk for ; Fri, 20 Jan 2023 11:51:36 +1100 (AEDT) Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id 663865C0041; Thu, 19 Jan 2023 19:51:31 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Thu, 19 Jan 2023 19:51:31 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=russell.cc; h=cc :cc:content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to; s=fm1; t=1674175891; x= 1674262291; bh=oVodv+f7tO66O7huZdWKERwv7Beigm5tN48FcTUIthc=; b=m Hrv+P2KS6He5GJADam0ymxMgPpSjBSs0fPLtwhPwL6V7rjBYswNdNvugTQOIJ+Dy 5awXIu77v5YBmJIw9qsFOv01IVRtF1y6nKHlTTMuzFRhu/fCRl3GK3sCOerjph2w SkTaYIz9o5C+aTeTY3Yzajdq4g7OJN+iFydgUJiDMNbFdqWypg+VJpCPeHtBhR5B oygQTVHXy7Xmn4p6YaFvp6EqsW1mX/GPxCgkYVI5KQ6XV/oN6c6SfRITRvAZZEd/ D39pqLOdc+wSWHjkD0Xk8wee3AiSNjNyh/nWQU/Aoqtw2qvkPHTbgg2GNUwl/YEt KSLR68cKOcRBU0le6/SsA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t=1674175891; x= 1674262291; bh=oVodv+f7tO66O7huZdWKERwv7Beigm5tN48FcTUIthc=; b=m py5pc4Y2rCa/bfxvG8c/0IDTR8Idu8DxfDeWdixFkb1e7DMaIzfvvcTaeFf4t0RH Qi4vF/dBWRuw2wlH3CYlIph5krdY2Ygbi647jn6xavgeKyGgITh0nkqnEyd/p/SZ JzrNaoX3tIZ+XoqFU+EFIwkQBEdl32eCKMeo+AOUU9fJbl5dcSU532dUsxPFVESB UhZESekMjsQ970nLj2UL/JVPEsO5pXRy9/cM/157F8rbERSqB6qEDCD9tJVKgTMX qzmxcLKo/L+Iar8RAhyBSrU8lXU3mY9bAu+4Ng/Jzz529g+usEcBejj3hpGyc8KE goHUqmvzHbuotZT9/sYeg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrudduuddgvdekucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne gfrhhlucfvnfffucdludehmdenucfjughrpefkuffhvfevffgjfhgtgfgfggesthhqredt tderjeenucfhrhhomheptfhushhsvghllhcuvehurhhrvgihuceorhhushgtuhhrsehruh hsshgvlhhlrdgttgeqnecuggftrfgrthhtvghrnheptefgieelhfeufeevvdekheeifeej gfefgeehtedukeeigfduuddtueekteevleelnecuvehluhhsthgvrhfuihiivgeptdenuc frrghrrghmpehmrghilhhfrhhomheprhhushgtuhhrsehruhhsshgvlhhlrdgttg X-ME-Proxy: Feedback-ID: i4421424f:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 19 Jan 2023 19:51:27 -0500 (EST) Message-ID: <26945a8372ddde0c2bea9f2382ea9f1e74b9fe63.camel@russell.cc> Subject: Re: [PATCH v3 04/24] powerpc/secvar: Handle format string in the consumer From: Russell Currey To: Nicholas Piggin , Andrew Donnellan , linuxppc-dev@lists.ozlabs.org, linux-integrity@vger.kernel.org Date: Fri, 20 Jan 2023 11:51:24 +1100 In-Reply-To: References: <20230118061049.1006141-1-ajd@linux.ibm.com> <20230118061049.1006141-5-ajd@linux.ibm.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.46.3 (3.46.3-1.fc37) MIME-Version: 1.0 X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: sudhakar@linux.ibm.com, erichte@linux.ibm.com, gregkh@linuxfoundation.org, nayna@linux.ibm.com, linux-kernel@vger.kernel.org, zohar@linux.ibm.com, gjoyce@linux.ibm.com, bgray@linux.ibm.com, gcwilson@linux.ibm.com Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Thu, 2023-01-19 at 11:17 +1000, Nicholas Piggin wrote: > On Wed Jan 18, 2023 at 4:10 PM AEST, Andrew Donnellan wrote: > > From: Russell Currey > >=20 > > The code that handles the format string in secvar-sysfs.c is > > entirely > > OPAL specific, so create a new "format" op in secvar_operations to > > make > > the secvar code more generic.=C2=A0 No functional change. > >=20 > > Signed-off-by: Russell Currey > > Signed-off-by: Andrew Donnellan > >=20 > > --- > >=20 > > v2: Use sysfs_emit() instead of sprintf() (gregkh) > >=20 > > v3: Enforce format string size limit (ruscur) > > --- > > =C2=A0arch/powerpc/include/asm/secvar.h=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0 3 +++ > > =C2=A0arch/powerpc/kernel/secvar-sysfs.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 | 23 ++++------------ > > -- > > =C2=A0arch/powerpc/platforms/powernv/opal-secvar.c | 25 > > ++++++++++++++++++++ > > =C2=A03 files changed, 33 insertions(+), 18 deletions(-) > >=20 > > diff --git a/arch/powerpc/include/asm/secvar.h > > b/arch/powerpc/include/asm/secvar.h > > index 07ba36f868a7..8b6475589120 100644 > > --- a/arch/powerpc/include/asm/secvar.h > > +++ b/arch/powerpc/include/asm/secvar.h > > @@ -11,12 +11,15 @@ > > =C2=A0#include > > =C2=A0#include > > =C2=A0 > > +#define SECVAR_MAX_FORMAT_LEN=C2=A0=C2=A030 // max length of string re= turned > > by ->format() > > + > > =C2=A0extern const struct secvar_operations *secvar_ops; > > =C2=A0 > > =C2=A0struct secvar_operations { > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0int (*get)(const char *= key, u64 key_len, u8 *data, u64 > > *data_size); > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0int (*get_next)(const c= har *key, u64 *key_len, u64 > > keybufsize); > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0int (*set)(const char *= key, u64 key_len, u8 *data, u64 > > data_size); > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0ssize_t (*format)(char *buf)= ; > > =C2=A0}; > > =C2=A0 > > =C2=A0#ifdef CONFIG_PPC_SECURE_BOOT > > diff --git a/arch/powerpc/kernel/secvar-sysfs.c > > b/arch/powerpc/kernel/secvar-sysfs.c > > index 462cacc0ca60..d3858eedd72c 100644 > > --- a/arch/powerpc/kernel/secvar-sysfs.c > > +++ b/arch/powerpc/kernel/secvar-sysfs.c > > @@ -21,26 +21,13 @@ static struct kset *secvar_kset; > > =C2=A0static ssize_t format_show(struct kobject *kobj, struct > > kobj_attribute *attr, > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 char *buf) > > =C2=A0{ > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0ssize_t rc =3D 0; > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0struct device_node *node; > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0const char *format; > > - > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0node =3D of_find_compatible_= node(NULL, NULL, "ibm,secvar- > > backend"); > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (!of_device_is_available(= node)) { > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0rc =3D -ENODEV; > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0goto out; > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0} > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0char tmp[SECVAR_MAX_FORMAT_L= EN]; > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0ssize_t len =3D secvar_ops->= format(tmp); > > =C2=A0 > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rc =3D of_property_read_stri= ng(node, "format", &format); > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (rc) > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0goto out; > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (len <=3D 0) > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0return -EIO; >=20 > AFAIKS this does have a functional change, it loses the return value. > Why not return len if it is < 0, and -EIO if len =3D=3D 0? In v2 mpe suggested the following: I'm not sure you should pass that raw error back to sysfs. Some of the values could be confusing, eg. if you return -EINVAL it looks like a parameter to the read() syscall was invalid. Might be better to just return -EIO. =20 Following that advice, I don't think we should return something other than -EIO, but we should at least pr_err() to document the error - this isn't something that should ever fail. >=20 > Thanks, > Nick