linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* Re: security, hugetlbfs: write to user memory in hugetlbfs_destroy_inode
       [not found] ` <CACT4Y+ZHqNYPE_uMrc1NwX3Rb1FXYoN47D4eJFn=T07bSQ7YEw@mail.gmail.com>
@ 2017-03-23 13:49   ` Tetsuo Handa
  2017-03-23 20:34     ` Mike Kravetz
  0 siblings, 1 reply; 2+ messages in thread
From: Tetsuo Handa @ 2017-03-23 13:49 UTC (permalink / raw)
  To: dvyukov, nyc, linux-kernel, paul, sds, eparis, james.l.morris,
	serge, keescook, anton, ccross, tony.luck, selinux,
	linux-security-module, linux-mm
  Cc: syzkaller

Dmitry Vyukov wrote:
> On Thu, Mar 23, 2017 at 2:06 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> > Hello,
> >
> > I've got the following report while running syzkaller fuzzer on
> > 093b995e3b55a0ae0670226ddfcb05bfbf0099ae. Note the preceding injected
> > kmalloc failure in inode_alloc_security, most likely it's the root
> > cause.

I don't think inode_alloc_security() failure is the root cause.
I think this is a bug in hugetlbfs or mm part.

If inode_alloc_security() fails, inode->i_security remains NULL
which was initialized to NULL at security_inode_alloc(). Thus,
security_inode_alloc() is irrelevant to this problem.

inode_init_always() returned -ENOMEM due to fault injection and

	if (unlikely(inode_init_always(sb, inode))) {
		if (inode->i_sb->s_op->destroy_inode)
			inode->i_sb->s_op->destroy_inode(inode);
		else
			kmem_cache_free(inode_cachep, inode);
		return NULL;
	}

hugetlbfs_destroy_inode() was called via inode->i_sb->s_op->destroy_inode()
when inode initialization failed

static void hugetlbfs_destroy_inode(struct inode *inode)
{
	hugetlbfs_inc_free_inodes(HUGETLBFS_SB(inode->i_sb));
	mpol_free_shared_policy(&HUGETLBFS_I(inode)->policy);
	call_rcu(&inode->i_rcu, hugetlbfs_i_callback);
}

but mpol_shared_policy_init() is called only when new_inode() succeeds.

	inode = new_inode(sb);
	if (inode) {
(...snipped...)
		info = HUGETLBFS_I(inode);
		/*
		 * The policy is initialized here even if we are creating a
		 * private inode because initialization simply creates an
		 * an empty rb tree and calls rwlock_init(), later when we
		 * call mpol_free_shared_policy() it will just return because
		 * the rb tree will still be empty.
		 */
		mpol_shared_policy_init(&info->policy, NULL);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: security, hugetlbfs: write to user memory in hugetlbfs_destroy_inode
  2017-03-23 13:49   ` security, hugetlbfs: write to user memory in hugetlbfs_destroy_inode Tetsuo Handa
@ 2017-03-23 20:34     ` Mike Kravetz
  0 siblings, 0 replies; 2+ messages in thread
From: Mike Kravetz @ 2017-03-23 20:34 UTC (permalink / raw)
  To: Tetsuo Handa, dvyukov, nyc, linux-kernel, paul, sds, eparis,
	james.l.morris, serge, keescook, anton, ccross, tony.luck,
	selinux, linux-security-module, linux-mm
  Cc: syzkaller

On 03/23/2017 06:49 AM, Tetsuo Handa wrote:
> Dmitry Vyukov wrote:
>> On Thu, Mar 23, 2017 at 2:06 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>>> Hello,
>>>
>>> I've got the following report while running syzkaller fuzzer on
>>> 093b995e3b55a0ae0670226ddfcb05bfbf0099ae. Note the preceding injected
>>> kmalloc failure in inode_alloc_security, most likely it's the root
>>> cause.
> 
> I don't think inode_alloc_security() failure is the root cause.
> I think this is a bug in hugetlbfs or mm part.
> 
> If inode_alloc_security() fails, inode->i_security remains NULL
> which was initialized to NULL at security_inode_alloc(). Thus,
> security_inode_alloc() is irrelevant to this problem.
> 
> inode_init_always() returned -ENOMEM due to fault injection and
> 
> 	if (unlikely(inode_init_always(sb, inode))) {
> 		if (inode->i_sb->s_op->destroy_inode)
> 			inode->i_sb->s_op->destroy_inode(inode);
> 		else
> 			kmem_cache_free(inode_cachep, inode);
> 		return NULL;
> 	}
> 
> hugetlbfs_destroy_inode() was called via inode->i_sb->s_op->destroy_inode()
> when inode initialization failed
> 
> static void hugetlbfs_destroy_inode(struct inode *inode)
> {
> 	hugetlbfs_inc_free_inodes(HUGETLBFS_SB(inode->i_sb));
> 	mpol_free_shared_policy(&HUGETLBFS_I(inode)->policy);
> 	call_rcu(&inode->i_rcu, hugetlbfs_i_callback);
> }
> 
> but mpol_shared_policy_init() is called only when new_inode() succeeds.
> 
> 	inode = new_inode(sb);
> 	if (inode) {
> (...snipped...)
> 		info = HUGETLBFS_I(inode);
> 		/*
> 		 * The policy is initialized here even if we are creating a
> 		 * private inode because initialization simply creates an
> 		 * an empty rb tree and calls rwlock_init(), later when we
> 		 * call mpol_free_shared_policy() it will just return because
> 		 * the rb tree will still be empty.
> 		 */
> 		mpol_shared_policy_init(&info->policy, NULL);
> 

Thank you for analysis (and Dmitry for reporting).

This certainly does look like a hugetlbfs bug.  I will put together a
patch to fix.

-- 
Mike Kravetz

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-03-23 20:34 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CACT4Y+Z1eodoxayi1qP-x05UoQ3nscXYUwA3UTN8ypOHfGJwjg@mail.gmail.com>
     [not found] ` <CACT4Y+ZHqNYPE_uMrc1NwX3Rb1FXYoN47D4eJFn=T07bSQ7YEw@mail.gmail.com>
2017-03-23 13:49   ` security, hugetlbfs: write to user memory in hugetlbfs_destroy_inode Tetsuo Handa
2017-03-23 20:34     ` Mike Kravetz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox