Re: [patch] mm: tiny-shmem fix lor, mmap_sem vs i_mutex

[Ingo Molnar <mingo@elte.hu>]

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

> On Mon, 15 Sep 2008 00:12:31 +0200
> Nick Piggin <npiggin@suse.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@suse.de>
 AuthorDate: Wed Sep 10 17:12:45 2008 +0200
 Commit:     Ingo Molnar <mingo@elte.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