Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 9 Jun 2016 19:37:14 -0400
From: Rik van Riel <riel@...hat.com>
To: Kees Cook <keescook@...omium.org>
Cc: kernel-hardening@...ts.openwall.com,
        Brad Spengler
 <spender@...ecurity.net>,
        PaX Team <pageexec@...email.hu>,
        Casey Schaufler
 <casey.schaufler@...el.com>,
        Christoph Lameter <cl@...ux.com>, Pekka Enberg
 <penberg@...nel.org>,
        David Rientjes <rientjes@...gle.com>,
        Joonsoo Kim
 <iamjoonsoo.kim@....com>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH v2 2/4] usercopy: avoid direct copying to userspace

On Wed,  8 Jun 2016 14:11:40 -0700
Kees Cook <keescook@...omium.org> wrote:

> Some non-whitelisted heap memory has small areas that need to be copied
> to userspace. For these cases, explicitly copy the needed contents out
> to stack first before sending to userspace. This lets their respective
> caches remain un-whitelisted (i.e. no SLAB_USERCOPY), since the bulk of
> their contents should not be exposed to userspace.
> 
> These changes, based on code by Brad Spengler and PaX Team, were extracted
> from grsecurity/PaX on a case-by-case basis as I ran into errors during
> testing of CONFIG_HARDENED_USERCOPY_WHITELIST:

You will want this bit as well. It is an adaptation, with
a slight change after digging through XFS code for an hour
and a half or so, of code originally from grsecurity.

With this change, my system boots a usercopy kernel without
any visible issues.

---8<---

Subject: mm,xfs: bounce buffer the file name in xfs_dir2_sf_getdents

"Short form" directories in XFS have the directory content inside the
in-memory inode, or other kernel memory. The directory contents can be
in the same slab object as other, more sensitive, contents.

Instead of marking the xfs_inode slab accessible to copy_from_user and
copy_to_user, bounce buffer the file name when doing getdents on a short
form directory.

This only affects short form directories, which will have a very small
number of entries. Large directories use different code.

Adapted from the grsecurity patch set. Thanks go out to pipacs and spender.

Signed-off-by: Rik van Riel <riel@...hat.com>
---
 fs/xfs/xfs_dir2_readdir.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
index f44f79996978..bc6c78cbe4c6 100644
--- a/fs/xfs/xfs_dir2_readdir.c
+++ b/fs/xfs/xfs_dir2_readdir.c
@@ -127,6 +127,7 @@ xfs_dir2_sf_getdents(
 	 */
 	sfep = xfs_dir2_sf_firstentry(sfp);
 	for (i = 0; i < sfp->count; i++) {
+		char name[sfep->namelen];
 		__uint8_t filetype;
 
 		off = xfs_dir2_db_off_to_dataptr(geo, geo->datablk,
@@ -140,7 +141,14 @@ xfs_dir2_sf_getdents(
 		ino = dp->d_ops->sf_get_ino(sfp, sfep);
 		filetype = dp->d_ops->sf_get_ftype(sfep);
 		ctx->pos = off & 0x7fffffff;
-		if (!dir_emit(ctx, (char *)sfep->name, sfep->namelen, ino,
+		/*
+		 * Short form directories have the file name stored in
+		 * memory that is not directly accessible to copy_to_user.
+		 * Bounce buffer the name, instead of potentially making
+		 * the other data accessible.
+		 */
+		memcpy(name, sfep->name, sfep->namelen);
+		if (!dir_emit(ctx, name, sfep->namelen, ino,
 			    xfs_dir3_get_dtype(dp->i_mount, filetype)))
 			return 0;
 		sfep = dp->d_ops->sf_nextentry(sfp, sfep);

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.