linux-kernel - Re: [patch] mm: tiny-shmem fix lor, mmap_sem vs i

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: <20080918111226.GD29968@elte.hu>
Date:	Thu, 18 Sep 2008 13:12:26 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Nick Piggin <npiggin@...e.de>, a.p.zijlstra@...llo.nl,
	linux-kernel@...r.kernel.org, mpm@...enic.com,
	Hugh Dickins <hugh@...itas.com>
Subject: Re: [patch] mm: tiny-shmem fix lor, mmap_sem vs i_mutex


* Andrew Morton <akpm@...ux-foundation.org> wrote:

> On Mon, 15 Sep 2008 00:12:31 +0200
> Nick Piggin <npiggin@...e.de> wrote:
> 
> > tiny-shmem calls do_truncate in shmem_file_setup. do_truncate takes i_mutex,
> > and shmem_file_setup is called with mmap_sem held. However i_mutex nests
> > outside mmap_sem.
> > 
> > Copy the code in shmem.c to avoid this problem.
> > 
> 
> It's a bit unfortunate (as in: arse-about) that we end up creating new
> files deep within the mmap code, but I guess we won't be changing that
> in a hurry.
> 
> 
> > ---
> > Index: linux-2.6/mm/tiny-shmem.c
> > ===================================================================
> > --- linux-2.6.orig/mm/tiny-shmem.c
> > +++ linux-2.6/mm/tiny-shmem.c
> > @@ -65,31 +65,25 @@ struct file *shmem_file_setup(char *name
> >  	if (!dentry)
> >  		goto put_memory;
> >  
> > +        error = -ENFILE;
> > +        file = get_empty_filp();
> > +        if (!file)
> > +                goto put_dentry;
> > +
> >  	error = -ENOSPC;
> >  	inode = ramfs_get_inode(root->d_sb, S_IFREG | S_IRWXUGO, 0);
> >  	if (!inode)
> > -		goto put_dentry;
> > -
> > -	d_instantiate(dentry, inode);
> > -	error = -ENFILE;
> > -	file = alloc_file(shm_mnt, dentry, FMODE_WRITE | FMODE_READ,
> > -			&ramfs_file_operations);
> > -	if (!file)
> > -		goto put_dentry;
> > -
> > -	inode->i_nlink = 0;	/* It is unlinked */
> > -
> > -	/* notify everyone as to the change of file size */
> > -	error = do_truncate(dentry, size, 0, file);
> > -	if (error < 0)
> >  		goto close_file;
> >  
> > +        d_instantiate(dentry, inode);
> > +        inode->i_size = size;
> > +        inode->i_nlink = 0;     /* It is unlinked */
> > +        init_file(file, shm_mnt, dentry, FMODE_WRITE | FMODE_READ,
> > +                        &ramfs_file_operations);
> >  	return file;
> >  
> >  close_file:
> >  	put_filp(file);
> > -	return ERR_PTR(error);
> > -
> >  put_dentry:
> >  	dput(dentry);
> >  put_memory:
> 
> That's a fairly substantial change.  Was it runtime tested?

yes, -tip testing. I queued it up in tip/out-of-tree a week ago:

 commit 20e27c7b26792dbd9af0543c4bc86b5de5653a89
 Author:     Nick Piggin <npiggin@...e.de>
 AuthorDate: Wed Sep 10 17:12:45 2008 +0200
 Commit:     Ingo Molnar <mingo@...e.hu>
 CommitDate: Thu Sep 11 09:13:36 2008 +0200

    mm: fix tiny-shmem circular locking

in 7 days that's about 7000 random bootups, 20% of which had TINY_SHMEM 
enabled, half 32-bit, half 64-bit x86. It did not blow up in any way 
that would have prevented the kernel from building its next random 
version from within itself and it did not produce any kernel messages 
with various random kernel debug, compile and boot options.

So i think it's a candidate for v2.6.27.

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/