Re: [PATCH v2 2/4] usercopy: avoid direct copying to userspace

Commit Message

Rik van Riel June 9, 2016, 11:37 p.m. UTC

On Wed,  8 Jun 2016 14:11:40 -0700
Kees Cook <[e-mail]> 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 <[e-mail]>
---
 fs/xfs/xfs_dir2_readdir.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Patch

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);