Re: fs: WARNING in locks_free_lock_context()

2016-02-03 Thread William Dauchy 
On Wed, Feb 3, 2016 at 7:26 PM, Jeff Layton  wrote:
> Yes...this commit in mainline fixes it:

Thanks Jeff, I missed it.

-- 
William

Re: fs: WARNING in locks_free_lock_context()

2016-02-03 Thread Jeff Layton 
On Wed, 3 Feb 2016 19:19:37 +0100
William Dauchy  wrote:

> Hello Jeff,
> 
> On Wed, Dec 23, 2015 at 2:54 PM, Jeff Layton  wrote:
> > Ooh, nice catch...and just in time for Christmas.
> >
> > filp_close does this after the fd has been detached from the file table
> > in __close_fd:
> >
> > if (likely(!(filp->f_mode & FMODE_PATH))) {
> > dnotify_flush(filp, id);
> > locks_remove_posix(filp, id);
> > }
> > fput(filp);
> >
> > ...and fcntl_setlk does this:
> >
> > /*
> >  * Attempt to detect a close/fcntl race and recover by
> >  * releasing the lock that was just acquired.
> >  */
> > /*
> >  * we need that spin_lock here - it prevents reordering between
> >  * update of i_flctx->flc_posix and check for it done in close().
> >  * rcu_read_lock() wouldn't do.
> >  */
> > spin_lock(>files->file_lock);
> > f = fcheck(fd);
> > spin_unlock(>files->file_lock);
> > if (!error && f != filp && flock.l_type != F_UNLCK) {
> > flock.l_type = F_UNLCK;
> > goto again;
> > }
> >
> > ...so in principle that should keep new locks from racing onto the list
> > just after we call filp_close. Hmm...I'll see if I can reproduce and
> > figure out how this could happen.  
> 
> Just wondering if you had the time to figure out this warning?
> 
> Thanks,

Yes...this commit in mainline fixes it:

commit 7f3697e24dc3820b10f445a4a7d914fc356012d1
Author: Jeff Layton 
Date:   Thu Jan 7 16:38:10 2016 -0500

locks: fix unlock when fcntl_setlk races with a close


...and the patch is applicable to all kernels currently in circulation.
The original bug is very old (from 2005).

-- 
Jeff Layton

Re: fs: WARNING in locks_free_lock_context()

2016-02-03 Thread William Dauchy 
Hello Jeff,

On Wed, Dec 23, 2015 at 2:54 PM, Jeff Layton  wrote:
> Ooh, nice catch...and just in time for Christmas.
>
> filp_close does this after the fd has been detached from the file table
> in __close_fd:
>
> if (likely(!(filp->f_mode & FMODE_PATH))) {
> dnotify_flush(filp, id);
> locks_remove_posix(filp, id);
> }
> fput(filp);
>
> ...and fcntl_setlk does this:
>
> /*
>  * Attempt to detect a close/fcntl race and recover by
>  * releasing the lock that was just acquired.
>  */
> /*
>  * we need that spin_lock here - it prevents reordering between
>  * update of i_flctx->flc_posix and check for it done in close().
>  * rcu_read_lock() wouldn't do.
>  */
> spin_lock(>files->file_lock);
> f = fcheck(fd);
> spin_unlock(>files->file_lock);
> if (!error && f != filp && flock.l_type != F_UNLCK) {
> flock.l_type = F_UNLCK;
> goto again;
> }
>
> ...so in principle that should keep new locks from racing onto the list
> just after we call filp_close. Hmm...I'll see if I can reproduce and
> figure out how this could happen.

Just wondering if you had the time to figure out this warning?

Thanks,
-- 
William

Re: fs: WARNING in locks_free_lock_context()

2016-02-03 Thread William Dauchy 
Hello Jeff,

On Wed, Dec 23, 2015 at 2:54 PM, Jeff Layton  wrote:
> Ooh, nice catch...and just in time for Christmas.
>
> filp_close does this after the fd has been detached from the file table
> in __close_fd:
>
> if (likely(!(filp->f_mode & FMODE_PATH))) {
> dnotify_flush(filp, id);
> locks_remove_posix(filp, id);
> }
> fput(filp);
>
> ...and fcntl_setlk does this:
>
> /*
>  * Attempt to detect a close/fcntl race and recover by
>  * releasing the lock that was just acquired.
>  */
> /*
>  * we need that spin_lock here - it prevents reordering between
>  * update of i_flctx->flc_posix and check for it done in close().
>  * rcu_read_lock() wouldn't do.
>  */
> spin_lock(>files->file_lock);
> f = fcheck(fd);
> spin_unlock(>files->file_lock);
> if (!error && f != filp && flock.l_type != F_UNLCK) {
> flock.l_type = F_UNLCK;
> goto again;
> }
>
> ...so in principle that should keep new locks from racing onto the list
> just after we call filp_close. Hmm...I'll see if I can reproduce and
> figure out how this could happen.

Just wondering if you had the time to figure out this warning?

Thanks,
-- 
William

Re: fs: WARNING in locks_free_lock_context()

2016-02-03 Thread Jeff Layton 
On Wed, 3 Feb 2016 19:19:37 +0100
William Dauchy  wrote:

> Hello Jeff,
> 
> On Wed, Dec 23, 2015 at 2:54 PM, Jeff Layton  wrote:
> > Ooh, nice catch...and just in time for Christmas.
> >
> > filp_close does this after the fd has been detached from the file table
> > in __close_fd:
> >
> > if (likely(!(filp->f_mode & FMODE_PATH))) {
> > dnotify_flush(filp, id);
> > locks_remove_posix(filp, id);
> > }
> > fput(filp);
> >
> > ...and fcntl_setlk does this:
> >
> > /*
> >  * Attempt to detect a close/fcntl race and recover by
> >  * releasing the lock that was just acquired.
> >  */
> > /*
> >  * we need that spin_lock here - it prevents reordering between
> >  * update of i_flctx->flc_posix and check for it done in close().
> >  * rcu_read_lock() wouldn't do.
> >  */
> > spin_lock(>files->file_lock);
> > f = fcheck(fd);
> > spin_unlock(>files->file_lock);
> > if (!error && f != filp && flock.l_type != F_UNLCK) {
> > flock.l_type = F_UNLCK;
> > goto again;
> > }
> >
> > ...so in principle that should keep new locks from racing onto the list
> > just after we call filp_close. Hmm...I'll see if I can reproduce and
> > figure out how this could happen.  
> 
> Just wondering if you had the time to figure out this warning?
> 
> Thanks,

Yes...this commit in mainline fixes it:

commit 7f3697e24dc3820b10f445a4a7d914fc356012d1
Author: Jeff Layton 
Date:   Thu Jan 7 16:38:10 2016 -0500

locks: fix unlock when fcntl_setlk races with a close


...and the patch is applicable to all kernels currently in circulation.
The original bug is very old (from 2005).

-- 
Jeff Layton

Re: fs: WARNING in locks_free_lock_context()

2016-02-03 Thread William Dauchy 
On Wed, Feb 3, 2016 at 7:26 PM, Jeff Layton  wrote:
> Yes...this commit in mainline fixes it:

Thanks Jeff, I missed it.

-- 
William

Re: fs: WARNING in locks_free_lock_context()

2015-12-23 Thread Jeff Layton 
On Wed, 23 Dec 2015 11:37:39 +0100
Dmitry Vyukov  wrote:

> Hello,
> 
> The following program triggers
> WARN_ON_ONCE(!list_empty(>flc_posix)) warning in
> locks_free_lock_context (run it in a loop):
> 
> // autogenerated by syzkaller (http://github.com/google/syzkaller)
> #include 
> #include 
> #include 
> #include 
> #include 
> 
> #ifndef SYS_memfd_create
> #define SYS_memfd_create 319
> #endif
> 
> long r[15];
> long done[14];
> 
> void *thr(void *arg)
> {
> if (rand()%2)
> usleep(100);
> 
> switch ((long)arg) {
> case 0:
> r[0] = syscall(SYS_mmap, 0x2000ul, 0x5000ul,
> 0x3ul, 0x32ul, 0xul, 0x0ul);
> break;
> case 1:
> memcpy((void*)0x2c49,
> "\xb6\x70\x70\x70\x31\x73\x65\x63\x75\x72\x69\x74\x79\x9e\x00", 15);
> r[2] = syscall(SYS_memfd_create, 0x2c49ul, 0x3ul,
> 0, 0, 0, 0);
> break;
> case 2:
> r[3] = syscall(SYS_socketpair, 0x1ul, 0x1ul, 0x0ul,
> 0x20001000ul, 0, 0);
> if (r[3] != -1)
> r[4] = *(uint32_t*)0x20001000;
> if (r[3] != -1)
> r[5] = *(uint32_t*)0x20001004;
> break;
> case 3:
> *(uint16_t*)0x2000 = (uint16_t)0x0;
> *(uint16_t*)0x2002 = (uint16_t)0x1;
> *(uint64_t*)0x2008 = (uint64_t)0x6;
> *(uint64_t*)0x2010 = (uint64_t)0xad;
> *(uint32_t*)0x2018 = (uint32_t)0x0;
> r[11] = syscall(SYS_fcntl, r[5], 0x7ul, 0x2000ul, 0, 0, 
> 0);
> break;
> case 4:
> r[12] = syscall(SYS_write, r[5], 0x26cbul,
> 0x1000ul, 0, 0, 0);
> break;
> case 5:
> r[13] = syscall(SYS_close, r[5], 0, 0, 0, 0, 0);
> break;
> case 6:
> r[14] = syscall(SYS_dup2, r[2], r[4], 0, 0, 0, 0);
> break;
> }
> done[(long)arg] = 1;
> return 0;
> }
> 
> int main()
> {
> long i, j;
> pthread_t th[14];
> 
> srand(time(0)+getpid());
> memset(r, -1, sizeof(r));
> for (i = 0; i < 7; i++) {
> pthread_create([i], 0, thr, (void*)i);
> for (j = 0; j < 10; j++) {
> if (done[i])
> break;
> usleep(100);
> }
> }
> for (i = 0; i < 7; i++)
> done[i] = 0;
> for (i = 0; i < 7; i++) {
> pthread_create([7+i], 0, thr, (void*)i);
> if (rand()%2)
> continue;
> for (j = 0; j < 10; j++) {
> if (done[i])
> break;
> usleep(100);
> }
> }
> usleep(100);
> return 0;
> }
> 
> 
> [ cut here ]
> WARNING: CPU: 3 PID: 1975 at fs/locks.c:241
> locks_free_lock_context+0x118/0x180()
> Modules linked in:
> CPU: 3 PID: 1975 Comm: a.out Not tainted 4.4.0-rc6+ #173
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>   880068e67bf8 82899ffd 
>  88006130af00 85e17d60 880068e67c38 812ebbb9
>  818162d8 85e17d60 00f1 8800685c2828
> Call Trace:
>  [< inline >] __dump_stack lib/dump_stack.c:15
>  [] dump_stack+0x6f/0xa2 lib/dump_stack.c:50
>  [] warn_slowpath_common+0xd9/0x140 kernel/panic.c:460
>  [] warn_slowpath_null+0x29/0x30 kernel/panic.c:493
>  [] locks_free_lock_context+0x118/0x180 fs/locks.c:241
>  [] __destroy_inode+0x1d3/0x4d0 fs/inode.c:228
>  [] destroy_inode+0x4b/0x120 fs/inode.c:253
>  [] evict+0x320/0x4f0 fs/inode.c:559
>  [< inline >] iput_final fs/inode.c:1477
>  [] iput+0x45c/0x850 fs/inode.c:1504
>  [< inline >] dentry_iput fs/dcache.c:358
>  [] __dentry_kill+0x457/0x620 fs/dcache.c:543
>  [< inline >] dentry_kill fs/dcache.c:587
>  [] dput+0x659/0x740 fs/dcache.c:796
>  [] __fput+0x42c/0x780 fs/file_table.c:226
>  [] fput+0x15/0x20 fs/file_table.c:244
>  [] task_work_run+0x16b/0x200 kernel/task_work.c:115
>  [< inline >] tracehook_notify_resume include/linux/tracehook.h:191
>  [] exit_to_usermode_loop+0x180/0x1a0
> arch/x86/entry/common.c:251
>  [< inline >] prepare_exit_to_usermode arch/x86/entry/common.c:282
>  [] syscall_return_slowpath+0x19f/0x210
> arch/x86/entry/common.c:344
>  [] int_ret_from_sys_call+0x25/0x9f
> arch/x86/entry/entry_64.S:281
> ---[ end trace 2dde0624dd974a19 ]---
> 
> 
> On commit 4ef7675344d687a0ef5b0d7c0cee12da005870c0 (Dec 20).

Ooh, nice catch...and just in time for Christmas.

filp_close does this after the fd has been detached from the file table
in __close_fd:

Re: fs: WARNING in locks_free_lock_context()

2015-12-23 Thread Jeff Layton 
On Wed, 23 Dec 2015 11:37:39 +0100
Dmitry Vyukov  wrote:

> Hello,
> 
> The following program triggers
> WARN_ON_ONCE(!list_empty(>flc_posix)) warning in
> locks_free_lock_context (run it in a loop):
> 
> // autogenerated by syzkaller (http://github.com/google/syzkaller)
> #include 
> #include 
> #include 
> #include 
> #include 
> 
> #ifndef SYS_memfd_create
> #define SYS_memfd_create 319
> #endif
> 
> long r[15];
> long done[14];
> 
> void *thr(void *arg)
> {
> if (rand()%2)
> usleep(100);
> 
> switch ((long)arg) {
> case 0:
> r[0] = syscall(SYS_mmap, 0x2000ul, 0x5000ul,
> 0x3ul, 0x32ul, 0xul, 0x0ul);
> break;
> case 1:
> memcpy((void*)0x2c49,
> "\xb6\x70\x70\x70\x31\x73\x65\x63\x75\x72\x69\x74\x79\x9e\x00", 15);
> r[2] = syscall(SYS_memfd_create, 0x2c49ul, 0x3ul,
> 0, 0, 0, 0);
> break;
> case 2:
> r[3] = syscall(SYS_socketpair, 0x1ul, 0x1ul, 0x0ul,
> 0x20001000ul, 0, 0);
> if (r[3] != -1)
> r[4] = *(uint32_t*)0x20001000;
> if (r[3] != -1)
> r[5] = *(uint32_t*)0x20001004;
> break;
> case 3:
> *(uint16_t*)0x2000 = (uint16_t)0x0;
> *(uint16_t*)0x2002 = (uint16_t)0x1;
> *(uint64_t*)0x2008 = (uint64_t)0x6;
> *(uint64_t*)0x2010 = (uint64_t)0xad;
> *(uint32_t*)0x2018 = (uint32_t)0x0;
> r[11] = syscall(SYS_fcntl, r[5], 0x7ul, 0x2000ul, 0, 0, 
> 0);
> break;
> case 4:
> r[12] = syscall(SYS_write, r[5], 0x26cbul,
> 0x1000ul, 0, 0, 0);
> break;
> case 5:
> r[13] = syscall(SYS_close, r[5], 0, 0, 0, 0, 0);
> break;
> case 6:
> r[14] = syscall(SYS_dup2, r[2], r[4], 0, 0, 0, 0);
> break;
> }
> done[(long)arg] = 1;
> return 0;
> }
> 
> int main()
> {
> long i, j;
> pthread_t th[14];
> 
> srand(time(0)+getpid());
> memset(r, -1, sizeof(r));
> for (i = 0; i < 7; i++) {
> pthread_create([i], 0, thr, (void*)i);
> for (j = 0; j < 10; j++) {
> if (done[i])
> break;
> usleep(100);
> }
> }
> for (i = 0; i < 7; i++)
> done[i] = 0;
> for (i = 0; i < 7; i++) {
> pthread_create([7+i], 0, thr, (void*)i);
> if (rand()%2)
> continue;
> for (j = 0; j < 10; j++) {
> if (done[i])
> break;
> usleep(100);
> }
> }
> usleep(100);
> return 0;
> }
> 
> 
> [ cut here ]
> WARNING: CPU: 3 PID: 1975 at fs/locks.c:241
> locks_free_lock_context+0x118/0x180()
> Modules linked in:
> CPU: 3 PID: 1975 Comm: a.out Not tainted 4.4.0-rc6+ #173
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>   880068e67bf8 82899ffd 
>  88006130af00 85e17d60 880068e67c38 812ebbb9
>  818162d8 85e17d60 00f1 8800685c2828
> Call Trace:
>  [< inline >] __dump_stack lib/dump_stack.c:15
>  [] dump_stack+0x6f/0xa2 lib/dump_stack.c:50
>  [] warn_slowpath_common+0xd9/0x140 kernel/panic.c:460
>  [] warn_slowpath_null+0x29/0x30 kernel/panic.c:493
>  [] locks_free_lock_context+0x118/0x180 fs/locks.c:241
>  [] __destroy_inode+0x1d3/0x4d0 fs/inode.c:228
>  [] destroy_inode+0x4b/0x120 fs/inode.c:253
>  [] evict+0x320/0x4f0 fs/inode.c:559
>  [< inline >] iput_final fs/inode.c:1477
>  [] iput+0x45c/0x850 fs/inode.c:1504
>  [< inline >] dentry_iput fs/dcache.c:358
>  [] __dentry_kill+0x457/0x620 fs/dcache.c:543
>  [< inline >] dentry_kill fs/dcache.c:587
>  [] dput+0x659/0x740 fs/dcache.c:796
>  [] __fput+0x42c/0x780 fs/file_table.c:226
>  [] fput+0x15/0x20 fs/file_table.c:244
>  [] task_work_run+0x16b/0x200 kernel/task_work.c:115
>  [< inline >] tracehook_notify_resume include/linux/tracehook.h:191
>  [] exit_to_usermode_loop+0x180/0x1a0
> arch/x86/entry/common.c:251
>  [< inline >] prepare_exit_to_usermode arch/x86/entry/common.c:282
>  [] syscall_return_slowpath+0x19f/0x210
> arch/x86/entry/common.c:344
>  [] int_ret_from_sys_call+0x25/0x9f
> arch/x86/entry/entry_64.S:281
> ---[ end trace 2dde0624dd974a19 ]---
> 
> 
> On commit 4ef7675344d687a0ef5b0d7c0cee12da005870c0 (Dec 20).

Ooh, nice catch...and just in time for Christmas.

filp_close does this after the fd has been detached from the file