On Wed, 2021-12-01 at 12:27 +0200, Jarkko Sakkinen wrote:

On Tue, Nov 30, 2021 at 05:21:45PM +0000, Eric Snowberg wrote:

quoted

quoted

On Nov 26, 2021, at 5:49 PM, Jarkko Sakkinen [off-list ref] wrote:

On Tue, 2021-11-23 at 23:41 -0500, Eric Snowberg wrote:

quoted

In preparation for returning either the existing
restrict_link_by_builtin_and_secondary_trusted or the upcoming
restriction that includes the trusted builtin, secondary and
machine keys, to improve clarity, rename
get_builtin_and_secondary_restriction to get_secondary_restriction.

Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
v6: Initial version
v7: Unmodified from v7
v8: Code unmodified from v7, added Mimi's Reviewed-by
---
 certs/system_keyring.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 692365dee2bd..8f1f87579819 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c

@@ -77,7 +77,7 @@ int restrict_link_by_builtin_and_secondary_trusted(
  * Allocate a struct key_restriction for the "builtin and secondary trust"
  * keyring. Only for use in system_trusted_keyring_init().
  */
-static __init struct key_restriction *get_builtin_and_secondary_restriction(void)
+static __init struct key_restriction *get_secondary_restriction(void)
 {
        struct key_restriction *restriction;
 

@@ -117,7 +117,7 @@ static __init int system_trusted_keyring_init(void)
                               KEY_USR_VIEW | KEY_USR_READ | KEY_USR_SEARCH |
                               KEY_USR_WRITE),
                              KEY_ALLOC_NOT_IN_QUOTA,
-                             get_builtin_and_secondary_restriction(),
+                             get_secondary_restriction(),
                              NULL);
        if (IS_ERR(secondary_trusted_keys))
                panic("Can't allocate secondary trusted keyring\n");

This is wrong order.

You should first do the changes that make the old name
obsolete and only after that have a patch that does the
rename. Unfortunately, this patch cannot possibly acked
with the current order.

I can change the order, but I'm confused how this would work for a git bisect. 
If the rename happens afterwards, now two patches will always need to be 
reverted instead of the possibility of one.  Is this your expectation?

If the keyring name change is independent of any other changes, as
Jarkko suggested, nothing would break.

I'd drop this patch altogether. Old name is a bit ugly but does it matter
all that much?

The name "get_builtin_and_secondary_restriction" implies trust based on
keys in the ".builtin_trusted_keys" and ".secondary_trusted_keys"
keyrings.  This patch set is extending that to include keys on the new
".machine" keyring, by linking it to the secondary keyring.  Is leaving
the name unchanged really an option?

You already 16 patches without this.

Agreed, it's a lot.  In the past, I've asked Eric to see if some of
them could be squashed.

Mimi