All of lore.kernel.org
 help / color / mirror / Atom feed
From: ebiggers3@gmail.com (Eric Biggers)
To: linux-security-module@vger.kernel.org
Subject: [PATCH] key: Convert big_key payload.data to struct
Date: Mon, 8 May 2017 15:19:43 -0700	[thread overview]
Message-ID: <20170508221943.GA46762@gmail.com> (raw)
In-Reply-To: <10235.1494280856@warthog.procyon.org.uk>

On Mon, May 08, 2017 at 11:00:56PM +0100, David Howells wrote:
> Kees Cook <keescook@chromium.org> wrote:
> 
> > There is a lot of needless casting happening in the big_key data payload.
> > This is harder to trivially verify by static analysis and specifically
> > the randstruct GCC plugin (which was unhappy about casting a struct
> > path across two entries of a void * array). This converts the payload to
> > the actually used structures (one pointer, one embedded struct, and one
> > size_t).
> 
> I'd really rather not do this as this moves the definition of an individual
> key type into the general structure (I know I've done this for the keyring
> type, but that's a special part of the keyring code).  That's the start of the
> slippery slope into moving all of them in there.
> 
> I'd rather you defined, say:
> 
> 	struct big_key_payload {
> 		u8		*key_data;
> 		struct path	key_path;
> 		size_t		key_len;
> 	};
> 
> in big_key.c and cast &key->payload to it.
> 

That still seems like a hack.  It probably would be easier to kmalloc() this
struct and store a pointer to it in key->payload.data[0], like some of the other
key types do, e.g. "encrypted" and "trusted".  The key data could even be inline
at the end, for non-file backed big_keys.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Eric Biggers <ebiggers3@gmail.com>
To: David Howells <dhowells@redhat.com>
Cc: Kees Cook <keescook@chromium.org>,
	James Morris <james.l.morris@oracle.com>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	keyrings@vger.kernel.org, linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] key: Convert big_key payload.data to struct
Date: Mon, 8 May 2017 15:19:43 -0700	[thread overview]
Message-ID: <20170508221943.GA46762@gmail.com> (raw)
In-Reply-To: <10235.1494280856@warthog.procyon.org.uk>

On Mon, May 08, 2017 at 11:00:56PM +0100, David Howells wrote:
> Kees Cook <keescook@chromium.org> wrote:
> 
> > There is a lot of needless casting happening in the big_key data payload.
> > This is harder to trivially verify by static analysis and specifically
> > the randstruct GCC plugin (which was unhappy about casting a struct
> > path across two entries of a void * array). This converts the payload to
> > the actually used structures (one pointer, one embedded struct, and one
> > size_t).
> 
> I'd really rather not do this as this moves the definition of an individual
> key type into the general structure (I know I've done this for the keyring
> type, but that's a special part of the keyring code).  That's the start of the
> slippery slope into moving all of them in there.
> 
> I'd rather you defined, say:
> 
> 	struct big_key_payload {
> 		u8		*key_data;
> 		struct path	key_path;
> 		size_t		key_len;
> 	};
> 
> in big_key.c and cast &key->payload to it.
> 

That still seems like a hack.  It probably would be easier to kmalloc() this
struct and store a pointer to it in key->payload.data[0], like some of the other
key types do, e.g. "encrypted" and "trusted".  The key data could even be inline
at the end, for non-file backed big_keys.

Eric

  reply	other threads:[~2017-05-08 22:19 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-08 21:43 [PATCH] key: Convert big_key payload.data to struct Kees Cook
2017-05-08 21:43 ` Kees Cook
2017-05-08 22:00 ` David Howells
2017-05-08 22:00   ` David Howells
2017-05-08 22:19   ` Eric Biggers [this message]
2017-05-08 22:19     ` Eric Biggers
2017-05-09  7:24     ` David Howells
2017-05-09  7:24       ` David Howells
2017-05-09 21:45       ` Eric Biggers
2017-05-09 21:45         ` Eric Biggers
2017-05-08 22:26   ` Kees Cook
2017-05-08 22:26     ` Kees Cook
2017-05-09  8:11     ` David Howells
2017-05-09  8:11       ` David Howells
2017-05-09 16:12       ` Kees Cook
2017-05-09 16:12         ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170508221943.GA46762@gmail.com \
    --to=ebiggers3@gmail.com \
    --cc=linux-security-module@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.