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 9A88DECAAA3 for ; Fri, 26 Aug 2022 05:42:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244767AbiHZFmc (ORCPT ); Fri, 26 Aug 2022 01:42:32 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43564 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230517AbiHZFm3 (ORCPT ); Fri, 26 Aug 2022 01:42:29 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DF04FD0201; Thu, 25 Aug 2022 22:42:28 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 2FEB2619FD; Fri, 26 Aug 2022 05:42:28 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 046B6C433C1; Fri, 26 Aug 2022 05:42:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1661492547; bh=+/7bgcM7XCyWZRFiTszQQDAjii7dFnz03LdfZR+j2a8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=kEDrPfXuEWYVmP7eEh+msDb+GYa8Idz9/kaI3MfF1Q3RfI1sAXJhMvI85+nzf4r5C Uge3vmZWoOOw3xTMv9RuRaIejaUZuTF8FEx3AFnAU6PLlKdpF/9qZu2LPc94EBHaea cnk43VRd1w0vkqykz4L413jU4pYS28Z16HyKXmoGBtiJvBNkX4L5BwnHUOscmXl3mK ZnsQ9JRgyKfE4HHpHjMevW/IkfLBi77MgRPLKy+2NSs1U7hK9cfXwy4TuDmm5EVR9f dAZHGqx14AymofhqDu29EHJ2yia6JgZ3rJjowwMwySNWrEMoRbmp3pGbOLjpJCXyfk J+DvtrRVaU/EQ== Date: Fri, 26 Aug 2022 08:42:20 +0300 From: Jarkko Sakkinen To: roberto.sassu@huaweicloud.com Cc: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org, martin.lau@linux.dev, song@kernel.org, yhs@fb.com, john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com, haoluo@google.com, jolsa@kernel.org, mykolal@fb.com, corbet@lwn.net, dhowells@redhat.com, rostedt@goodmis.org, mingo@redhat.com, paul@paul-moore.com, jmorris@namei.org, serge@hallyn.com, shuah@kernel.org, bpf@vger.kernel.org, linux-doc@vger.kernel.org, keyrings@vger.kernel.org, linux-security-module@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, deso@posteo.net, Roberto Sassu Subject: Re: [PATCH v12 04/10] KEYS: Move KEY_LOOKUP_ to include/linux/key.h Message-ID: References: <20220818152929.402605-1-roberto.sassu@huaweicloud.com> <20220818152929.402605-5-roberto.sassu@huaweicloud.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220818152929.402605-5-roberto.sassu@huaweicloud.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Aug 18, 2022 at 05:29:23PM +0200, roberto.sassu@huaweicloud.com wrote: > From: Roberto Sassu > > In preparation for the patch that introduces the bpf_lookup_user_key() eBPF > kfunc, move KEY_LOOKUP_ definitions to include/linux/key.h, to be able to > validate the kfunc parameters. > > Also, introduce key_lookup_flags_check() directly in include/linux/key.h, > to reduce the risk that the check is not in sync with currently defined > flags. Missing the description what the heck this function even is. Please, explain that. Also, the short subject is misleading because this *just* does not move flags. > > Signed-off-by: Roberto Sassu > Reviewed-by: KP Singh > --- > include/linux/key.h | 11 +++++++++++ > security/keys/internal.h | 2 -- > 2 files changed, 11 insertions(+), 2 deletions(-) > > diff --git a/include/linux/key.h b/include/linux/key.h > index 7febc4881363..b5bbae77a9e7 100644 > --- a/include/linux/key.h > +++ b/include/linux/key.h > @@ -88,6 +88,17 @@ enum key_need_perm { > KEY_DEFER_PERM_CHECK, /* Special: permission check is deferred */ > }; > > +#define KEY_LOOKUP_CREATE 0x01 > +#define KEY_LOOKUP_PARTIAL 0x02 > + /* * Explain what the heck this function is. */ > +static inline int key_lookup_flags_check(u64 flags) > +{ > + if (flags & ~(KEY_LOOKUP_CREATE | KEY_LOOKUP_PARTIAL)) > + return -EINVAL; > + > + return 0; > +} This is essentially a boolean function, right? I.e. the implementation can be just: !!(flags & ~(KEY_LOOKUP_CREATE | KEY_LOOKUP_PARTIAL)) Not even sure if this is needed in the first place, or would it be better just to open code it. How many call sites does it have anyway? BR, Jarkko