From patchwork Mon May 8 21:43:24 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 9716743 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id CCAAA602A0 for ; Mon, 8 May 2017 21:43:30 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C7C96205D1 for ; Mon, 8 May 2017 21:43:30 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id BBCDE265B9; Mon, 8 May 2017 21:43:30 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0ABAA205D1 for ; Mon, 8 May 2017 21:43:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754100AbdEHVn2 (ORCPT ); Mon, 8 May 2017 17:43:28 -0400 Received: from mail-pg0-f54.google.com ([74.125.83.54]:36241 "EHLO mail-pg0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751243AbdEHVn0 (ORCPT ); Mon, 8 May 2017 17:43:26 -0400 Received: by mail-pg0-f54.google.com with SMTP id 64so17508894pgb.3 for ; Mon, 08 May 2017 14:43:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:mime-version:content-disposition; bh=A1RiMI/wCFUnKk2u6jeeDnhjK/+4MFQI/oeVSh0GWrc=; b=NCp7zSrUUnPTOQZmm/3RJip9cGTCLvGbmmm7zeKRM8uhGeqzeeAiG4m1Ax6N6Nrul9 3fSKEtIsrhbc7uCEh8nnRGvQWJ7mLit+uIOOKOmWp5bcTJ6WcECcaTQmQNxJZIG8SDYG KPhpFlH7eO0nSsoHkN+2l+m+OiZ3MvRMiwYmE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:mime-version :content-disposition; bh=A1RiMI/wCFUnKk2u6jeeDnhjK/+4MFQI/oeVSh0GWrc=; b=eoK0028MUxNrPYmu3YZXH0jqyC7H9wVqz1v8dALjw/gwZCXgGSkwiMjpxvAQqJkGxO qLBaxb87YCIc5r1Hc1kxAnEHpF20hgKAJPyO0lbfy34wXDMRLF3fslRCm6hdWWXob+Tv sq8dYTQxbaMyA54/JGNUXWnAKY3ZLJJssMfIjs87YIzXvCQ3nLqJlKe1xnavtl307xV7 QVMCn80YbYjkolozmGoIiGd0ZXZZ5WBx/xkWfIyDL0lVlQ+yHveUBfvQJRBJrHQ9VabN KMWhlNo5Ekk3Tli6AhKfqB2sFEvjaM3IkYMkSCE6A64SOht54IV4vhyKs0ZXeJk9HH6W Pk3Q== X-Gm-Message-State: AODbwcASK0g5KqnQcAmUWlsUv2tXSM/F9Wv75KGo6dAZ6SxzGAglHWat F4giny+9TMUQ2Ece X-Received: by 10.98.12.142 with SMTP id 14mr7595420pfm.66.1494279805413; Mon, 08 May 2017 14:43:25 -0700 (PDT) Received: from www.outflux.net (173-164-112-133-Oregon.hfc.comcastbusiness.net. [173.164.112.133]) by smtp.gmail.com with ESMTPSA id h21sm17157873pgn.18.2017.05.08.14.43.24 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 08 May 2017 14:43:24 -0700 (PDT) Date: Mon, 8 May 2017 14:43:24 -0700 From: Kees Cook To: David Howells Cc: James Morris , "Serge E. Hallyn" , keyrings@vger.kernel.org, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] key: Convert big_key payload.data to struct Message-ID: <20170508214324.GA124468@beast> MIME-Version: 1.0 Content-Disposition: inline Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: X-Virus-Scanned: ClamAV using ClamSMTP 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). Signed-off-by: Kees Cook --- include/linux/key.h | 11 ++++++++++- security/keys/big_key.c | 45 +++++++++++++++++---------------------------- 2 files changed, 27 insertions(+), 29 deletions(-) diff --git a/include/linux/key.h b/include/linux/key.h index 78e25aabedaf..2390830e3b1a 100644 --- a/include/linux/key.h +++ b/include/linux/key.h @@ -24,6 +24,7 @@ #include #include #include +#include #ifdef __KERNEL__ #include @@ -92,7 +93,15 @@ struct keyring_index_key { union key_payload { void __rcu *rcu_data0; - void *data[4]; + union { + void *data[4]; + /* Layout of big_key payload words. */ + struct { + u8 *key_data; + struct path key_path; + size_t key_len; + }; + }; }; /*****************************************************************************/ diff --git a/security/keys/big_key.c b/security/keys/big_key.c index 835c1ab30d01..6a0feb964e4a 100644 --- a/security/keys/big_key.c +++ b/security/keys/big_key.c @@ -22,16 +22,6 @@ #include /* - * Layout of key payload words. - */ -enum { - big_key_data, - big_key_path, - big_key_path_2nd_part, - big_key_len, -}; - -/* * Crypto operation with big_key data */ enum big_key_op { @@ -123,7 +113,7 @@ static int big_key_crypt(enum big_key_op op, u8 *data, size_t datalen, u8 *key) */ int big_key_preparse(struct key_preparsed_payload *prep) { - struct path *path = (struct path *)&prep->payload.data[big_key_path]; + struct path *path = &prep->payload.key_path; struct file *file; u8 *enckey; u8 *data = NULL; @@ -138,7 +128,7 @@ int big_key_preparse(struct key_preparsed_payload *prep) /* Set an arbitrary quota */ prep->quotalen = 16; - prep->payload.data[big_key_len] = (void *)(unsigned long)datalen; + prep->payload.key_len = datalen; if (datalen > BIG_KEY_FILE_THRESHOLD) { /* Create a shmem file to store the data in. This will permit the data @@ -190,7 +180,7 @@ int big_key_preparse(struct key_preparsed_payload *prep) /* Pin the mount and dentry to the key so that we can open it again * later */ - prep->payload.data[big_key_data] = enckey; + prep->payload.key_data = enckey; *path = file->f_path; path_get(path); fput(file); @@ -202,7 +192,7 @@ int big_key_preparse(struct key_preparsed_payload *prep) if (!data) return -ENOMEM; - prep->payload.data[big_key_data] = data; + prep->payload.key_data = data; memcpy(data, prep->data, prep->datalen); } return 0; @@ -222,11 +212,11 @@ int big_key_preparse(struct key_preparsed_payload *prep) void big_key_free_preparse(struct key_preparsed_payload *prep) { if (prep->datalen > BIG_KEY_FILE_THRESHOLD) { - struct path *path = (struct path *)&prep->payload.data[big_key_path]; + struct path *path = &prep->payload.key_path; path_put(path); } - kfree(prep->payload.data[big_key_data]); + kfree(prep->payload.key_data); } /* @@ -235,12 +225,12 @@ void big_key_free_preparse(struct key_preparsed_payload *prep) */ void big_key_revoke(struct key *key) { - struct path *path = (struct path *)&key->payload.data[big_key_path]; + struct path *path = &key->payload.key_path; /* clear the quota */ key_payload_reserve(key, 0); if (key_is_instantiated(key) && - (size_t)key->payload.data[big_key_len] > BIG_KEY_FILE_THRESHOLD) + key->payload.key_len > BIG_KEY_FILE_THRESHOLD) vfs_truncate(path, 0); } @@ -249,17 +239,17 @@ void big_key_revoke(struct key *key) */ void big_key_destroy(struct key *key) { - size_t datalen = (size_t)key->payload.data[big_key_len]; + size_t datalen = key->payload.key_len; if (datalen > BIG_KEY_FILE_THRESHOLD) { - struct path *path = (struct path *)&key->payload.data[big_key_path]; + struct path *path = &key->payload.key_path; path_put(path); path->mnt = NULL; path->dentry = NULL; } - kfree(key->payload.data[big_key_data]); - key->payload.data[big_key_data] = NULL; + kfree(key->payload.key_data); + key->payload.key_data = NULL; } /* @@ -267,7 +257,7 @@ void big_key_destroy(struct key *key) */ void big_key_describe(const struct key *key, struct seq_file *m) { - size_t datalen = (size_t)key->payload.data[big_key_len]; + size_t datalen = key->payload.key_len; seq_puts(m, key->description); @@ -283,17 +273,17 @@ void big_key_describe(const struct key *key, struct seq_file *m) */ long big_key_read(const struct key *key, char __user *buffer, size_t buflen) { - size_t datalen = (size_t)key->payload.data[big_key_len]; + size_t datalen = key->payload.key_len; long ret; if (!buffer || buflen < datalen) return datalen; if (datalen > BIG_KEY_FILE_THRESHOLD) { - struct path *path = (struct path *)&key->payload.data[big_key_path]; + struct path *path = &key->payload.key_path; struct file *file; u8 *data; - u8 *enckey = (u8 *)key->payload.data[big_key_data]; + u8 *enckey = key->payload.key_data; size_t enclen = ALIGN(datalen, crypto_skcipher_blocksize(big_key_skcipher)); data = kmalloc(enclen, GFP_KERNEL); @@ -329,8 +319,7 @@ long big_key_read(const struct key *key, char __user *buffer, size_t buflen) kfree(data); } else { ret = datalen; - if (copy_to_user(buffer, key->payload.data[big_key_data], - datalen) != 0) + if (copy_to_user(buffer, key->payload.key_data, datalen) != 0) ret = -EFAULT; }