From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751528AbdEHWTt (ORCPT ); Mon, 8 May 2017 18:19:49 -0400 Received: from mail-pg0-f67.google.com ([74.125.83.67]:34954 "EHLO mail-pg0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750839AbdEHWTr (ORCPT ); Mon, 8 May 2017 18:19:47 -0400 Date: Mon, 8 May 2017 15:19:43 -0700 From: Eric Biggers To: David Howells Cc: Kees Cook , James Morris , "Serge E. Hallyn" , 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 Message-ID: <20170508221943.GA46762@gmail.com> References: <20170508214324.GA124468@beast> <10235.1494280856@warthog.procyon.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <10235.1494280856@warthog.procyon.org.uk> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, May 08, 2017 at 11:00:56PM +0100, David Howells wrote: > Kees Cook 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