linux-kernel - Re: nfsd changes for 2.6.37

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: <201010270921.37032.arnd@arndb.de>
Date:	Wed, 27 Oct 2010 09:21:36 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	"J. Bruce Fields" <bfields@...ldses.org>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Bryan Schumaker <bjschuma@...app.com>,
	linux-nfs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: nfsd changes for 2.6.37

On Wednesday 27 October 2010 00:11:56 J. Bruce Fields wrote:
> > BUG: sleeping function called from invalid context at mm/slab.c:3101
> > in_atomic(): 1, irqs_disabled(): 0, pid: 4345, name: lease_tests
> > 1 lock held by lease_tests/4345:
> >  #0:  (file_lock_lock){+.+.+.}, at: [<ffffffff81128be5>] lock_flocks+0x15/0x20
> > Pid: 4345, comm: lease_tests Not tainted 2.6.36-05858-gbd5e20b #1028
> > Call Trace:
> >  [<ffffffff8103141d>] __might_sleep+0x10d/0x140
> >  [<ffffffff810e3ad3>] kmem_cache_alloc+0x1f3/0x230
> >  [<ffffffff8112a4d2>] generic_setlease+0x112/0x2c0
> >  [<ffffffff8112a6b5>] __vfs_setlease+0x35/0x40
> >  [<ffffffff8112acfe>] fcntl_setlease+0xce/0x180
> >  [<ffffffff810f7c2e>] sys_fcntl+0x2fe/0x630
> >  [<ffffffff81961999>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> >  [<ffffffff81002658>] system_call_fastpath+0x16/0x1b
> > 
> > I'm testing a patch.
> 

Thanks for the report!

> @@ -1524,8 +1528,6 @@ int fcntl_setlease(unsigned int fd, struct file *filp, long arg)
>         if (error)
>                 return error;
>  
> -       lock_flocks();
> -
>         error = __vfs_setlease(filp, arg, &flp);
>         if (error || arg == F_UNLCK)
>                 goto out_unlock;
> @@ -1541,7 +1543,6 @@ int fcntl_setlease(unsigned int fd, struct file *filp, long arg)
>  
>         error = __f_setown(filp, task_pid(current), PIDTYPE_PID, 0);
>  out_unlock:
> -       unlock_flocks();
>         return error;
>  }

If you don't hold lock_flocks throughout fcntl_setlease, the flp variable
points to a flock that may get modified by another thread and you call
time_out_leases() without holding lock_flocks, which it requires.

The two alternatives I can see are to either use GFP_ATOMIC or to
take the lock inside of generic_setlease and drop it outside.
Neither of the two sounds particularly appealing.

	Arnd
--
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/