Re: [PATCH] Keys: Add possessor permissions to keys

Andrew Morton
2005-09-21T17:16:52Z

David Howells <dhowells@redhat.com> wrote:
>
>
> The attached patch adds extra permission grants to keys for the possessor of a
> key in addition to the owner, group and other permissions bits. This makes
> SUID binaries easier to support without going as far as labelling keys and key
> targets using the LSM facilities.
>
> This patch adds a second "pointer type" to key structures (struct key_ref *)
> that can have the bottom bit of the address set to indicate the possession of
> a key. This is propagated through searches from the keyring to the discovered
> key. It has been made a separate type so that the compiler can spot attempts
> to dereference a potentially incorrect pointer.

The above bit needs to be captured in a code comment. Because:

> ...
> /*
> + * key reference with possession flag handling
> + */
> +static inline struct key_ref *key_mkref(const struct key *key, unsigned long possession)
> +{
> + return (struct key_ref *) ((unsigned long) key | possession);
> +}

Is hair-raising and makes people want to come after you with a stick ;)

(And an 80-col xterm)

ugh, I see. `struct key_ref' doesn't actually exist anywhere. The code
only ever deals with pointers to this non-existent structure and they are
munged `struct key *'s.

Did this _have_ to happen?

> + }
> + else if (key->uid == context->fsuid) {

Documentation/CodingStyle?

> + "%s;%d;%d;%08x;%s",
> + key_deref(key)->type->name,
> + key_deref(key)->uid,
> + key_deref(key)->gid,
> + key_deref(key)->perm,
> + key_deref(key)->description ? key_deref(key)->description : ""
> );

This doesn't actually make things clear.

> + if (PTR_ERR(key_ref) != -EAGAIN) {
> + if (IS_ERR(key_ref))
> + key = key_deref(key_ref);
> + else
> + key = ERR_PTR(PTR_ERR(key_ref));
> + break;
> + }
> + }

That's getting a bit intimate with how IS_ERR and PTR_ERR are implemented
but I guess we're unlikely to change that.

This all seems quite inappropriate to -rc2?