[{"id":1760071,"web_url":"http://patchwork.ozlabs.org/comment/1760071/","msgid":"<20170830112247.GA30640@quack2.suse.cz>","list_archive_url":null,"date":"2017-08-30T11:22:47","subject":"Re: [PATCH v2 08/30] ext2: Define usercopy region in\n\text2_inode_cache slab cache","submitter":{"id":363,"url":"http://patchwork.ozlabs.org/api/people/363/","name":"Jan Kara","email":"jack@suse.cz"},"content":"On Mon 28-08-17 14:34:49, Kees Cook wrote:\n> From: David Windsor <dave@nullcore.net>\n> \n> The ext2 symlink pathnames, stored in struct ext2_inode_info.i_data and\n> therefore contained in the ext2_inode_cache slab cache, need to be copied\n> to/from userspace.\n> \n> cache object allocation:\n>     fs/ext2/super.c:\n>         ext2_alloc_inode(...):\n>             struct ext2_inode_info *ei;\n>             ...\n>             ei = kmem_cache_alloc(ext2_inode_cachep, GFP_NOFS);\n>             ...\n>             return &ei->vfs_inode;\n> \n>     fs/ext2/ext2.h:\n>         EXT2_I(struct inode *inode):\n>             return container_of(inode, struct ext2_inode_info, vfs_inode);\n> \n>     fs/ext2/namei.c:\n>         ext2_symlink(...):\n>             ...\n>             inode->i_link = (char *)&EXT2_I(inode)->i_data;\n> \n> example usage trace:\n>     readlink_copy+0x43/0x70\n>     vfs_readlink+0x62/0x110\n>     SyS_readlinkat+0x100/0x130\n> \n>     fs/namei.c:\n>         readlink_copy(..., link):\n>             ...\n>             copy_to_user(..., link, len);\n> \n>         (inlined into vfs_readlink)\n>         generic_readlink(dentry, ...):\n>             struct inode *inode = d_inode(dentry);\n>             const char *link = inode->i_link;\n>             ...\n>             readlink_copy(..., link);\n> \n> In support of usercopy hardening, this patch defines a region in the\n> ext2_inode_cache slab cache in which userspace copy operations are\n> allowed.\n> \n> This region is known as the slab cache's usercopy region. Slab caches can\n> now check that each copy operation involving cache-managed memory falls\n> entirely within the slab's usercopy region.\n> \n> This patch is modified from Brad Spengler/PaX Team's PAX_USERCOPY\n> whitelisting code in the last public patch of grsecurity/PaX based on my\n> understanding of the code. Changes or omissions from the original code are\n> mine and don't reflect the original grsecurity/PaX code.\n> \n> Signed-off-by: David Windsor <dave@nullcore.net>\n> [kees: adjust commit log, provide usage trace]\n> Cc: Jan Kara <jack@suse.com>\n> Cc: linux-ext4@vger.kernel.org\n> Signed-off-by: Kees Cook <keescook@chromium.org>\n\nLooks good. You can add:\n\nAcked-by: Jan Kara <jack@suse.cz>\n\n\t\t\t\t\t\t\t\tHonza\n\n> ---\n>  fs/ext2/super.c | 12 +++++++-----\n>  1 file changed, 7 insertions(+), 5 deletions(-)\n> \n> diff --git a/fs/ext2/super.c b/fs/ext2/super.c\n> index 7b1bc9059863..670142cde59d 100644\n> --- a/fs/ext2/super.c\n> +++ b/fs/ext2/super.c\n> @@ -219,11 +219,13 @@ static void init_once(void *foo)\n>  \n>  static int __init init_inodecache(void)\n>  {\n> -\text2_inode_cachep = kmem_cache_create(\"ext2_inode_cache\",\n> -\t\t\t\t\t     sizeof(struct ext2_inode_info),\n> -\t\t\t\t\t     0, (SLAB_RECLAIM_ACCOUNT|\n> -\t\t\t\t\t\tSLAB_MEM_SPREAD|SLAB_ACCOUNT),\n> -\t\t\t\t\t     init_once);\n> +\text2_inode_cachep = kmem_cache_create_usercopy(\"ext2_inode_cache\",\n> +\t\t\t\tsizeof(struct ext2_inode_info), 0,\n> +\t\t\t\t(SLAB_RECLAIM_ACCOUNT|SLAB_MEM_SPREAD|\n> +\t\t\t\t\tSLAB_ACCOUNT),\n> +\t\t\t\toffsetof(struct ext2_inode_info, i_data),\n> +\t\t\t\tsizeof_field(struct ext2_inode_info, i_data),\n> +\t\t\t\tinit_once);\n>  \tif (ext2_inode_cachep == NULL)\n>  \t\treturn -ENOMEM;\n>  \treturn 0;\n> -- \n> 2.7.4\n> \n>","headers":{"Return-Path":"<linux-ext4-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-ext4-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xj35j2KQVz9sP5\n\tfor <patchwork-incoming@ozlabs.org>;\n\tWed, 30 Aug 2017 21:23:05 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751317AbdH3LWv (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tWed, 30 Aug 2017 07:22:51 -0400","from mx2.suse.de ([195.135.220.15]:42808 \"EHLO mx1.suse.de\"\n\trhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP\n\tid S1751282AbdH3LWu (ORCPT <rfc822;linux-ext4@vger.kernel.org>);\n\tWed, 30 Aug 2017 07:22:50 -0400","from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254])\n\tby mx1.suse.de (Postfix) with ESMTP id CCF66ABB2;\n\tWed, 30 Aug 2017 11:22:48 +0000 (UTC)","by quack2.suse.cz (Postfix, from userid 1000)\n\tid 3E0DD1E3414; Wed, 30 Aug 2017 13:22:47 +0200 (CEST)"],"X-Virus-Scanned":"by amavisd-new at test-mx.suse.de","Date":"Wed, 30 Aug 2017 13:22:47 +0200","From":"Jan Kara <jack@suse.cz>","To":"Kees Cook <keescook@chromium.org>","Cc":"linux-kernel@vger.kernel.org, David Windsor <dave@nullcore.net>,\n\tJan Kara <jack@suse.com>, linux-ext4@vger.kernel.org,\n\tlinux-mm@kvack.org, kernel-hardening@lists.openwall.com","Subject":"Re: [PATCH v2 08/30] ext2: Define usercopy region in\n\text2_inode_cache slab cache","Message-ID":"<20170830112247.GA30640@quack2.suse.cz>","References":"<1503956111-36652-1-git-send-email-keescook@chromium.org>\n\t<1503956111-36652-9-git-send-email-keescook@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<1503956111-36652-9-git-send-email-keescook@chromium.org>","User-Agent":"Mutt/1.5.24 (2015-08-30)","Sender":"linux-ext4-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<linux-ext4.vger.kernel.org>","X-Mailing-List":"linux-ext4@vger.kernel.org"}}]