linux-kernel - Re: [PATCH v2 06/11] don't put symlink bodies in pagecache into highmem

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160114222544.GB17997@ZenIV.linux.org.uk>
Date:	Thu, 14 Jan 2016 22:25:44 +0000
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Tomeu Vizoso <tomeu@...euvizoso.net>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Neil Brown <neilb@...e.com>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH v2 06/11] don't put symlink bodies in pagecache into
 highmem

On Thu, Jan 14, 2016 at 01:40:32PM -0800, Linus Torvalds wrote:
> On Thu, Jan 14, 2016 at 1:02 PM, Al Viro <viro@...iv.linux.org.uk> wrote:
> >
> > Arrrgh.  Try this:
> 
> Yeah, that would do it.
> 
> Al, did you check any other filesystems do this?

There's one more turd like that - shmem should've done inode_nohighmem()
a bit earlier in shmem_symlink().  The rest is OK.

> Also, I'm wondering if we should perhaps revert the "don't use
> highmem". Do we actually have examples of running out of kmaps? Do we
> care?

For one thing, we'll lose RCU ->get_link() for those.  For another... yes,
it was a nasty bug (I missed the possibility that filesystem might seed
the page cache on ->symlink() directly and use a highmem page - mea culpa),
but we can easily catch it at runtime.  We really shouldn't put highmem
pages into address_space without __GFP_HIGHMEM, and catching those in
__add_to_page_cache_locked() isn't costly.

Anyway, mm/shmem.c bit follows.  With that + NFS one we ought to be OK
wrt that class of bogosities.  I'll write the bits for
Documentation/filesystems/porting (basically, "if you preseed the pagecache
at ->symlink() time, don't put highmem pages there; page_symlink() will
take care of that, provided that inode_nohighmem() is called first") and
push the combined patch to #for-linus.

diff --git a/mm/shmem.c b/mm/shmem.c
index 5813b7f..642471b 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2469,6 +2469,7 @@ static int shmem_symlink(struct inode *dir, struct dentry *dentry, const char *s
 		inode->i_op = &shmem_short_symlink_operations;
 		inode->i_link = info->symlink;
 	} else {
+		inode_nohighmem(inode);
 		error = shmem_getpage(inode, 0, &page, SGP_WRITE, NULL);
 		if (error) {
 			iput(inode);
@@ -2476,7 +2477,6 @@ static int shmem_symlink(struct inode *dir, struct dentry *dentry, const char *s
 		}
 		inode->i_mapping->a_ops = &shmem_aops;
 		inode->i_op = &shmem_symlink_inode_operations;
-		inode_nohighmem(inode);
 		memcpy(page_address(page), symname, len);
 		SetPageUptodate(page);
 		set_page_dirty(page);