Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1721983 > unrolled thread
| Started by | Kees Cook <keescook@chromium.org> |
|---|---|
| First post | 2017-08-28 23:40 +0200 |
| Last post | 2017-08-29 00:00 +0200 |
| Articles | 20 on this page of 51 — 7 participants |
Back to article view | Back to linux.kernel
[PATCH v2 00/30] Hardened usercopy whitelisting Kees Cook <keescook@chromium.org> - 2017-08-28 23:40 +0200
[PATCH v2 07/30] ext4: Define usercopy region in ext4_inode_cache slab cache Kees Cook <keescook@chromium.org> - 2017-08-28 23:40 +0200
[PATCH v2 09/30] jfs: Define usercopy region in jfs_ip slab cache Kees Cook <keescook@chromium.org> - 2017-08-28 23:40 +0200
[PATCH v2 06/30] vfs: Copy struct mount.mnt_id to userspace using put_user() Kees Cook <keescook@chromium.org> - 2017-08-28 23:40 +0200
[PATCH v2 05/30] vfs: Define usercopy region in names_cache slab caches Kees Cook <keescook@chromium.org> - 2017-08-28 23:40 +0200
[PATCH v2 08/30] ext2: Define usercopy region in ext2_inode_cache slab cache Kees Cook <keescook@chromium.org> - 2017-08-28 23:40 +0200
Re: [PATCH v2 08/30] ext2: Define usercopy region in ext2_inode_cache slab cache Jan Kara <jack@suse.cz> - 2017-08-30 13:30 +0200
[PATCH v2 13/30] ufs: Define usercopy region in ufs_inode_cache slab cache Kees Cook <keescook@chromium.org> - 2017-08-28 23:40 +0200
[PATCH v2 16/30] cifs: Define usercopy region in cifs_request slab cache Kees Cook <keescook@chromium.org> - 2017-08-28 23:40 +0200
[PATCH v2 02/30] usercopy: Enforce slab cache usercopy region boundaries Kees Cook <keescook@chromium.org> - 2017-08-28 23:40 +0200
[PATCH v2 20/30] caif: Define usercopy region in caif proto slab cache Kees Cook <keescook@chromium.org> - 2017-08-28 23:40 +0200
[PATCH v2 01/30] usercopy: Prepare for usercopy whitelisting Kees Cook <keescook@chromium.org> - 2017-08-28 23:40 +0200
[PATCH v2 11/30] exofs: Define usercopy region in exofs_inode_cache slab cache Kees Cook <keescook@chromium.org> - 2017-08-28 23:40 +0200
[PATCH v2 04/30] dcache: Define usercopy region in dentry_cache slab cache Kees Cook <keescook@chromium.org> - 2017-08-28 23:40 +0200
[PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache Kees Cook <keescook@chromium.org> - 2017-08-28 23:40 +0200
Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache Kees Cook <keescook@chromium.org> - 2017-08-29 00:00 +0200
Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache "Darrick J. Wong" <darrick.wong@oracle.com> - 2017-08-29 06:50 +0200
Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache Kees Cook <keescook@chromium.org> - 2017-08-29 20:50 +0200
Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache "Darrick J. Wong" <darrick.wong@oracle.com> - 2017-08-29 21:10 +0200
Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache Dave Chinner <david@fromorbit.com> - 2017-08-30 00:20 +0200
Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache Kees Cook <keescook@chromium.org> - 2017-08-30 00:30 +0200
Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache "Darrick J. Wong" <darrick.wong@oracle.com> - 2017-08-29 00:00 +0200
Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache Christoph Hellwig <hch@infradead.org> - 2017-08-29 10:20 +0200
Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache Dave Chinner <david@fromorbit.com> - 2017-08-29 14:40 +0200
Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache Christoph Hellwig <hch@infradead.org> - 2017-08-29 14:50 +0200
Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache Dave Chinner <david@fromorbit.com> - 2017-08-30 00:00 +0200
Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache Christoph Hellwig <hch@infradead.org> - 2017-08-30 09:20 +0200
Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache Dave Chinner <david@fromorbit.com> - 2017-08-30 10:10 +0200
Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache Christoph Hellwig <hch@infradead.org> - 2017-08-30 10:40 +0200
Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache Kees Cook <keescook@chromium.org> - 2017-08-29 21:00 +0200
[PATCH v2 10/30] befs: Define usercopy region in befs_inode_cache slab cache Kees Cook <keescook@chromium.org> - 2017-08-28 23:40 +0200
Re: [PATCH v2 10/30] befs: Define usercopy region in befs_inode_cache slab cache Luis de Bethencourt <luisbg@kernel.org> - 2017-08-29 12:20 +0200
Re: [PATCH v2 10/30] befs: Define usercopy region in befs_inode_cache slab cache Kees Cook <keescook@chromium.org> - 2017-08-29 17:40 +0200
Re: [PATCH v2 10/30] befs: Define usercopy region in befs_inode_cache slab cache Luis de Bethencourt <luisbg@kernel.org> - 2017-08-29 19:20 +0200
[PATCH v2 25/30] fork: Define usercopy region in thread_stack slab caches Kees Cook <keescook@chromium.org> - 2017-08-28 23:50 +0200
Re: [kernel-hardening] [PATCH v2 25/30] fork: Define usercopy region in thread_stack slab caches Rik van Riel <riel@redhat.com> - 2017-08-30 21:00 +0200
[PATCH v2 21/30] sctp: Define usercopy region in SCTP proto slab cache Kees Cook <keescook@chromium.org> - 2017-08-28 23:50 +0200
[PATCH v2 19/30] ip: Define usercopy region in IP proto slab cache Kees Cook <keescook@chromium.org> - 2017-08-28 23:50 +0200
[PATCH v2 22/30] sctp: Copy struct sctp_sock.autoclose to userspace using put_user() Kees Cook <keescook@chromium.org> - 2017-08-28 23:50 +0200
[PATCH v2 29/30] arm: Implement thread_struct whitelist for hardened usercopy Kees Cook <keescook@chromium.org> - 2017-08-28 23:50 +0200
[PATCH v2 30/30] usercopy: Restrict non-usercopy caches to size 0 Kees Cook <keescook@chromium.org> - 2017-08-28 23:50 +0200
[PATCH v2 14/30] vxfs: Define usercopy region in vxfs_inode slab cache Kees Cook <keescook@chromium.org> - 2017-08-28 23:50 +0200
[PATCH v2 24/30] fork: Define usercopy region in mm_struct slab caches Kees Cook <keescook@chromium.org> - 2017-08-28 23:50 +0200
Re: [kernel-hardening] [PATCH v2 24/30] fork: Define usercopy region in mm_struct slab caches Rik van Riel <riel@redhat.com> - 2017-08-30 21:30 +0200
[PATCH v2 23/30] net: Restrict unwhitelisted proto caches to size 0 Kees Cook <keescook@chromium.org> - 2017-08-28 23:50 +0200
[PATCH v2 26/30] fork: Provide usercopy whitelisting for task_struct Kees Cook <keescook@chromium.org> - 2017-08-28 23:50 +0200
Re: [kernel-hardening] [PATCH v2 26/30] fork: Provide usercopy whitelisting for task_struct Rik van Riel <riel@redhat.com> - 2017-08-30 21:00 +0200
[PATCH v2 28/30] arm64: Implement thread_struct whitelist for hardened usercopy Kees Cook <keescook@chromium.org> - 2017-08-28 23:50 +0200
[PATCH v2 27/30] x86: Implement thread_struct whitelist for hardened usercopy Kees Cook <keescook@chromium.org> - 2017-08-29 00:00 +0200
Re: [kernel-hardening] [PATCH v2 27/30] x86: Implement thread_struct whitelist for hardened usercopy Rik van Riel <riel@redhat.com> - 2017-08-30 21:00 +0200
[PATCH v2 18/30] net: Define usercopy region in struct proto slab cache Kees Cook <keescook@chromium.org> - 2017-08-29 00:00 +0200
Page 1 of 3 [1] 2 3 Next page →
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-08-28 23:40 +0200 |
| Subject | [PATCH v2 00/30] Hardened usercopy whitelisting |
| Message-ID | <ujzVf-2ie-3@gated-at.bofh.it> |
This series is modified from Brad Spengler/PaX Team's PAX_USERCOPY code
in the last public patch of grsecurity/PaX based on our understanding
of the code. Changes or omissions from the original code are ours and
don't reflect the original grsecurity/PaX code.
David Windsor did the bulk of the porting, refactoring, splitting,
testing, etc; I just did some extra tweaks, hunk moving, traces,
and extra patches.
Description from patch 1:
Currently, hardened usercopy performs dynamic bounds checking on slab
cache objects. This is good, but still leaves a lot of kernel memory
available to be copied to/from userspace in the face of bugs. To further
restrict what memory is available for copying, this creates a way to
whitelist specific areas of a given slab cache object for copying to/from
userspace, allowing much finer granularity of access control. Slab caches
that are never exposed to userspace can declare no whitelist for their
objects, thereby keeping them unavailable to userspace via dynamic copy
operations. (Note, an implicit form of whitelisting is the use of constant
sizes in usercopy operations and get_user()/put_user(); these bypass
hardened usercopy checks since these sizes cannot change at runtime.)
To support this whitelist annotation, usercopy region offset and size
members are added to struct kmem_cache. The slab allocator receives a
new function, kmem_cache_create_usercopy(), that creates a new cache
with a usercopy region defined, suitable for declaring spans of fields
within the objects that get copied to/from userspace.
In this patch, the default kmem_cache_create() marks the entire allocation
as whitelisted, leaving it semantically unchanged. Once all fine-grained
whitelists have been added (in subsequent patches), this will be changed
to a usersize of 0, making caches created with kmem_cache_create() not
copyable to/from userspace.
After the entire usercopy whitelist series is applied, less than 15%
of the slab cache memory remains exposed to potential usercopy bugs
after a fresh boot:
Total Slab Memory: 48074720
Usercopyable Memory: 6367532 13.2%
task_struct 0.2% 4480/1630720
RAW 0.3% 300/96000
RAWv6 2.1% 1408/64768
ext4_inode_cache 3.0% 269760/8740224
dentry 11.1% 585984/5273856
mm_struct 29.1% 54912/188448
kmalloc-8 100.0% 24576/24576
kmalloc-16 100.0% 28672/28672
kmalloc-32 100.0% 81920/81920
kmalloc-192 100.0% 96768/96768
kmalloc-128 100.0% 143360/143360
names_cache 100.0% 163840/163840
kmalloc-64 100.0% 167936/167936
kmalloc-256 100.0% 339968/339968
kmalloc-512 100.0% 350720/350720
kmalloc-96 100.0% 455616/455616
kmalloc-8192 100.0% 655360/655360
kmalloc-1024 100.0% 812032/812032
kmalloc-4096 100.0% 819200/819200
kmalloc-2048 100.0% 1310720/1310720
After some kernel build workloads, the percentage (mainly driven by
dentry and inode caches expanding) drops under 10%:
Total Slab Memory: 95516184
Usercopyable Memory: 8497452 8.8%
task_struct 0.2% 4000/1456000
RAW 0.3% 300/96000
RAWv6 2.1% 1408/64768
ext4_inode_cache 3.0% 1217280/39439872
dentry 11.1% 1623200/14608800
mm_struct 29.1% 73216/251264
kmalloc-8 100.0% 24576/24576
kmalloc-16 100.0% 28672/28672
kmalloc-32 100.0% 94208/94208
kmalloc-192 100.0% 96768/96768
kmalloc-128 100.0% 143360/143360
names_cache 100.0% 163840/163840
kmalloc-64 100.0% 245760/245760
kmalloc-256 100.0% 339968/339968
kmalloc-512 100.0% 350720/350720
kmalloc-96 100.0% 563520/563520
kmalloc-8192 100.0% 655360/655360
kmalloc-1024 100.0% 794624/794624
kmalloc-4096 100.0% 819200/819200
kmalloc-2048 100.0% 1257472/1257472
------
The patches are broken in several stages of changes:
Prepare and whitelist kmalloc:
[PATCH 01/30] usercopy: Prepare for usercopy whitelisting
[PATCH 02/30] usercopy: Enforce slab cache usercopy region boundaries
[PATCH 03/30] usercopy: Mark kmalloc caches as usercopy caches
Update VFS layer for symlinks and other inline storage:
[PATCH 04/30] dcache: Define usercopy region in dentry_cache slab
[PATCH 05/30] vfs: Define usercopy region in names_cache slab caches
[PATCH 06/30] vfs: Copy struct mount.mnt_id to userspace using
[PATCH 07/30] ext4: Define usercopy region in ext4_inode_cache slab
[PATCH 08/30] ext2: Define usercopy region in ext2_inode_cache slab
[PATCH 09/30] jfs: Define usercopy region in jfs_ip slab cache
[PATCH 10/30] befs: Define usercopy region in befs_inode_cache slab
[PATCH 11/30] exofs: Define usercopy region in exofs_inode_cache slab
[PATCH 12/30] orangefs: Define usercopy region in
[PATCH 13/30] ufs: Define usercopy region in ufs_inode_cache slab
[PATCH 14/30] vxfs: Define usercopy region in vxfs_inode slab cache
[PATCH 15/30] xfs: Define usercopy region in xfs_inode slab cache
[PATCH 16/30] cifs: Define usercopy region in cifs_request slab cache
Update scsi layer for inline storage:
[PATCH 17/30] scsi: Define usercopy region in scsi_sense_cache slab
Whitelist a few network protocol-specific areas of memory:
[PATCH 18/30] net: Define usercopy region in struct proto slab cache
[PATCH 19/30] ip: Define usercopy region in IP proto slab cache
[PATCH 20/30] caif: Define usercopy region in caif proto slab cache
[PATCH 21/30] sctp: Define usercopy region in SCTP proto slab cache
[PATCH 22/30] sctp: Copy struct sctp_sock.autoclose to userspace
[PATCH 23/30] net: Restrict unwhitelisted proto caches to size 0
Whitelist areas of process memory:
[PATCH 24/30] fork: Define usercopy region in mm_struct slab caches
[PATCH 25/30] fork: Define usercopy region in thread_stack slab
Deal with per-architecture thread_struct whitelisting:
[PATCH 26/30] fork: Provide usercopy whitelisting for task_struct
[PATCH 27/30] x86: Implement thread_struct whitelist for hardened
[PATCH 28/30] arm64: Implement thread_struct whitelist for hardened
[PATCH 29/30] arm: Implement thread_struct whitelist for hardened
Make blacklisting the default:
[PATCH 30/30] usercopy: Restrict non-usercopy caches to size 0
v2:
- added tracing of allocation and usage
- refactored solutions for task_struct
- split up network patches for readability
I intend for this to land via my usercopy hardening tree, so Acks,
Reviewed, and Tested-bys would be greatly appreciated. I have some
questions in a few patches (e.g. CIFS and thread_stack) that would be
nice to get answered for completeness. FWIW, this series has survived
over the weekend in 0-day testing.
Thanks!
-Kees (and David)
[toc] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-08-28 23:40 +0200 |
| Subject | [PATCH v2 07/30] ext4: Define usercopy region in ext4_inode_cache slab cache |
| Message-ID | <ujzVf-2ie-9@gated-at.bofh.it> |
| In reply to | #1721983 |
From: David Windsor <dave@nullcore.net>
The ext4 symlink pathnames, stored in struct ext4_inode_info.i_data
and therefore contained in the ext4_inode_cache slab cache, need
to be copied to/from userspace.
cache object allocation:
fs/ext4/super.c:
ext4_alloc_inode(...):
struct ext4_inode_info *ei;
...
ei = kmem_cache_alloc(ext4_inode_cachep, GFP_NOFS);
...
return &ei->vfs_inode;
include/trace/events/ext4.h:
#define EXT4_I(inode) \
(container_of(inode, struct ext4_inode_info, vfs_inode))
fs/ext4/namei.c:
ext4_symlink(...):
...
inode->i_link = (char *)&EXT4_I(inode)->i_data;
example usage trace:
readlink_copy+0x43/0x70
vfs_readlink+0x62/0x110
SyS_readlinkat+0x100/0x130
fs/namei.c:
readlink_copy(..., link):
...
copy_to_user(..., link, len)
(inlined into vfs_readlink)
generic_readlink(dentry, ...):
struct inode *inode = d_inode(dentry);
const char *link = inode->i_link;
...
readlink_copy(..., link);
In support of usercopy hardening, this patch defines a region in the
ext4_inode_cache slab cache in which userspace copy operations are
allowed.
This region is known as the slab cache's usercopy region. Slab caches can
now check that each copy operation involving cache-managed memory falls
entirely within the slab's usercopy region.
This patch is modified from Brad Spengler/PaX Team's PAX_USERCOPY
whitelisting code in the last public patch of grsecurity/PaX based on my
understanding of the code. Changes or omissions from the original code are
mine and don't reflect the original grsecurity/PaX code.
Signed-off-by: David Windsor <dave@nullcore.net>
[kees: adjust commit log, provide usage trace]
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Andreas Dilger <adilger.kernel@dilger.ca>
Cc: linux-ext4@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
fs/ext4/super.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 0886fe82e9c4..79c3b1b11364 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1038,11 +1038,13 @@ static void init_once(void *foo)
static int __init init_inodecache(void)
{
- ext4_inode_cachep = kmem_cache_create("ext4_inode_cache",
- sizeof(struct ext4_inode_info),
- 0, (SLAB_RECLAIM_ACCOUNT|
- SLAB_MEM_SPREAD|SLAB_ACCOUNT),
- init_once);
+ ext4_inode_cachep = kmem_cache_create_usercopy("ext4_inode_cache",
+ sizeof(struct ext4_inode_info), 0,
+ (SLAB_RECLAIM_ACCOUNT|SLAB_MEM_SPREAD|
+ SLAB_ACCOUNT),
+ offsetof(struct ext4_inode_info, i_data),
+ sizeof_field(struct ext4_inode_info, i_data),
+ init_once);
if (ext4_inode_cachep == NULL)
return -ENOMEM;
return 0;
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-08-28 23:40 +0200 |
| Subject | [PATCH v2 09/30] jfs: Define usercopy region in jfs_ip slab cache |
| Message-ID | <ujzVg-2ie-23@gated-at.bofh.it> |
| In reply to | #1721983 |
From: David Windsor <dave@nullcore.net>
The jfs symlink pathnames, stored in struct jfs_inode_info.i_inline and
therefore contained in the jfs_ip slab cache, need to be copied to/from
userspace.
cache object allocation:
fs/jfs/super.c:
jfs_alloc_inode(...):
...
jfs_inode = kmem_cache_alloc(jfs_inode_cachep, GFP_NOFS);
...
return &jfs_inode->vfs_inode;
fs/jfs/jfs_incore.h:
JFS_IP(struct inode *inode):
return container_of(inode, struct jfs_inode_info, vfs_inode);
fs/jfs/inode.c:
jfs_iget(...):
...
inode->i_link = JFS_IP(inode)->i_inline;
example usage trace:
readlink_copy+0x43/0x70
vfs_readlink+0x62/0x110
SyS_readlinkat+0x100/0x130
fs/namei.c:
readlink_copy(..., link):
...
copy_to_user(..., link, len);
(inlined in vfs_readlink)
generic_readlink(dentry, ...):
struct inode *inode = d_inode(dentry);
const char *link = inode->i_link;
...
readlink_copy(..., link);
In support of usercopy hardening, this patch defines a region in the
jfs_ip slab cache in which userspace copy operations are allowed.
This region is known as the slab cache's usercopy region. Slab caches can
now check that each copy operation involving cache-managed memory falls
entirely within the slab's usercopy region.
This patch is modified from Brad Spengler/PaX Team's PAX_USERCOPY
whitelisting code in the last public patch of grsecurity/PaX based on my
understanding of the code. Changes or omissions from the original code are
mine and don't reflect the original grsecurity/PaX code.
Signed-off-by: David Windsor <dave@nullcore.net>
[kees: adjust commit log, provide usage trace]
Cc: Dave Kleikamp <shaggy@kernel.org>
Cc: jfs-discussion@lists.sourceforge.net
Signed-off-by: Kees Cook <keescook@chromium.org>
---
fs/jfs/super.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/fs/jfs/super.c b/fs/jfs/super.c
index e8aad7d87b8c..10b958f49f57 100644
--- a/fs/jfs/super.c
+++ b/fs/jfs/super.c
@@ -972,9 +972,11 @@ static int __init init_jfs_fs(void)
int rc;
jfs_inode_cachep =
- kmem_cache_create("jfs_ip", sizeof(struct jfs_inode_info), 0,
- SLAB_RECLAIM_ACCOUNT|SLAB_MEM_SPREAD|SLAB_ACCOUNT,
- init_once);
+ kmem_cache_create_usercopy("jfs_ip", sizeof(struct jfs_inode_info),
+ 0, SLAB_RECLAIM_ACCOUNT|SLAB_MEM_SPREAD|SLAB_ACCOUNT,
+ offsetof(struct jfs_inode_info, i_inline),
+ sizeof_field(struct jfs_inode_info, i_inline),
+ init_once);
if (jfs_inode_cachep == NULL)
return -ENOMEM;
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-08-28 23:40 +0200 |
| Subject | [PATCH v2 06/30] vfs: Copy struct mount.mnt_id to userspace using put_user() |
| Message-ID | <ujzVf-2ie-11@gated-at.bofh.it> |
| In reply to | #1721983 |
From: David Windsor <dave@nullcore.net> The mnt_id field can be copied with put_user(), so there is no need to use copy_to_user(). In both cases, hardened usercopy is being bypassed since the size is constant, and not open to runtime manipulation. This patch is verbatim from Brad Spengler/PaX Team's PAX_USERCOPY whitelisting code in the last public patch of grsecurity/PaX based on my understanding of the code. Changes or omissions from the original code are mine and don't reflect the original grsecurity/PaX code. Signed-off-by: David Windsor <dave@nullcore.net> [kees: adjust commit log] Cc: Alexander Viro <viro@zeniv.linux.org.uk> Cc: linux-fsdevel@vger.kernel.org Signed-off-by: Kees Cook <keescook@chromium.org> --- fs/fhandle.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/fhandle.c b/fs/fhandle.c index 58a61f55e0d0..46e00ccca8f0 100644 --- a/fs/fhandle.c +++ b/fs/fhandle.c @@ -68,8 +68,7 @@ static long do_sys_name_to_handle(struct path *path, } else retval = 0; /* copy the mount id */ - if (copy_to_user(mnt_id, &real_mount(path->mnt)->mnt_id, - sizeof(*mnt_id)) || + if (put_user(real_mount(path->mnt)->mnt_id, mnt_id) || copy_to_user(ufh, handle, sizeof(struct file_handle) + handle_bytes)) retval = -EFAULT; -- 2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-08-28 23:40 +0200 |
| Subject | [PATCH v2 05/30] vfs: Define usercopy region in names_cache slab caches |
| Message-ID | <ujzVg-2ie-21@gated-at.bofh.it> |
| In reply to | #1721983 |
From: David Windsor <dave@nullcore.net>
VFS pathnames are stored in the names_cache slab cache, either inline
or across an entire allocation entry (when approaching PATH_MAX). These
are copied to/from userspace, so they must be entirely whitelisted.
cache object allocation:
include/linux/fs.h:
#define __getname() kmem_cache_alloc(names_cachep, GFP_KERNEL)
example usage trace:
strncpy_from_user+0x4d/0x170
getname_flags+0x6f/0x1f0
user_path_at_empty+0x23/0x40
do_mount+0x69/0xda0
SyS_mount+0x83/0xd0
fs/namei.c:
getname_flags(...):
...
result = __getname();
...
kname = (char *)result->iname;
result->name = kname;
len = strncpy_from_user(kname, filename, EMBEDDED_NAME_MAX);
...
if (unlikely(len == EMBEDDED_NAME_MAX)) {
const size_t size = offsetof(struct filename, iname[1]);
kname = (char *)result;
result = kzalloc(size, GFP_KERNEL);
...
result->name = kname;
len = strncpy_from_user(kname, filename, PATH_MAX);
In support of usercopy hardening, this patch defines the entire cache
object in the names_cache slab cache as whitelisted, since it may entirely
hold name strings to be copied to/from userspace.
This patch is verbatim from Brad Spengler/PaX Team's PAX_USERCOPY
whitelisting code in the last public patch of grsecurity/PaX based on my
understanding of the code. Changes or omissions from the original code are
mine and don't reflect the original grsecurity/PaX code.
Signed-off-by: David Windsor <dave@nullcore.net>
[kees: adjust commit log, add usage trace]
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
fs/dcache.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/dcache.c b/fs/dcache.c
index 5f5e7c1fcf4b..34ef9a9169be 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -3642,8 +3642,8 @@ void __init vfs_caches_init_early(void)
void __init vfs_caches_init(void)
{
- names_cachep = kmem_cache_create("names_cache", PATH_MAX, 0,
- SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
+ names_cachep = kmem_cache_create_usercopy("names_cache", PATH_MAX, 0,
+ SLAB_HWCACHE_ALIGN|SLAB_PANIC, 0, PATH_MAX, NULL);
dcache_init();
inode_init();
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-08-28 23:40 +0200 |
| Subject | [PATCH v2 08/30] ext2: Define usercopy region in ext2_inode_cache slab cache |
| Message-ID | <ujzVf-2ie-13@gated-at.bofh.it> |
| In reply to | #1721983 |
From: David Windsor <dave@nullcore.net>
The ext2 symlink pathnames, stored in struct ext2_inode_info.i_data and
therefore contained in the ext2_inode_cache slab cache, need to be copied
to/from userspace.
cache object allocation:
fs/ext2/super.c:
ext2_alloc_inode(...):
struct ext2_inode_info *ei;
...
ei = kmem_cache_alloc(ext2_inode_cachep, GFP_NOFS);
...
return &ei->vfs_inode;
fs/ext2/ext2.h:
EXT2_I(struct inode *inode):
return container_of(inode, struct ext2_inode_info, vfs_inode);
fs/ext2/namei.c:
ext2_symlink(...):
...
inode->i_link = (char *)&EXT2_I(inode)->i_data;
example usage trace:
readlink_copy+0x43/0x70
vfs_readlink+0x62/0x110
SyS_readlinkat+0x100/0x130
fs/namei.c:
readlink_copy(..., link):
...
copy_to_user(..., link, len);
(inlined into vfs_readlink)
generic_readlink(dentry, ...):
struct inode *inode = d_inode(dentry);
const char *link = inode->i_link;
...
readlink_copy(..., link);
In support of usercopy hardening, this patch defines a region in the
ext2_inode_cache slab cache in which userspace copy operations are
allowed.
This region is known as the slab cache's usercopy region. Slab caches can
now check that each copy operation involving cache-managed memory falls
entirely within the slab's usercopy region.
This patch is modified from Brad Spengler/PaX Team's PAX_USERCOPY
whitelisting code in the last public patch of grsecurity/PaX based on my
understanding of the code. Changes or omissions from the original code are
mine and don't reflect the original grsecurity/PaX code.
Signed-off-by: David Windsor <dave@nullcore.net>
[kees: adjust commit log, provide usage trace]
Cc: Jan Kara <jack@suse.com>
Cc: linux-ext4@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
fs/ext2/super.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 7b1bc9059863..670142cde59d 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -219,11 +219,13 @@ static void init_once(void *foo)
static int __init init_inodecache(void)
{
- ext2_inode_cachep = kmem_cache_create("ext2_inode_cache",
- sizeof(struct ext2_inode_info),
- 0, (SLAB_RECLAIM_ACCOUNT|
- SLAB_MEM_SPREAD|SLAB_ACCOUNT),
- init_once);
+ ext2_inode_cachep = kmem_cache_create_usercopy("ext2_inode_cache",
+ sizeof(struct ext2_inode_info), 0,
+ (SLAB_RECLAIM_ACCOUNT|SLAB_MEM_SPREAD|
+ SLAB_ACCOUNT),
+ offsetof(struct ext2_inode_info, i_data),
+ sizeof_field(struct ext2_inode_info, i_data),
+ init_once);
if (ext2_inode_cachep == NULL)
return -ENOMEM;
return 0;
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Jan Kara <jack@suse.cz> |
|---|---|
| Date | 2017-08-30 13:30 +0200 |
| Subject | Re: [PATCH v2 08/30] ext2: Define usercopy region in ext2_inode_cache slab cache |
| Message-ID | <uk9m1-7F5-11@gated-at.bofh.it> |
| In reply to | #1721988 |
On Mon 28-08-17 14:34:49, Kees Cook wrote:
> From: David Windsor <dave@nullcore.net>
>
> The ext2 symlink pathnames, stored in struct ext2_inode_info.i_data and
> therefore contained in the ext2_inode_cache slab cache, need to be copied
> to/from userspace.
>
> cache object allocation:
> fs/ext2/super.c:
> ext2_alloc_inode(...):
> struct ext2_inode_info *ei;
> ...
> ei = kmem_cache_alloc(ext2_inode_cachep, GFP_NOFS);
> ...
> return &ei->vfs_inode;
>
> fs/ext2/ext2.h:
> EXT2_I(struct inode *inode):
> return container_of(inode, struct ext2_inode_info, vfs_inode);
>
> fs/ext2/namei.c:
> ext2_symlink(...):
> ...
> inode->i_link = (char *)&EXT2_I(inode)->i_data;
>
> example usage trace:
> readlink_copy+0x43/0x70
> vfs_readlink+0x62/0x110
> SyS_readlinkat+0x100/0x130
>
> fs/namei.c:
> readlink_copy(..., link):
> ...
> copy_to_user(..., link, len);
>
> (inlined into vfs_readlink)
> generic_readlink(dentry, ...):
> struct inode *inode = d_inode(dentry);
> const char *link = inode->i_link;
> ...
> readlink_copy(..., link);
>
> In support of usercopy hardening, this patch defines a region in the
> ext2_inode_cache slab cache in which userspace copy operations are
> allowed.
>
> This region is known as the slab cache's usercopy region. Slab caches can
> now check that each copy operation involving cache-managed memory falls
> entirely within the slab's usercopy region.
>
> This patch is modified from Brad Spengler/PaX Team's PAX_USERCOPY
> whitelisting code in the last public patch of grsecurity/PaX based on my
> understanding of the code. Changes or omissions from the original code are
> mine and don't reflect the original grsecurity/PaX code.
>
> Signed-off-by: David Windsor <dave@nullcore.net>
> [kees: adjust commit log, provide usage trace]
> Cc: Jan Kara <jack@suse.com>
> Cc: linux-ext4@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
Looks good. You can add:
Acked-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext2/super.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ext2/super.c b/fs/ext2/super.c
> index 7b1bc9059863..670142cde59d 100644
> --- a/fs/ext2/super.c
> +++ b/fs/ext2/super.c
> @@ -219,11 +219,13 @@ static void init_once(void *foo)
>
> static int __init init_inodecache(void)
> {
> - ext2_inode_cachep = kmem_cache_create("ext2_inode_cache",
> - sizeof(struct ext2_inode_info),
> - 0, (SLAB_RECLAIM_ACCOUNT|
> - SLAB_MEM_SPREAD|SLAB_ACCOUNT),
> - init_once);
> + ext2_inode_cachep = kmem_cache_create_usercopy("ext2_inode_cache",
> + sizeof(struct ext2_inode_info), 0,
> + (SLAB_RECLAIM_ACCOUNT|SLAB_MEM_SPREAD|
> + SLAB_ACCOUNT),
> + offsetof(struct ext2_inode_info, i_data),
> + sizeof_field(struct ext2_inode_info, i_data),
> + init_once);
> if (ext2_inode_cachep == NULL)
> return -ENOMEM;
> return 0;
> --
> 2.7.4
>
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-08-28 23:40 +0200 |
| Subject | [PATCH v2 13/30] ufs: Define usercopy region in ufs_inode_cache slab cache |
| Message-ID | <ujzVg-2ie-29@gated-at.bofh.it> |
| In reply to | #1721983 |
From: David Windsor <dave@nullcore.net>
The ufs symlink pathnames, stored in struct ufs_inode_info.i_u1.i_symlink
and therefore contained in the ufs_inode_cache slab cache, need to be
copied to/from userspace.
cache object allocation:
fs/ufs/super.c:
ufs_alloc_inode(...):
...
ei = kmem_cache_alloc(ufs_inode_cachep, GFP_NOFS);
...
return &ei->vfs_inode;
fs/ufs/ufs.h:
UFS_I(struct inode *inode):
return container_of(inode, struct ufs_inode_info, vfs_inode);
fs/ufs/namei.c:
ufs_symlink(...):
...
inode->i_link = (char *)UFS_I(inode)->i_u1.i_symlink;
example usage trace:
readlink_copy+0x43/0x70
vfs_readlink+0x62/0x110
SyS_readlinkat+0x100/0x130
fs/namei.c:
readlink_copy(..., link):
...
copy_to_user(..., link, len);
(inlined in vfs_readlink)
generic_readlink(dentry, ...):
struct inode *inode = d_inode(dentry);
const char *link = inode->i_link;
...
readlink_copy(..., link);
In support of usercopy hardening, this patch defines a region in the
ufs_inode_cache slab cache in which userspace copy operations are allowed.
This region is known as the slab cache's usercopy region. Slab caches can
now check that each copy operation involving cache-managed memory falls
entirely within the slab's usercopy region.
This patch is modified from Brad Spengler/PaX Team's PAX_USERCOPY
whitelisting code in the last public patch of grsecurity/PaX based on my
understanding of the code. Changes or omissions from the original code are
mine and don't reflect the original grsecurity/PaX code.
Signed-off-by: David Windsor <dave@nullcore.net>
[kees: adjust commit log, provide usage trace]
Cc: Evgeniy Dushistov <dushistov@mail.ru>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
fs/ufs/super.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/fs/ufs/super.c b/fs/ufs/super.c
index 0a4f58a5073c..646f971067bc 100644
--- a/fs/ufs/super.c
+++ b/fs/ufs/super.c
@@ -1466,11 +1466,14 @@ static void init_once(void *foo)
static int __init init_inodecache(void)
{
- ufs_inode_cachep = kmem_cache_create("ufs_inode_cache",
- sizeof(struct ufs_inode_info),
- 0, (SLAB_RECLAIM_ACCOUNT|
- SLAB_MEM_SPREAD|SLAB_ACCOUNT),
- init_once);
+ ufs_inode_cachep = kmem_cache_create_usercopy("ufs_inode_cache",
+ sizeof(struct ufs_inode_info), 0,
+ (SLAB_RECLAIM_ACCOUNT|SLAB_MEM_SPREAD|
+ SLAB_ACCOUNT),
+ offsetof(struct ufs_inode_info, i_u1.i_symlink),
+ sizeof_field(struct ufs_inode_info,
+ i_u1.i_symlink),
+ init_once);
if (ufs_inode_cachep == NULL)
return -ENOMEM;
return 0;
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-08-28 23:40 +0200 |
| Subject | [PATCH v2 16/30] cifs: Define usercopy region in cifs_request slab cache |
| Message-ID | <ujzVg-2ie-31@gated-at.bofh.it> |
| In reply to | #1721983 |
From: David Windsor <dave@nullcore.net>
CIFS request buffers, stored in the cifs_request slab cache, need to be
copied to/from userspace.
cache object allocation:
fs/cifs/cifsfs.c:
cifs_init_request_bufs():
...
cifs_req_poolp = mempool_create_slab_pool(cifs_min_rcv,
cifs_req_cachep);
fs/cifs/misc.c:
cifs_buf_get():
...
ret_buf = mempool_alloc(cifs_req_poolp, GFP_NOFS);
...
return ret_buf;
In support of usercopy hardening, this patch defines a region in the
cifs_request slab cache in which userspace copy operations are allowed.
This region is known as the slab cache's usercopy region. Slab
caches can now check that each copy operation involving cache-managed
memory falls entirely within the slab's usercopy region.
This patch is verbatim from Brad Spengler/PaX Team's PAX_USERCOPY
whitelisting code in the last public patch of grsecurity/PaX based on my
understanding of the code. Changes or omissions from the original code are
mine and don't reflect the original grsecurity/PaX code.
Signed-off-by: David Windsor <dave@nullcore.net>
[kees: adjust commit log, provide usage trace]
Cc: Steve French <sfrench@samba.org>
Cc: linux-cifs@vger.kernel.org
Cc: samba-technical@lists.samba.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
fs/cifs/cifsfs.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 180b3356ff86..09dfdf76c738 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -1229,9 +1229,11 @@ cifs_init_request_bufs(void)
cifs_dbg(VFS, "CIFSMaxBufSize %d 0x%x\n",
CIFSMaxBufSize, CIFSMaxBufSize);
*/
- cifs_req_cachep = kmem_cache_create("cifs_request",
+ cifs_req_cachep = kmem_cache_create_usercopy("cifs_request",
CIFSMaxBufSize + max_hdr_size, 0,
- SLAB_HWCACHE_ALIGN, NULL);
+ SLAB_HWCACHE_ALIGN, 0,
+ CIFSMaxBufSize + max_hdr_size,
+ NULL);
if (cifs_req_cachep == NULL)
return -ENOMEM;
@@ -1257,9 +1259,9 @@ cifs_init_request_bufs(void)
more SMBs to use small buffer alloc and is still much more
efficient to alloc 1 per page off the slab compared to 17K (5page)
alloc of large cifs buffers even when page debugging is on */
- cifs_sm_req_cachep = kmem_cache_create("cifs_small_rq",
+ cifs_sm_req_cachep = kmem_cache_create_usercopy("cifs_small_rq",
MAX_CIFS_SMALL_BUFFER_SIZE, 0, SLAB_HWCACHE_ALIGN,
- NULL);
+ 0, MAX_CIFS_SMALL_BUFFER_SIZE, NULL);
if (cifs_sm_req_cachep == NULL) {
mempool_destroy(cifs_req_poolp);
kmem_cache_destroy(cifs_req_cachep);
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-08-28 23:40 +0200 |
| Subject | [PATCH v2 02/30] usercopy: Enforce slab cache usercopy region boundaries |
| Message-ID | <ujzVf-2ie-19@gated-at.bofh.it> |
| In reply to | #1721983 |
From: David Windsor <dave@nullcore.net>
This patch adds the enforcement component of usercopy cache whitelisting,
and is modified from Brad Spengler/PaX Team's PAX_USERCOPY whitelisting
code in the last public patch of grsecurity/PaX based on my understanding
of the code. Changes or omissions from the original code are mine and
don't reflect the original grsecurity/PaX code.
The SLAB and SLUB allocators are modified to deny all copy operations
in which the kernel heap memory being modified falls outside of the cache's
defined usercopy region.
Signed-off-by: David Windsor <dave@nullcore.net>
[kees: adjust commit log and comments]
Cc: Christoph Lameter <cl@linux.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Laura Abbott <labbott@redhat.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: linux-mm@kvack.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
mm/slab.c | 16 +++++++++++-----
mm/slub.c | 18 +++++++++++-------
mm/usercopy.c | 12 ++++++++++++
3 files changed, 34 insertions(+), 12 deletions(-)
diff --git a/mm/slab.c b/mm/slab.c
index 87b6e5e0cdaf..df268999cf02 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -4408,7 +4408,9 @@ module_init(slab_proc_init);
#ifdef CONFIG_HARDENED_USERCOPY
/*
- * Rejects objects that are incorrectly sized.
+ * Rejects incorrectly sized objects and objects that are to be copied
+ * to/from userspace but do not fall entirely within the containing slab
+ * cache's usercopy region.
*
* Returns NULL if check passes, otherwise const char * to name of cache
* to indicate an error.
@@ -4428,11 +4430,15 @@ const char *__check_heap_object(const void *ptr, unsigned long n,
/* Find offset within object. */
offset = ptr - index_to_obj(cachep, page, objnr) - obj_offset(cachep);
- /* Allow address range falling entirely within object size. */
- if (offset <= cachep->object_size && n <= cachep->object_size - offset)
- return NULL;
+ /* Make sure object falls entirely within cache's usercopy region. */
+ if (offset < cachep->useroffset)
+ return cachep->name;
+ if (offset - cachep->useroffset > cachep->usersize)
+ return cachep->name;
+ if (n > cachep->useroffset - offset + cachep->usersize)
+ return cachep->name;
- return cachep->name;
+ return NULL;
}
#endif /* CONFIG_HARDENED_USERCOPY */
diff --git a/mm/slub.c b/mm/slub.c
index dfed8ef99b68..bd4e2b9d4524 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3797,7 +3797,9 @@ EXPORT_SYMBOL(__kmalloc_node);
#ifdef CONFIG_HARDENED_USERCOPY
/*
- * Rejects objects that are incorrectly sized.
+ * Rejects incorrectly sized objects and objects that are to be copied
+ * to/from userspace but do not fall entirely within the containing slab
+ * cache's usercopy region.
*
* Returns NULL if check passes, otherwise const char * to name of cache
* to indicate an error.
@@ -3807,11 +3809,9 @@ const char *__check_heap_object(const void *ptr, unsigned long n,
{
struct kmem_cache *s;
unsigned long offset;
- size_t object_size;
/* Find object and usable object size. */
s = page->slab_cache;
- object_size = slab_ksize(s);
/* Reject impossible pointers. */
if (ptr < page_address(page))
@@ -3827,11 +3827,15 @@ const char *__check_heap_object(const void *ptr, unsigned long n,
offset -= s->red_left_pad;
}
- /* Allow address range falling entirely within object size. */
- if (offset <= object_size && n <= object_size - offset)
- return NULL;
+ /* Make sure object falls entirely within cache's usercopy region. */
+ if (offset < s->useroffset)
+ return s->name;
+ if (offset - s->useroffset > s->usersize)
+ return s->name;
+ if (n > s->useroffset - offset + s->usersize)
+ return s->name;
- return s->name;
+ return NULL;
}
#endif /* CONFIG_HARDENED_USERCOPY */
diff --git a/mm/usercopy.c b/mm/usercopy.c
index a9852b24715d..cbffde670c49 100644
--- a/mm/usercopy.c
+++ b/mm/usercopy.c
@@ -58,6 +58,18 @@ static noinline int check_stack_object(const void *obj, unsigned long len)
return GOOD_STACK;
}
+/*
+ * If this function is reached, then CONFIG_HARDENED_USERCOPY has found an
+ * unexpected state during a copy_from_user() or copy_to_user() call.
+ * There are several checks being performed on the buffer by the
+ * __check_object_size() function. Normal stack buffer usage should never
+ * trip the checks, and kernel text addressing will always trip the check.
+ * For cache objects, it is checking that only the whitelisted range of
+ * bytes for a given cache is being accessed (via the cache's usersize and
+ * useroffset fields). To adjust a cache whitelist, use the usercopy-aware
+ * kmem_cache_create_usercopy() function to create the cache (and
+ * carefully audit the whitelist range).
+ */
static void report_usercopy(const void *ptr, unsigned long len,
bool to_user, const char *type)
{
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-08-28 23:40 +0200 |
| Subject | [PATCH v2 20/30] caif: Define usercopy region in caif proto slab cache |
| Message-ID | <ujzVg-2ie-33@gated-at.bofh.it> |
| In reply to | #1721983 |
From: David Windsor <dave@nullcore.net>
The CAIF channel connection request parameters need to be copied to/from
userspace. In support of usercopy hardening, this patch defines a region
in the struct proto slab cache in which userspace copy operations are
allowed.
example usage trace:
net/caif/caif_socket.c:
setsockopt(...):
...
copy_from_user(&cf_sk->conn_req.param.data, ..., ol)
This region is known as the slab cache's usercopy region. Slab caches can
now check that each copy operation involving cache-managed memory falls
entirely within the slab's usercopy region.
This patch is modified from Brad Spengler/PaX Team's PAX_USERCOPY
whitelisting code in the last public patch of grsecurity/PaX based on my
understanding of the code. Changes or omissions from the original code are
mine and don't reflect the original grsecurity/PaX code.
Signed-off-by: David Windsor <dave@nullcore.net>
[kees: split from network patch, provide usage trace]
Cc: Dmitry Tarnyagin <dmitry.tarnyagin@lockless.no>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
net/caif/caif_socket.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c
index 632d5a416d97..c76d513b9a7a 100644
--- a/net/caif/caif_socket.c
+++ b/net/caif/caif_socket.c
@@ -1032,6 +1032,8 @@ static int caif_create(struct net *net, struct socket *sock, int protocol,
static struct proto prot = {.name = "PF_CAIF",
.owner = THIS_MODULE,
.obj_size = sizeof(struct caifsock),
+ .useroffset = offsetof(struct caifsock, conn_req.param),
+ .usersize = sizeof_field(struct caifsock, conn_req.param)
};
if (!capable(CAP_SYS_ADMIN) && !capable(CAP_NET_ADMIN))
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-08-28 23:40 +0200 |
| Subject | [PATCH v2 01/30] usercopy: Prepare for usercopy whitelisting |
| Message-ID | <ujzVg-2ie-39@gated-at.bofh.it> |
| In reply to | #1721983 |
From: David Windsor <dave@nullcore.net>
This patch prepares the slab allocator to handle caches having annotations
(useroffset and usersize) defining usercopy regions.
This patch is modified from Brad Spengler/PaX Team's PAX_USERCOPY
whitelisting code in the last public patch of grsecurity/PaX based on
my understanding of the code. Changes or omissions from the original
code are mine and don't reflect the original grsecurity/PaX code.
Currently, hardened usercopy performs dynamic bounds checking on slab
cache objects. This is good, but still leaves a lot of kernel memory
available to be copied to/from userspace in the face of bugs. To further
restrict what memory is available for copying, this creates a way to
whitelist specific areas of a given slab cache object for copying to/from
userspace, allowing much finer granularity of access control. Slab caches
that are never exposed to userspace can declare no whitelist for their
objects, thereby keeping them unavailable to userspace via dynamic copy
operations. (Note, an implicit form of whitelisting is the use of constant
sizes in usercopy operations and get_user()/put_user(); these bypass
hardened usercopy checks since these sizes cannot change at runtime.)
To support this whitelist annotation, usercopy region offset and size
members are added to struct kmem_cache. The slab allocator receives a
new function, kmem_cache_create_usercopy(), that creates a new cache
with a usercopy region defined, suitable for declaring spans of fields
within the objects that get copied to/from userspace.
In this patch, the default kmem_cache_create() marks the entire allocation
as whitelisted, leaving it semantically unchanged. Once all fine-grained
whitelists have been added (in subsequent patches), this will be changed
to a usersize of 0, making caches created with kmem_cache_create() not
copyable to/from userspace.
After the entire usercopy whitelist series is applied, less than 15%
of the slab cache memory remains exposed to potential usercopy bugs
after a fresh boot:
Total Slab Memory: 48074720
Usercopyable Memory: 6367532 13.2%
task_struct 0.2% 4480/1630720
RAW 0.3% 300/96000
RAWv6 2.1% 1408/64768
ext4_inode_cache 3.0% 269760/8740224
dentry 11.1% 585984/5273856
mm_struct 29.1% 54912/188448
kmalloc-8 100.0% 24576/24576
kmalloc-16 100.0% 28672/28672
kmalloc-32 100.0% 81920/81920
kmalloc-192 100.0% 96768/96768
kmalloc-128 100.0% 143360/143360
names_cache 100.0% 163840/163840
kmalloc-64 100.0% 167936/167936
kmalloc-256 100.0% 339968/339968
kmalloc-512 100.0% 350720/350720
kmalloc-96 100.0% 455616/455616
kmalloc-8192 100.0% 655360/655360
kmalloc-1024 100.0% 812032/812032
kmalloc-4096 100.0% 819200/819200
kmalloc-2048 100.0% 1310720/1310720
After some kernel build workloads, the percentage (mainly driven by
dentry and inode caches expanding) drops under 10%:
Total Slab Memory: 95516184
Usercopyable Memory: 8497452 8.8%
task_struct 0.2% 4000/1456000
RAW 0.3% 300/96000
RAWv6 2.1% 1408/64768
ext4_inode_cache 3.0% 1217280/39439872
dentry 11.1% 1623200/14608800
mm_struct 29.1% 73216/251264
kmalloc-8 100.0% 24576/24576
kmalloc-16 100.0% 28672/28672
kmalloc-32 100.0% 94208/94208
kmalloc-192 100.0% 96768/96768
kmalloc-128 100.0% 143360/143360
names_cache 100.0% 163840/163840
kmalloc-64 100.0% 245760/245760
kmalloc-256 100.0% 339968/339968
kmalloc-512 100.0% 350720/350720
kmalloc-96 100.0% 563520/563520
kmalloc-8192 100.0% 655360/655360
kmalloc-1024 100.0% 794624/794624
kmalloc-4096 100.0% 819200/819200
kmalloc-2048 100.0% 1257472/1257472
Signed-off-by: David Windsor <dave@nullcore.net>
[kees: adjust commit log, split out a few extra kmalloc hunks]
[kees: add field names to function declarations]
[kees: add attack surface reduction analysis to commit log]
Cc: Christoph Lameter <cl@linux.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
include/linux/slab.h | 27 +++++++++++++++++++++------
include/linux/slab_def.h | 3 +++
include/linux/slub_def.h | 3 +++
include/linux/stddef.h | 2 ++
mm/slab.c | 2 +-
mm/slab.h | 5 ++++-
mm/slab_common.c | 42 ++++++++++++++++++++++++++++++++++--------
mm/slub.c | 11 +++++++++--
8 files changed, 77 insertions(+), 18 deletions(-)
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 41473df6dfb0..8b6cb384f8b6 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -126,9 +126,13 @@ struct mem_cgroup;
void __init kmem_cache_init(void);
bool slab_is_available(void);
-struct kmem_cache *kmem_cache_create(const char *, size_t, size_t,
- unsigned long,
- void (*)(void *));
+struct kmem_cache *kmem_cache_create(const char *name, size_t size,
+ size_t align, unsigned long flags,
+ void (*ctor)(void *));
+struct kmem_cache *kmem_cache_create_usercopy(const char *name,
+ size_t size, size_t align, unsigned long flags,
+ size_t useroffset, size_t usersize,
+ void (*ctor)(void *));
void kmem_cache_destroy(struct kmem_cache *);
int kmem_cache_shrink(struct kmem_cache *);
@@ -144,9 +148,20 @@ void memcg_destroy_kmem_caches(struct mem_cgroup *);
* f.e. add ____cacheline_aligned_in_smp to the struct declaration
* then the objects will be properly aligned in SMP configurations.
*/
-#define KMEM_CACHE(__struct, __flags) kmem_cache_create(#__struct,\
- sizeof(struct __struct), __alignof__(struct __struct),\
- (__flags), NULL)
+#define KMEM_CACHE(__struct, __flags) \
+ kmem_cache_create(#__struct, sizeof(struct __struct), \
+ __alignof__(struct __struct), (__flags), NULL)
+
+/*
+ * To whitelist a single field for copying to/from usercopy, use this
+ * macro instead for KMEM_CACHE() above.
+ */
+#define KMEM_CACHE_USERCOPY(__struct, __flags, __field) \
+ kmem_cache_create_usercopy(#__struct, \
+ sizeof(struct __struct), \
+ __alignof__(struct __struct), (__flags), \
+ offsetof(struct __struct, __field), \
+ sizeof_field(struct __struct, __field), NULL)
/*
* Common kmalloc functions provided by all allocators
diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h
index 4ad2c5a26399..03eef0df8648 100644
--- a/include/linux/slab_def.h
+++ b/include/linux/slab_def.h
@@ -84,6 +84,9 @@ struct kmem_cache {
unsigned int *random_seq;
#endif
+ size_t useroffset; /* Usercopy region offset */
+ size_t usersize; /* Usercopy region size */
+
struct kmem_cache_node *node[MAX_NUMNODES];
};
diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h
index cc0faf3a90be..7f373a8ee155 100644
--- a/include/linux/slub_def.h
+++ b/include/linux/slub_def.h
@@ -130,6 +130,9 @@ struct kmem_cache {
struct kasan_cache kasan_info;
#endif
+ size_t useroffset; /* Usercopy region offset */
+ size_t usersize; /* Usercopy region size */
+
struct kmem_cache_node *node[MAX_NUMNODES];
};
diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index 9c61c7cda936..f00355086fb2 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -18,6 +18,8 @@ enum {
#define offsetof(TYPE, MEMBER) ((size_t)&((TYPE *)0)->MEMBER)
#endif
+#define sizeof_field(structure, field) sizeof((((structure *)0)->field))
+
/**
* offsetofend(TYPE, MEMBER)
*
diff --git a/mm/slab.c b/mm/slab.c
index 04dec48c3ed7..87b6e5e0cdaf 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1281,7 +1281,7 @@ void __init kmem_cache_init(void)
create_boot_cache(kmem_cache, "kmem_cache",
offsetof(struct kmem_cache, node) +
nr_node_ids * sizeof(struct kmem_cache_node *),
- SLAB_HWCACHE_ALIGN);
+ SLAB_HWCACHE_ALIGN, 0, 0);
list_add(&kmem_cache->list, &slab_caches);
slab_state = PARTIAL;
diff --git a/mm/slab.h b/mm/slab.h
index 6885e1192ec5..5d4b0fb6b7de 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -21,6 +21,8 @@ struct kmem_cache {
unsigned int size; /* The aligned/padded/added on size */
unsigned int align; /* Alignment as calculated */
unsigned long flags; /* Active flags on the slab */
+ size_t useroffset; /* Usercopy region offset */
+ size_t usersize; /* Usercopy region size */
const char *name; /* Slab name for sysfs */
int refcount; /* Use counter */
void (*ctor)(void *); /* Called on object slot creation */
@@ -96,7 +98,8 @@ extern int __kmem_cache_create(struct kmem_cache *, unsigned long flags);
extern struct kmem_cache *create_kmalloc_cache(const char *name, size_t size,
unsigned long flags);
extern void create_boot_cache(struct kmem_cache *, const char *name,
- size_t size, unsigned long flags);
+ size_t size, unsigned long flags, size_t useroffset,
+ size_t usersize);
int slab_unmergeable(struct kmem_cache *s);
struct kmem_cache *find_mergeable(size_t size, size_t align,
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 904a83be82de..4b1bca7c1a42 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -272,6 +272,9 @@ int slab_unmergeable(struct kmem_cache *s)
if (s->ctor)
return 1;
+ if (s->usersize)
+ return 1;
+
/*
* We may have set a slab to be unmergeable during bootstrap.
*/
@@ -357,12 +360,15 @@ unsigned long calculate_alignment(unsigned long flags,
static struct kmem_cache *create_cache(const char *name,
size_t object_size, size_t size, size_t align,
- unsigned long flags, void (*ctor)(void *),
+ unsigned long flags, size_t useroffset,
+ size_t usersize, void (*ctor)(void *),
struct mem_cgroup *memcg, struct kmem_cache *root_cache)
{
struct kmem_cache *s;
int err;
+ BUG_ON(useroffset + usersize > object_size);
+
err = -ENOMEM;
s = kmem_cache_zalloc(kmem_cache, GFP_KERNEL);
if (!s)
@@ -373,6 +379,8 @@ static struct kmem_cache *create_cache(const char *name,
s->size = size;
s->align = align;
s->ctor = ctor;
+ s->useroffset = useroffset;
+ s->usersize = usersize;
err = init_memcg_params(s, memcg, root_cache);
if (err)
@@ -397,11 +405,13 @@ static struct kmem_cache *create_cache(const char *name,
}
/*
- * kmem_cache_create - Create a cache.
+ * kmem_cache_create_usercopy - Create a cache.
* @name: A string which is used in /proc/slabinfo to identify this cache.
* @size: The size of objects to be created in this cache.
* @align: The required alignment for the objects.
* @flags: SLAB flags
+ * @useroffset: Usercopy region offset
+ * @usersize: Usercopy region size
* @ctor: A constructor for the objects.
*
* Returns a ptr to the cache on success, NULL on failure.
@@ -421,8 +431,9 @@ static struct kmem_cache *create_cache(const char *name,
* as davem.
*/
struct kmem_cache *
-kmem_cache_create(const char *name, size_t size, size_t align,
- unsigned long flags, void (*ctor)(void *))
+kmem_cache_create_usercopy(const char *name, size_t size, size_t align,
+ unsigned long flags, size_t useroffset, size_t usersize,
+ void (*ctor)(void *))
{
struct kmem_cache *s = NULL;
const char *cache_name;
@@ -453,7 +464,10 @@ kmem_cache_create(const char *name, size_t size, size_t align,
*/
flags &= CACHE_CREATE_MASK;
- s = __kmem_cache_alias(name, size, align, flags, ctor);
+ BUG_ON(!usersize && useroffset);
+ BUG_ON(size < usersize || size - usersize < useroffset);
+ if (!usersize)
+ s = __kmem_cache_alias(name, size, align, flags, ctor);
if (s)
goto out_unlock;
@@ -465,7 +479,7 @@ kmem_cache_create(const char *name, size_t size, size_t align,
s = create_cache(cache_name, size, size,
calculate_alignment(flags, align, size),
- flags, ctor, NULL, NULL);
+ flags, useroffset, usersize, ctor, NULL, NULL);
if (IS_ERR(s)) {
err = PTR_ERR(s);
kfree_const(cache_name);
@@ -491,6 +505,15 @@ kmem_cache_create(const char *name, size_t size, size_t align,
}
return s;
}
+EXPORT_SYMBOL(kmem_cache_create_usercopy);
+
+struct kmem_cache *
+kmem_cache_create(const char *name, size_t size, size_t align,
+ unsigned long flags, void (*ctor)(void *))
+{
+ return kmem_cache_create_usercopy(name, size, align, flags, 0, size,
+ ctor);
+}
EXPORT_SYMBOL(kmem_cache_create);
static void slab_caches_to_rcu_destroy_workfn(struct work_struct *work)
@@ -603,6 +626,7 @@ void memcg_create_kmem_cache(struct mem_cgroup *memcg,
s = create_cache(cache_name, root_cache->object_size,
root_cache->size, root_cache->align,
root_cache->flags & CACHE_CREATE_MASK,
+ root_cache->useroffset, root_cache->usersize,
root_cache->ctor, memcg, root_cache);
/*
* If we could not create a memcg cache, do not complain, because
@@ -870,13 +894,15 @@ bool slab_is_available(void)
#ifndef CONFIG_SLOB
/* Create a cache during boot when no slab services are available yet */
void __init create_boot_cache(struct kmem_cache *s, const char *name, size_t size,
- unsigned long flags)
+ unsigned long flags, size_t useroffset, size_t usersize)
{
int err;
s->name = name;
s->size = s->object_size = size;
s->align = calculate_alignment(flags, ARCH_KMALLOC_MINALIGN, size);
+ s->useroffset = useroffset;
+ s->usersize = usersize;
slab_init_memcg_params(s);
@@ -897,7 +923,7 @@ struct kmem_cache *__init create_kmalloc_cache(const char *name, size_t size,
if (!s)
panic("Out of memory when creating slab %s\n", name);
- create_boot_cache(s, name, size, flags);
+ create_boot_cache(s, name, size, flags, 0, size);
list_add(&s->list, &slab_caches);
memcg_link_cache(s);
s->refcount = 1;
diff --git a/mm/slub.c b/mm/slub.c
index 1d3f9835f4ea..dfed8ef99b68 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4165,7 +4165,7 @@ void __init kmem_cache_init(void)
kmem_cache = &boot_kmem_cache;
create_boot_cache(kmem_cache_node, "kmem_cache_node",
- sizeof(struct kmem_cache_node), SLAB_HWCACHE_ALIGN);
+ sizeof(struct kmem_cache_node), SLAB_HWCACHE_ALIGN, 0, 0);
register_hotmemory_notifier(&slab_memory_callback_nb);
@@ -4175,7 +4175,7 @@ void __init kmem_cache_init(void)
create_boot_cache(kmem_cache, "kmem_cache",
offsetof(struct kmem_cache, node) +
nr_node_ids * sizeof(struct kmem_cache_node *),
- SLAB_HWCACHE_ALIGN);
+ SLAB_HWCACHE_ALIGN, 0, 0);
kmem_cache = bootstrap(&boot_kmem_cache);
@@ -5045,6 +5045,12 @@ static ssize_t cache_dma_show(struct kmem_cache *s, char *buf)
SLAB_ATTR_RO(cache_dma);
#endif
+static ssize_t usersize_show(struct kmem_cache *s, char *buf)
+{
+ return sprintf(buf, "%zu\n", s->usersize);
+}
+SLAB_ATTR_RO(usersize);
+
static ssize_t destroy_by_rcu_show(struct kmem_cache *s, char *buf)
{
return sprintf(buf, "%d\n", !!(s->flags & SLAB_TYPESAFE_BY_RCU));
@@ -5419,6 +5425,7 @@ static struct attribute *slab_attrs[] = {
#ifdef CONFIG_FAILSLAB
&failslab_attr.attr,
#endif
+ &usersize_attr.attr,
NULL
};
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-08-28 23:40 +0200 |
| Subject | [PATCH v2 11/30] exofs: Define usercopy region in exofs_inode_cache slab cache |
| Message-ID | <ujzVg-2ie-41@gated-at.bofh.it> |
| In reply to | #1721983 |
From: David Windsor <dave@nullcore.net>
The exofs short symlink names, stored in struct exofs_i_info.i_data and
therefore contained in the exofs_inode_cache slab cache, need to be copied
to/from userspace.
cache object allocation:
fs/exofs/super.c:
exofs_alloc_inode(...):
...
oi = kmem_cache_alloc(exofs_inode_cachep, GFP_KERNEL);
...
return &oi->vfs_inode;
fs/exofs/namei.c:
exofs_symlink(...):
...
inode->i_link = (char *)oi->i_data;
example usage trace:
readlink_copy+0x43/0x70
vfs_readlink+0x62/0x110
SyS_readlinkat+0x100/0x130
fs/namei.c:
readlink_copy(..., link):
...
copy_to_user(..., link, len);
(inlined in vfs_readlink)
generic_readlink(dentry, ...):
struct inode *inode = d_inode(dentry);
const char *link = inode->i_link;
...
readlink_copy(..., link);
In support of usercopy hardening, this patch defines a region in the
exofs_inode_cache slab cache in which userspace copy operations are
allowed.
This region is known as the slab cache's usercopy region. Slab caches can
now check that each copy operation involving cache-managed memory falls
entirely within the slab's usercopy region.
This patch is modified from Brad Spengler/PaX Team's PAX_USERCOPY
whitelisting code in the last public patch of grsecurity/PaX based on my
understanding of the code. Changes or omissions from the original code are
mine and don't reflect the original grsecurity/PaX code.
Signed-off-by: David Windsor <dave@nullcore.net>
[kees: adjust commit log, provide usage trace]
Cc: Boaz Harrosh <ooo@electrozaur.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
fs/exofs/super.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/fs/exofs/super.c b/fs/exofs/super.c
index 819624cfc8da..e5c532875bb7 100644
--- a/fs/exofs/super.c
+++ b/fs/exofs/super.c
@@ -192,10 +192,13 @@ static void exofs_init_once(void *foo)
*/
static int init_inodecache(void)
{
- exofs_inode_cachep = kmem_cache_create("exofs_inode_cache",
+ exofs_inode_cachep = kmem_cache_create_usercopy("exofs_inode_cache",
sizeof(struct exofs_i_info), 0,
SLAB_RECLAIM_ACCOUNT | SLAB_MEM_SPREAD |
- SLAB_ACCOUNT, exofs_init_once);
+ SLAB_ACCOUNT,
+ offsetof(struct exofs_i_info, i_data),
+ sizeof_field(struct exofs_i_info, i_data),
+ exofs_init_once);
if (exofs_inode_cachep == NULL)
return -ENOMEM;
return 0;
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-08-28 23:40 +0200 |
| Subject | [PATCH v2 04/30] dcache: Define usercopy region in dentry_cache slab cache |
| Message-ID | <ujzVh-2ie-45@gated-at.bofh.it> |
| In reply to | #1721983 |
From: David Windsor <dave@nullcore.net>
When a dentry name is short enough, it can be stored directly in the
dentry itself (instead in a separate kmalloc allocation). These dentry
short names, stored in struct dentry.d_iname and therefore contained in
the dentry_cache slab cache, need to be coped to userspace.
cache object allocation:
fs/dcache.c:
__d_alloc(...):
...
dentry = kmem_cache_alloc(dentry_cache, ...);
...
dentry->d_name.name = dentry->d_iname;
example usage trace:
filldir+0xb0/0x140
dcache_readdir+0x82/0x170
iterate_dir+0x142/0x1b0
SyS_getdents+0xb5/0x160
fs/readdir.c:
(called via ctx.actor by dir_emit)
filldir(..., const char *name, ...):
...
copy_to_user(..., name, namlen)
fs/libfs.c:
dcache_readdir(...):
...
next = next_positive(dentry, p, 1)
...
dir_emit(..., next->d_name.name, ...)
In support of usercopy hardening, this patch defines a region in the
dentry_cache slab cache in which userspace copy operations are allowed.
This region is known as the slab cache's usercopy region. Slab caches can
now check that each copy operation involving cache-managed memory falls
entirely within the slab's usercopy region.
This patch is modified from Brad Spengler/PaX Team's PAX_USERCOPY
whitelisting code in the last public patch of grsecurity/PaX based on my
understanding of the code. Changes or omissions from the original code are
mine and don't reflect the original grsecurity/PaX code.
Signed-off-by: David Windsor <dave@nullcore.net>
[kees: adjust hunks for kmalloc-specific things moved later]
[kees: adjust commit log, provide usage trace]
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
fs/dcache.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/dcache.c b/fs/dcache.c
index f90141387f01..5f5e7c1fcf4b 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -3603,8 +3603,9 @@ static void __init dcache_init(void)
* but it is probably not worth it because of the cache nature
* of the dcache.
*/
- dentry_cache = KMEM_CACHE(dentry,
- SLAB_RECLAIM_ACCOUNT|SLAB_PANIC|SLAB_MEM_SPREAD|SLAB_ACCOUNT);
+ dentry_cache = KMEM_CACHE_USERCOPY(dentry,
+ SLAB_RECLAIM_ACCOUNT|SLAB_PANIC|SLAB_MEM_SPREAD|SLAB_ACCOUNT,
+ d_iname);
/* Hash may have been set up in dcache_init_early */
if (!hashdist)
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-08-28 23:40 +0200 |
| Subject | [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache |
| Message-ID | <ujzVg-2ie-37@gated-at.bofh.it> |
| In reply to | #1721983 |
From: David Windsor <dave@nullcore.net>
The XFS inline inode data, stored in struct xfs_inode_t field
i_df.if_u2.if_inline_data and therefore contained in the xfs_inode slab
cache, needs to be copied to/from userspace.
cache object allocation:
fs/xfs/xfs_icache.c:
xfs_inode_alloc(...):
...
ip = kmem_zone_alloc(xfs_inode_zone, KM_SLEEP);
fs/xfs/libxfs/xfs_inode_fork.c:
xfs_init_local_fork(...):
...
if (mem_size <= sizeof(ifp->if_u2.if_inline_data))
ifp->if_u1.if_data = ifp->if_u2.if_inline_data;
...
fs/xfs/xfs_symlink.c:
xfs_symlink(...):
...
xfs_init_local_fork(ip, XFS_DATA_FORK, target_path, pathlen);
example usage trace:
readlink_copy+0x43/0x70
vfs_readlink+0x62/0x110
SyS_readlinkat+0x100/0x130
fs/xfs/xfs_iops.c:
(via inode->i_op->get_link)
xfs_vn_get_link_inline(...):
...
return XFS_I(inode)->i_df.if_u1.if_data;
fs/namei.c:
readlink_copy(..., link):
...
copy_to_user(..., link, len);
generic_readlink(dentry, ...):
struct inode *inode = d_inode(dentry);
const char *link = inode->i_link;
...
if (!link) {
link = inode->i_op->get_link(dentry, inode, &done);
...
readlink_copy(..., link);
In support of usercopy hardening, this patch defines a region in the
xfs_inode slab cache in which userspace copy operations are allowed.
This region is known as the slab cache's usercopy region. Slab caches can
now check that each copy operation involving cache-managed memory falls
entirely within the slab's usercopy region.
This patch is modified from Brad Spengler/PaX Team's PAX_USERCOPY
whitelisting code in the last public patch of grsecurity/PaX based on my
understanding of the code. Changes or omissions from the original code are
mine and don't reflect the original grsecurity/PaX code.
Signed-off-by: David Windsor <dave@nullcore.net>
[kees: adjust commit log, provide usage trace]
Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-xfs@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
fs/xfs/kmem.h | 10 ++++++++++
fs/xfs/xfs_super.c | 7 +++++--
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/kmem.h b/fs/xfs/kmem.h
index 4d85992d75b2..08358f38dee6 100644
--- a/fs/xfs/kmem.h
+++ b/fs/xfs/kmem.h
@@ -110,6 +110,16 @@ kmem_zone_init_flags(int size, char *zone_name, unsigned long flags,
return kmem_cache_create(zone_name, size, 0, flags, construct);
}
+static inline kmem_zone_t *
+kmem_zone_init_flags_usercopy(int size, char *zone_name, unsigned long flags,
+ size_t useroffset, size_t usersize,
+ void (*construct)(void *))
+{
+ return kmem_cache_create_usercopy(zone_name, size, 0, flags,
+ useroffset, usersize, construct);
+}
+
+
static inline void
kmem_zone_free(kmem_zone_t *zone, void *ptr)
{
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 38aaacdbb8b3..6ca428c6f943 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1829,9 +1829,12 @@ xfs_init_zones(void)
goto out_destroy_efd_zone;
xfs_inode_zone =
- kmem_zone_init_flags(sizeof(xfs_inode_t), "xfs_inode",
+ kmem_zone_init_flags_usercopy(sizeof(xfs_inode_t), "xfs_inode",
KM_ZONE_HWALIGN | KM_ZONE_RECLAIM | KM_ZONE_SPREAD |
- KM_ZONE_ACCOUNT, xfs_fs_inode_init_once);
+ KM_ZONE_ACCOUNT,
+ offsetof(xfs_inode_t, i_df.if_u2.if_inline_data),
+ sizeof_field(xfs_inode_t, i_df.if_u2.if_inline_data),
+ xfs_fs_inode_init_once);
if (!xfs_inode_zone)
goto out_destroy_efi_zone;
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-08-29 00:00 +0200 |
| Subject | Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache |
| Message-ID | <ujAeB-2oT-11@gated-at.bofh.it> |
| In reply to | #1721998 |
On Mon, Aug 28, 2017 at 2:49 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote: > On Mon, Aug 28, 2017 at 02:34:56PM -0700, Kees Cook wrote: >> From: David Windsor <dave@nullcore.net> >> >> The XFS inline inode data, stored in struct xfs_inode_t field >> i_df.if_u2.if_inline_data and therefore contained in the xfs_inode slab >> cache, needs to be copied to/from userspace. >> >> cache object allocation: >> fs/xfs/xfs_icache.c: >> xfs_inode_alloc(...): >> ... >> ip = kmem_zone_alloc(xfs_inode_zone, KM_SLEEP); >> >> fs/xfs/libxfs/xfs_inode_fork.c: >> xfs_init_local_fork(...): >> ... >> if (mem_size <= sizeof(ifp->if_u2.if_inline_data)) >> ifp->if_u1.if_data = ifp->if_u2.if_inline_data; > > Hmm, what happens when mem_size > sizeof(if_inline_data)? A slab object > will be allocated for ifp->if_u1.if_data which can then be used for > readlink in the same manner as the example usage trace below. Does > that allocated object have a need for a usercopy annotation like > the one we're adding for if_inline_data? Or is that already covered > elsewhere? Yeah, the xfs helper kmem_alloc() is used in the other case, which ultimately boils down to a call to kmalloc(), which is entirely whitelisted by an earlier patch in the series: https://lkml.org/lkml/2017/8/28/1026 (It's possible that at some future time we can start segregating kernel-only kmallocs from usercopy-able kmallocs, but for now, there are no plans for this.) -Kees -- Kees Cook Pixel Security
[toc] | [prev] | [next] | [standalone]
| From | "Darrick J. Wong" <darrick.wong@oracle.com> |
|---|---|
| Date | 2017-08-29 06:50 +0200 |
| Subject | Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache |
| Message-ID | <ujGDn-6rm-1@gated-at.bofh.it> |
| In reply to | #1722015 |
On Mon, Aug 28, 2017 at 02:57:14PM -0700, Kees Cook wrote: > On Mon, Aug 28, 2017 at 2:49 PM, Darrick J. Wong > <darrick.wong@oracle.com> wrote: > > On Mon, Aug 28, 2017 at 02:34:56PM -0700, Kees Cook wrote: > >> From: David Windsor <dave@nullcore.net> > >> > >> The XFS inline inode data, stored in struct xfs_inode_t field > >> i_df.if_u2.if_inline_data and therefore contained in the xfs_inode slab > >> cache, needs to be copied to/from userspace. > >> > >> cache object allocation: > >> fs/xfs/xfs_icache.c: > >> xfs_inode_alloc(...): > >> ... > >> ip = kmem_zone_alloc(xfs_inode_zone, KM_SLEEP); > >> > >> fs/xfs/libxfs/xfs_inode_fork.c: > >> xfs_init_local_fork(...): > >> ... > >> if (mem_size <= sizeof(ifp->if_u2.if_inline_data)) > >> ifp->if_u1.if_data = ifp->if_u2.if_inline_data; > > > > Hmm, what happens when mem_size > sizeof(if_inline_data)? A slab object > > will be allocated for ifp->if_u1.if_data which can then be used for > > readlink in the same manner as the example usage trace below. Does > > that allocated object have a need for a usercopy annotation like > > the one we're adding for if_inline_data? Or is that already covered > > elsewhere? > > Yeah, the xfs helper kmem_alloc() is used in the other case, which > ultimately boils down to a call to kmalloc(), which is entirely > whitelisted by an earlier patch in the series: > > https://lkml.org/lkml/2017/8/28/1026 Ah. It would've been helpful to have the first three patches cc'd to the xfs list. So basically this series establishes the ability to set regions within a slab object into which copy_to_user can copy memory contents, and vice versa. Have you seen any runtime performance impact? The overhead looks like it ought to be minimal. > (It's possible that at some future time we can start segregating > kernel-only kmallocs from usercopy-able kmallocs, but for now, there > are no plans for this.) A pity. It would be interesting to create no-usercopy versions of the kmalloc-* slabs and see how much of XFS' memory consumption never touches userspace buffers. :) --D > > -Kees > > -- > Kees Cook > Pixel Security > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-08-29 20:50 +0200 |
| Subject | Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache |
| Message-ID | <ujTKh-6eS-1@gated-at.bofh.it> |
| In reply to | #1722136 |
On Mon, Aug 28, 2017 at 9:47 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote: > On Mon, Aug 28, 2017 at 02:57:14PM -0700, Kees Cook wrote: >> On Mon, Aug 28, 2017 at 2:49 PM, Darrick J. Wong >> <darrick.wong@oracle.com> wrote: >> > On Mon, Aug 28, 2017 at 02:34:56PM -0700, Kees Cook wrote: >> >> From: David Windsor <dave@nullcore.net> >> >> >> >> The XFS inline inode data, stored in struct xfs_inode_t field >> >> i_df.if_u2.if_inline_data and therefore contained in the xfs_inode slab >> >> cache, needs to be copied to/from userspace. >> >> >> >> cache object allocation: >> >> fs/xfs/xfs_icache.c: >> >> xfs_inode_alloc(...): >> >> ... >> >> ip = kmem_zone_alloc(xfs_inode_zone, KM_SLEEP); >> >> >> >> fs/xfs/libxfs/xfs_inode_fork.c: >> >> xfs_init_local_fork(...): >> >> ... >> >> if (mem_size <= sizeof(ifp->if_u2.if_inline_data)) >> >> ifp->if_u1.if_data = ifp->if_u2.if_inline_data; >> > >> > Hmm, what happens when mem_size > sizeof(if_inline_data)? A slab object >> > will be allocated for ifp->if_u1.if_data which can then be used for >> > readlink in the same manner as the example usage trace below. Does >> > that allocated object have a need for a usercopy annotation like >> > the one we're adding for if_inline_data? Or is that already covered >> > elsewhere? >> >> Yeah, the xfs helper kmem_alloc() is used in the other case, which >> ultimately boils down to a call to kmalloc(), which is entirely >> whitelisted by an earlier patch in the series: >> >> https://lkml.org/lkml/2017/8/28/1026 > > Ah. It would've been helpful to have the first three patches cc'd to > the xfs list. So basically this series establishes the ability to set I went back and forth on that, and given all the things it touched, it seemed like too large a CC list. :) I can explicitly add the xfs list to the first three for any future versions. > regions within a slab object into which copy_to_user can copy memory > contents, and vice versa. Have you seen any runtime performance impact? > The overhead looks like it ought to be minimal. Under CONFIG_HARDENED_USERCOPY, there's no difference in performance between the earlier bounds checking (of the whole slab object) vs the new bounds checking (of the useroffset/usersize portion of the slab object). Perf difference of CONFIG_HARDENED_USERCOPY itself has proven hard to measure, which likely means it's very minimal. >> (It's possible that at some future time we can start segregating >> kernel-only kmallocs from usercopy-able kmallocs, but for now, there >> are no plans for this.) > > A pity. It would be interesting to create no-usercopy versions of the > kmalloc-* slabs and see how much of XFS' memory consumption never > touches userspace buffers. :) There are plans for building either a new helper (kmalloc_usercopy()) or adding a new flag (GFP_USERCOPY), but I haven't had time yet to come back around to it. I wanted to land this step first, and we could then move forward on the rest in future. -Kees -- Kees Cook Pixel Security
[toc] | [prev] | [next] | [standalone]
| From | "Darrick J. Wong" <darrick.wong@oracle.com> |
|---|---|
| Date | 2017-08-29 21:10 +0200 |
| Subject | Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache |
| Message-ID | <ujU3D-6AB-13@gated-at.bofh.it> |
| In reply to | #1722691 |
On Tue, Aug 29, 2017 at 11:48:49AM -0700, Kees Cook wrote: > On Mon, Aug 28, 2017 at 9:47 PM, Darrick J. Wong > <darrick.wong@oracle.com> wrote: > > On Mon, Aug 28, 2017 at 02:57:14PM -0700, Kees Cook wrote: > >> On Mon, Aug 28, 2017 at 2:49 PM, Darrick J. Wong > >> <darrick.wong@oracle.com> wrote: > >> > On Mon, Aug 28, 2017 at 02:34:56PM -0700, Kees Cook wrote: > >> >> From: David Windsor <dave@nullcore.net> > >> >> > >> >> The XFS inline inode data, stored in struct xfs_inode_t field > >> >> i_df.if_u2.if_inline_data and therefore contained in the xfs_inode slab > >> >> cache, needs to be copied to/from userspace. > >> >> > >> >> cache object allocation: > >> >> fs/xfs/xfs_icache.c: > >> >> xfs_inode_alloc(...): > >> >> ... > >> >> ip = kmem_zone_alloc(xfs_inode_zone, KM_SLEEP); > >> >> > >> >> fs/xfs/libxfs/xfs_inode_fork.c: > >> >> xfs_init_local_fork(...): > >> >> ... > >> >> if (mem_size <= sizeof(ifp->if_u2.if_inline_data)) > >> >> ifp->if_u1.if_data = ifp->if_u2.if_inline_data; > >> > > >> > Hmm, what happens when mem_size > sizeof(if_inline_data)? A slab object > >> > will be allocated for ifp->if_u1.if_data which can then be used for > >> > readlink in the same manner as the example usage trace below. Does > >> > that allocated object have a need for a usercopy annotation like > >> > the one we're adding for if_inline_data? Or is that already covered > >> > elsewhere? > >> > >> Yeah, the xfs helper kmem_alloc() is used in the other case, which > >> ultimately boils down to a call to kmalloc(), which is entirely > >> whitelisted by an earlier patch in the series: > >> > >> https://lkml.org/lkml/2017/8/28/1026 > > > > Ah. It would've been helpful to have the first three patches cc'd to > > the xfs list. So basically this series establishes the ability to set > > I went back and forth on that, and given all the things it touched, it > seemed like too large a CC list. :) I can explicitly add the xfs list > to the first three for any future versions. > > > regions within a slab object into which copy_to_user can copy memory > > contents, and vice versa. Have you seen any runtime performance impact? > > The overhead looks like it ought to be minimal. > > Under CONFIG_HARDENED_USERCOPY, there's no difference in performance > between the earlier bounds checking (of the whole slab object) vs the > new bounds checking (of the useroffset/usersize portion of the slab > object). Perf difference of CONFIG_HARDENED_USERCOPY itself has proven > hard to measure, which likely means it's very minimal. > > >> (It's possible that at some future time we can start segregating > >> kernel-only kmallocs from usercopy-able kmallocs, but for now, there > >> are no plans for this.) > > > > A pity. It would be interesting to create no-usercopy versions of the > > kmalloc-* slabs and see how much of XFS' memory consumption never > > touches userspace buffers. :) > > There are plans for building either a new helper (kmalloc_usercopy()) > or adding a new flag (GFP_USERCOPY), but I haven't had time yet to > come back around to it. I wanted to land this step first, and we could > then move forward on the rest in future. Heh, fair enough. For the XFS bits, Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > > -Kees > > -- > Kees Cook > Pixel Security > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
[toc] | [prev] | [next] | [standalone]
| From | Dave Chinner <david@fromorbit.com> |
|---|---|
| Date | 2017-08-30 00:20 +0200 |
| Subject | Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache |
| Message-ID | <ujX1w-8qU-15@gated-at.bofh.it> |
| In reply to | #1722691 |
On Tue, Aug 29, 2017 at 11:48:49AM -0700, Kees Cook wrote: > On Mon, Aug 28, 2017 at 9:47 PM, Darrick J. Wong > <darrick.wong@oracle.com> wrote: > > On Mon, Aug 28, 2017 at 02:57:14PM -0700, Kees Cook wrote: > >> On Mon, Aug 28, 2017 at 2:49 PM, Darrick J. Wong > >> <darrick.wong@oracle.com> wrote: > >> > On Mon, Aug 28, 2017 at 02:34:56PM -0700, Kees Cook wrote: > >> >> From: David Windsor <dave@nullcore.net> > >> >> > >> >> The XFS inline inode data, stored in struct xfs_inode_t field > >> >> i_df.if_u2.if_inline_data and therefore contained in the xfs_inode slab > >> >> cache, needs to be copied to/from userspace. > >> >> > >> >> cache object allocation: > >> >> fs/xfs/xfs_icache.c: > >> >> xfs_inode_alloc(...): > >> >> ... > >> >> ip = kmem_zone_alloc(xfs_inode_zone, KM_SLEEP); > >> >> > >> >> fs/xfs/libxfs/xfs_inode_fork.c: > >> >> xfs_init_local_fork(...): > >> >> ... > >> >> if (mem_size <= sizeof(ifp->if_u2.if_inline_data)) > >> >> ifp->if_u1.if_data = ifp->if_u2.if_inline_data; > >> > > >> > Hmm, what happens when mem_size > sizeof(if_inline_data)? A slab object > >> > will be allocated for ifp->if_u1.if_data which can then be used for > >> > readlink in the same manner as the example usage trace below. Does > >> > that allocated object have a need for a usercopy annotation like > >> > the one we're adding for if_inline_data? Or is that already covered > >> > elsewhere? > >> > >> Yeah, the xfs helper kmem_alloc() is used in the other case, which > >> ultimately boils down to a call to kmalloc(), which is entirely > >> whitelisted by an earlier patch in the series: > >> > >> https://lkml.org/lkml/2017/8/28/1026 > > > > Ah. It would've been helpful to have the first three patches cc'd to > > the xfs list. So basically this series establishes the ability to set > > I went back and forth on that, and given all the things it touched, it > seemed like too large a CC list. :) I can explicitly add the xfs list > to the first three for any future versions. If you are touching multiple filesystems, you really should cc the entire patchset to linux-fsdevel, similar to how you sent the entire patchset to lkml. That way the entire series will end up on a list that almost all fs developers read. LKML is not a list you can rely on all filesystem developers reading (or developers in any other subsystem, for that matter)... Cheers, Dave. -- Dave Chinner david@fromorbit.com
[toc] | [prev] | [next] | [standalone]
Page 1 of 3 [1] 2 3 Next page →
Back to top | Article view | linux.kernel
csiph-web