William Dauchy reported some instability after trying to apply
db2efec0caba to the v3.14 stable series kernels. The problem was that
that patch had us taking an extra reference to the filp in the NFSv4
unlock code. If we're unlocking locks on task exit though, then we may
be taking a reference after the last reference had already been put.
This fixes the problem in a different way. Most of the locking code is
not terribly concerned with the actual filp, but rather with the inode.
So we can fix this by adding helpers {flock|posix}_inode_lock_wait
helpers and using those from the NFSv4 code. We _know_ that we hold a
reference to the inode by virtue of the NFS open context, so this should
be safe.
I intend to do some more testing of this set over the weekend, but wanted
to get this out there so we can go ahead and get the patch reverted and
get some early feedback.
Jeff Layton (4):
Revert "nfs: take extra reference to fl->fl_file when running a LOCKU
operation"
locks: have flock_lock_file take an inode pointer instead of a filp
locks: new helpers - flock_lock_inode_wait and posix_lock_inode_wait
nfs4: have do_vfs_lock take an inode pointer
fs/locks.c | 60 ++++++++++++++++++++++++++++++++++++++----------------
fs/nfs/nfs4proc.c | 18 ++++++++--------
include/linux/fs.h | 14 +++++++++++++
3 files changed, 65 insertions(+), 27 deletions(-)
--
2.4.3
This reverts commit db2efec0caba4f81a22d95a34da640b86c313c8e.
William reported that he was seeing instability with this patch, which
is likely due to the fact that it can cause the kernel to take a new
reference to a filp after the last reference has already been put.
Revert this patch for now, as we'll need to fix this in another way.
Cc: [email protected]
Reported-by: William Dauchy <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfs/nfs4proc.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 6f228b5af819..60be01f69b84 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5484,7 +5484,6 @@ static struct nfs4_unlockdata *nfs4_alloc_unlockdata(struct file_lock *fl,
atomic_inc(&lsp->ls_count);
/* Ensure we don't close file until we're done freeing locks! */
p->ctx = get_nfs_open_context(ctx);
- get_file(fl->fl_file);
memcpy(&p->fl, fl, sizeof(p->fl));
p->server = NFS_SERVER(inode);
return p;
@@ -5496,7 +5495,6 @@ static void nfs4_locku_release_calldata(void *data)
nfs_free_seqid(calldata->arg.seqid);
nfs4_put_lock_state(calldata->lsp);
put_nfs_open_context(calldata->ctx);
- fput(calldata->fl.fl_file);
kfree(calldata);
}
--
2.4.3
...and rename it to better describe how it works.
In order to fix a use-after-free in NFS, we need to be able to remove
locks from an inode after the filp associated with them may have already
been freed. flock_lock_file already only dereferences the filp to get to
the inode, so just change it so the callers do that.
All of the callers already pass in a lock request that has the fl_file
set properly, so we don't need to pass it in individually. With that
change it now only dereferences the filp to get to the inode, so just
push that out to the callers.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/locks.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/locks.c b/fs/locks.c
index 653faabb07f4..4366b7c54e6d 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -862,12 +862,11 @@ static int posix_locks_deadlock(struct file_lock *caller_fl,
* whether or not a lock was successfully freed by testing the return
* value for -ENOENT.
*/
-static int flock_lock_file(struct file *filp, struct file_lock *request)
+static int flock_lock_inode(struct inode *inode, struct file_lock *request)
{
struct file_lock *new_fl = NULL;
struct file_lock *fl;
struct file_lock_context *ctx;
- struct inode *inode = file_inode(filp);
int error = 0;
bool found = false;
LIST_HEAD(dispose);
@@ -890,7 +889,7 @@ static int flock_lock_file(struct file *filp, struct file_lock *request)
goto find_conflict;
list_for_each_entry(fl, &ctx->flc_flock, fl_list) {
- if (filp != fl->fl_file)
+ if (request->fl_file != fl->fl_file)
continue;
if (request->fl_type == fl->fl_type)
goto out;
@@ -1862,7 +1861,7 @@ int flock_lock_file_wait(struct file *filp, struct file_lock *fl)
int error;
might_sleep();
for (;;) {
- error = flock_lock_file(filp, fl);
+ error = flock_lock_inode(file_inode(filp), fl);
if (error != FILE_LOCK_DEFERRED)
break;
error = wait_event_interruptible(fl->fl_wait, !fl->fl_next);
@@ -2401,7 +2400,8 @@ locks_remove_flock(struct file *filp)
.fl_type = F_UNLCK,
.fl_end = OFFSET_MAX,
};
- struct file_lock_context *flctx = file_inode(filp)->i_flctx;
+ struct inode *inode = file_inode(filp);
+ struct file_lock_context *flctx = inode->i_flctx;
if (list_empty(&flctx->flc_flock))
return;
@@ -2409,7 +2409,7 @@ locks_remove_flock(struct file *filp)
if (filp->f_op->flock)
filp->f_op->flock(filp, F_SETLKW, &fl);
else
- flock_lock_file(filp, &fl);
+ flock_lock_inode(inode, &fl);
if (fl.fl_ops && fl.fl_ops->fl_release_private)
fl.fl_ops->fl_release_private(&fl);
--
2.4.3
Now that we have file locking helpers that can deal with an inode
instead of a filp, we can change the NFSv4 locking code to use that
instead.
This should fix the case where we have a filp that is closed while flock
or OFD locks are set on it, and the task is signaled so that it doesn't
wait for the LOCKU reply to come in before the filp is freed. At that
point we can end up with a use-after-free with the current code, which
relies on dereferencing the fl_file in the lock request.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfs/nfs4proc.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 60be01f69b84..8bee93469617 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5439,15 +5439,15 @@ static int nfs4_proc_getlk(struct nfs4_state *state, int cmd, struct file_lock *
return err;
}
-static int do_vfs_lock(struct file *file, struct file_lock *fl)
+static int do_vfs_lock(struct inode *inode, struct file_lock *fl)
{
int res = 0;
switch (fl->fl_flags & (FL_POSIX|FL_FLOCK)) {
case FL_POSIX:
- res = posix_lock_file_wait(file, fl);
+ res = posix_lock_inode_wait(inode, fl);
break;
case FL_FLOCK:
- res = flock_lock_file_wait(file, fl);
+ res = flock_lock_inode_wait(inode, fl);
break;
default:
BUG();
@@ -5507,7 +5507,7 @@ static void nfs4_locku_done(struct rpc_task *task, void *data)
switch (task->tk_status) {
case 0:
renew_lease(calldata->server, calldata->timestamp);
- do_vfs_lock(calldata->fl.fl_file, &calldata->fl);
+ do_vfs_lock(calldata->lsp->ls_state->inode, &calldata->fl);
if (nfs4_update_lock_stateid(calldata->lsp,
&calldata->res.stateid))
break;
@@ -5615,7 +5615,7 @@ static int nfs4_proc_unlck(struct nfs4_state *state, int cmd, struct file_lock *
mutex_lock(&sp->so_delegreturn_mutex);
/* Exclude nfs4_reclaim_open_stateid() - note nesting! */
down_read(&nfsi->rwsem);
- if (do_vfs_lock(request->fl_file, request) == -ENOENT) {
+ if (do_vfs_lock(inode, request) == -ENOENT) {
up_read(&nfsi->rwsem);
mutex_unlock(&sp->so_delegreturn_mutex);
goto out;
@@ -5756,7 +5756,7 @@ static void nfs4_lock_done(struct rpc_task *task, void *calldata)
data->timestamp);
if (data->arg.new_lock) {
data->fl.fl_flags &= ~(FL_SLEEP | FL_ACCESS);
- if (do_vfs_lock(data->fl.fl_file, &data->fl) < 0) {
+ if (do_vfs_lock(lsp->ls_state->inode, &data->fl) < 0) {
rpc_restart_call_prepare(task);
break;
}
@@ -5998,7 +5998,7 @@ static int _nfs4_proc_setlk(struct nfs4_state *state, int cmd, struct file_lock
if (status != 0)
goto out;
request->fl_flags |= FL_ACCESS;
- status = do_vfs_lock(request->fl_file, request);
+ status = do_vfs_lock(state->inode, request);
if (status < 0)
goto out;
down_read(&nfsi->rwsem);
@@ -6006,7 +6006,7 @@ static int _nfs4_proc_setlk(struct nfs4_state *state, int cmd, struct file_lock
/* Yes: cache locks! */
/* ...but avoid races with delegation recall... */
request->fl_flags = fl_flags & ~FL_SLEEP;
- status = do_vfs_lock(request->fl_file, request);
+ status = do_vfs_lock(state->inode, request);
up_read(&nfsi->rwsem);
goto out;
}
--
2.4.3
Allow callers to pass in an inode instead of a filp.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/locks.c | 50 ++++++++++++++++++++++++++++++++++++++------------
include/linux/fs.h | 14 ++++++++++++++
2 files changed, 52 insertions(+), 12 deletions(-)
diff --git a/fs/locks.c b/fs/locks.c
index 4366b7c54e6d..ba268a503c1b 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1163,20 +1163,19 @@ int posix_lock_file(struct file *filp, struct file_lock *fl,
EXPORT_SYMBOL(posix_lock_file);
/**
- * posix_lock_file_wait - Apply a POSIX-style lock to a file
- * @filp: The file to apply the lock to
+ * posix_lock_inode_wait - Apply a POSIX-style lock to a file
+ * @inode: inode of file to which lock request should be applied
* @fl: The lock to be applied
*
- * Add a POSIX style lock to a file.
- * We merge adjacent & overlapping locks whenever possible.
- * POSIX locks are sorted by owner task, then by starting address
+ * Variant of posix_lock_file_wait that does not take a filp, and so can be
+ * used after the filp has already been torn down.
*/
-int posix_lock_file_wait(struct file *filp, struct file_lock *fl)
+int posix_lock_inode_wait(struct inode *inode, struct file_lock *fl)
{
int error;
might_sleep ();
for (;;) {
- error = posix_lock_file(filp, fl, NULL);
+ error = __posix_lock_file(inode, fl, NULL);
if (error != FILE_LOCK_DEFERRED)
break;
error = wait_event_interruptible(fl->fl_wait, !fl->fl_next);
@@ -1188,6 +1187,21 @@ int posix_lock_file_wait(struct file *filp, struct file_lock *fl)
}
return error;
}
+EXPORT_SYMBOL(posix_lock_inode_wait);
+
+/**
+ * posix_lock_file_wait - Apply a POSIX-style lock to a file
+ * @filp: The file to apply the lock to
+ * @fl: The lock to be applied
+ *
+ * Add a POSIX style lock to a file.
+ * We merge adjacent & overlapping locks whenever possible.
+ * POSIX locks are sorted by owner task, then by starting address
+ */
+int posix_lock_file_wait(struct file *filp, struct file_lock *fl)
+{
+ return posix_lock_inode_wait(file_inode(filp), fl);
+}
EXPORT_SYMBOL(posix_lock_file_wait);
/**
@@ -1850,18 +1864,18 @@ int fcntl_setlease(unsigned int fd, struct file *filp, long arg)
}
/**
- * flock_lock_file_wait - Apply a FLOCK-style lock to a file
- * @filp: The file to apply the lock to
+ * flock_lock_inode_wait - Apply a FLOCK-style lock to a file
+ * @inode: inode of the file to apply to
* @fl: The lock to be applied
*
- * Add a FLOCK style lock to a file.
+ * Apply a FLOCK style lock request to an inode.
*/
-int flock_lock_file_wait(struct file *filp, struct file_lock *fl)
+int flock_lock_inode_wait(struct inode *inode, struct file_lock *fl)
{
int error;
might_sleep();
for (;;) {
- error = flock_lock_inode(file_inode(filp), fl);
+ error = flock_lock_inode(inode, fl);
if (error != FILE_LOCK_DEFERRED)
break;
error = wait_event_interruptible(fl->fl_wait, !fl->fl_next);
@@ -1873,7 +1887,19 @@ int flock_lock_file_wait(struct file *filp, struct file_lock *fl)
}
return error;
}
+EXPORT_SYMBOL(flock_lock_inode_wait);
+/**
+ * flock_lock_file_wait - Apply a FLOCK-style lock to a file
+ * @filp: The file to apply the lock to
+ * @fl: The lock to be applied
+ *
+ * Add a FLOCK style lock to a file.
+ */
+int flock_lock_file_wait(struct file *filp, struct file_lock *fl)
+{
+ return flock_lock_inode_wait(file_inode(filp), fl);
+}
EXPORT_SYMBOL(flock_lock_file_wait);
/**
diff --git a/include/linux/fs.h b/include/linux/fs.h
index a0653e560c26..4c990edd1377 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1046,11 +1046,13 @@ extern void locks_remove_file(struct file *);
extern void locks_release_private(struct file_lock *);
extern void posix_test_lock(struct file *, struct file_lock *);
extern int posix_lock_file(struct file *, struct file_lock *, struct file_lock *);
+extern int posix_lock_inode_wait(struct inode *, struct file_lock *);
extern int posix_lock_file_wait(struct file *, struct file_lock *);
extern int posix_unblock_lock(struct file_lock *);
extern int vfs_test_lock(struct file *, struct file_lock *);
extern int vfs_lock_file(struct file *, unsigned int, struct file_lock *, struct file_lock *);
extern int vfs_cancel_lock(struct file *filp, struct file_lock *fl);
+extern int flock_lock_inode_wait(struct inode *inode, struct file_lock *fl);
extern int flock_lock_file_wait(struct file *filp, struct file_lock *fl);
extern int __break_lease(struct inode *inode, unsigned int flags, unsigned int type);
extern void lease_get_mtime(struct inode *, struct timespec *time);
@@ -1137,6 +1139,12 @@ static inline int posix_lock_file(struct file *filp, struct file_lock *fl,
return -ENOLCK;
}
+static inline int posix_lock_inode_wait(struct inode *inode,
+ struct file_lock *fl)
+{
+ return -ENOLCK;
+}
+
static inline int posix_lock_file_wait(struct file *filp, struct file_lock *fl)
{
return -ENOLCK;
@@ -1163,6 +1171,12 @@ static inline int vfs_cancel_lock(struct file *filp, struct file_lock *fl)
return 0;
}
+static inline int flock_lock_inode_wait(struct inode *inode,
+ struct file_lock *request)
+{
+ return -ENOLCK;
+}
+
static inline int flock_lock_file_wait(struct file *filp,
struct file_lock *request)
{
--
2.4.3
On Fri, 10 Jul 2015 16:33:31 -0400
Jeff Layton <[email protected]> wrote:
> This reverts commit db2efec0caba4f81a22d95a34da640b86c313c8e.
>
> William reported that he was seeing instability with this patch, which
> is likely due to the fact that it can cause the kernel to take a new
> reference to a filp after the last reference has already been put.
>
> Revert this patch for now, as we'll need to fix this in another way.
>
> Cc: [email protected]
> Reported-by: William Dauchy <[email protected]>
> Signed-off-by: Jeff Layton <[email protected]>
> ---
> fs/nfs/nfs4proc.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 6f228b5af819..60be01f69b84 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -5484,7 +5484,6 @@ static struct nfs4_unlockdata *nfs4_alloc_unlockdata(struct file_lock *fl,
> atomic_inc(&lsp->ls_count);
> /* Ensure we don't close file until we're done freeing locks! */
> p->ctx = get_nfs_open_context(ctx);
> - get_file(fl->fl_file);
> memcpy(&p->fl, fl, sizeof(p->fl));
> p->server = NFS_SERVER(inode);
> return p;
> @@ -5496,7 +5495,6 @@ static void nfs4_locku_release_calldata(void *data)
> nfs_free_seqid(calldata->arg.seqid);
> nfs4_put_lock_state(calldata->lsp);
> put_nfs_open_context(calldata->ctx);
> - fput(calldata->fl.fl_file);
> kfree(calldata);
> }
>
Even though the title says "RFC" here, we should probably go ahead and
revert this patch in mainline and any stable series kernels in which it
got applied.
Let me know if I need to do anything further in order to make that
happen.
Thanks,
--
Jeff Layton <[email protected]>
On Fri, Jul 10, 2015 at 04:33:30PM -0400, Jeff Layton wrote:
> William Dauchy reported some instability after trying to apply
> db2efec0caba to the v3.14 stable series kernels. The problem was that
> that patch had us taking an extra reference to the filp in the NFSv4
> unlock code. If we're unlocking locks on task exit though, then we may
> be taking a reference after the last reference had already been put.
>
> This fixes the problem in a different way. Most of the locking code is
> not terribly concerned with the actual filp, but rather with the inode.
> So we can fix this by adding helpers {flock|posix}_inode_lock_wait
> helpers and using those from the NFSv4 code. We _know_ that we hold a
> reference to the inode by virtue of the NFS open context, so this should
> be safe.
>
> I intend to do some more testing of this set over the weekend, but wanted
> to get this out there so we can go ahead and get the patch reverted and
> get some early feedback.
>
> Jeff Layton (4):
> Revert "nfs: take extra reference to fl->fl_file when running a LOCKU
> operation"
> locks: have flock_lock_file take an inode pointer instead of a filp
> locks: new helpers - flock_lock_inode_wait and posix_lock_inode_wait
> nfs4: have do_vfs_lock take an inode pointer
>
> fs/locks.c | 60 ++++++++++++++++++++++++++++++++++++++----------------
> fs/nfs/nfs4proc.c | 18 ++++++++--------
> include/linux/fs.h | 14 +++++++++++++
> 3 files changed, 65 insertions(+), 27 deletions(-)
By the way, I noticed just yesterday that my testing of recent upstream
was failing--processes getting stuck in some lock tests, with log
warnings as appended below. After applying these four patches it looks
like that's no longer happening.
I can run some more tests over the weekend if that'd help confirm.
Definitely in favor of expediting these fixes if possible as it was
blocking my testing and looks easy to reproduce. But I haven't reviewed
them yet.
--b.
[ 158.972038] ------------[ cut here ]------------
[ 158.972346] WARNING: CPU: 0 PID: 0 at kernel/rcu/tree.c:2694
rcu_process_callbacks+0x6d1/0x980()
[ 158.972893] Modules linked in: rpcsec_gss_krb5 nfsv4 nfs nfsd
auth_rpcgss oid_registry nfs_acl lockd grace sunrpc
[ 158.974257] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
4.2.0-rc1-00004-gfbb2913 #222
[ 158.974714] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS 1.7.5-20140709_153950- 04/01/2014
[ 158.975307] ffffffff81f0a3de ffff88007f803e28 ffffffff81ac1cf7
0000000000000102
[ 158.976022] 0000000000000000 ffff88007f803e68 ffffffff81078ec6
0000000100000000
[ 158.976022] ffff88007f8160c0 0000000000000002 0000000000000000
0000000000000246
[ 158.976022] Call Trace:
[ 158.976022] <IRQ> [<ffffffff81ac1cf7>] dump_stack+0x4f/0x7b
[ 158.976022] [<ffffffff81078ec6>] warn_slowpath_common+0x86/0xc0
[ 158.976022] [<ffffffff81078fba>] warn_slowpath_null+0x1a/0x20
[ 158.976022] [<ffffffff810e1691>] rcu_process_callbacks+0x6d1/0x980
[ 158.976022] [<ffffffff810e127d>] ? rcu_process_callbacks+0x2bd/0x980
[ 158.976022] [<ffffffff810b6466>] ? run_rebalance_domains+0x156/0x3d0
[ 158.976022] [<ffffffff810b6315>] ? run_rebalance_domains+0x5/0x3d0
[ 158.976022] [<ffffffff8107d834>] __do_softirq+0xd4/0x5f0
[ 158.976022] [<ffffffff8107de9c>] irq_exit+0x5c/0x60
[ 158.976022] [<ffffffff81acdee6>] smp_apic_timer_interrupt+0x46/0x60
[ 158.976022] [<ffffffff81acc2ad>] apic_timer_interrupt+0x6d/0x80
[ 158.976022] <EOI> [<ffffffff8100dd55>] ? default_idle+0x25/0x210
[ 158.976022] [<ffffffff8100dd53>] ? default_idle+0x23/0x210
[ 158.976022] [<ffffffff8100e4ef>] arch_cpu_idle+0xf/0x20
[ 158.976022] [<ffffffff810bd37a>] default_idle_call+0x2a/0x40
[ 158.976022] [<ffffffff810bd607>] cpu_startup_entry+0x217/0x450
[ 158.976022] [<ffffffff81abc074>] rest_init+0x134/0x140
[ 158.976022] [<ffffffff82354ebe>] start_kernel+0x43c/0x449
[ 158.976022] [<ffffffff82354495>] x86_64_start_reservations+0x2a/0x2c
[ 158.976022] [<ffffffff8235457d>] x86_64_start_kernel+0xe6/0xea
[ 158.976022] ---[ end trace 9d29787e24defe89 ]---
[ 245.953371] general protection fault: 0000 [#1] PREEMPT SMP
DEBUG_PAGEALLOC
[ 245.954280] Modules linked in: nfsv3 rpcsec_gss_krb5 nfsv4 nfs nfsd
auth_rpcgss oid_registry nfs_acl lockd grace sunrpc
[ 245.955795] CPU: 1 PID: 6604 Comm: plock_tests Tainted: G W
4.2.0-rc1-00004-gfbb2913 #222
[ 245.956098] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS 1.7.5-20140709_153950- 04/01/2014
[ 245.956098] task: ffff8800502ec0c0 ti: ffff8800576cc000 task.ti:
ffff8800576cc000
[ 245.956098] RIP: 0010:[<ffffffff811ae3c1>] [<ffffffff811ae3c1>]
filp_close+0x31/0x80
[ 245.956098] RSP: 0018:ffff8800576cfbf8 EFLAGS: 00010206
[ 245.956098] RAX: 5d5f415e415d415c RBX: ffff8800727dee40 RCX:
ffff880063c00dd0
[ 245.956098] RDX: 0000000080000000 RSI: ffff880063c00cc0 RDI:
ffff8800727dee40
[ 245.956098] RBP: ffff8800576cfc18 R08: 0000000000000000 R09:
0000000000000001
[ 245.956098] R10: 0000000000000000 R11: 0000000000000000 R12:
0000000000000000
[ 245.956098] R13: ffff880063c00cc0 R14: ffff880063c00d18 R15:
0000000000000007
[ 245.956098] FS: 00007f3a4655d700(0000) GS:ffff88007f900000(0000)
knlGS:0000000000000000
[ 245.956098] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 245.956098] CR2: 00007f3a4655cff8 CR3: 0000000049c7b000 CR4:
00000000000406e0
[ 245.956098] Stack:
[ 245.956098] 00000000576cfc18 00000000000001ff 0000000000000000
ffff880063c00cc0
[ 245.956098] ffff8800576cfc68 ffffffff811d001a ffff880063c00cc0
00000001502eccb0
[ 245.956098] 0000000000000008 ffff8800502ec0c0 ffff880063c00cc0
ffff8800502eccb0
[ 245.956098] Call Trace:
[ 245.956098] [<ffffffff811d001a>] put_files_struct+0x7a/0xf0
[ 245.956098] [<ffffffff811d013b>] exit_files+0x4b/0x60
[ 245.956098] [<ffffffff8107acc7>] do_exit+0x3a7/0xc90
[ 245.956098] [<ffffffff81acaaa0>] ? _raw_spin_unlock_irq+0x30/0x60
[ 245.956098] [<ffffffff8107c96e>] do_group_exit+0x4e/0xc0
[ 245.956098] [<ffffffff810891b8>] get_signal+0x238/0x9b0
[ 245.956098] [<ffffffff81003278>] do_signal+0x28/0x690
[ 245.956098] [<ffffffff81205082>] ? fcntl_setlk+0x72/0x430
[ 245.956098] [<ffffffff81acb5e0>] ? int_very_careful+0x5/0x3f
[ 245.956098] [<ffffffff810c7942>] ?
trace_hardirqs_on_caller+0x122/0x1b0
[ 245.956098] [<ffffffff81003925>] do_notify_resume+0x45/0x60
[ 245.956098] [<ffffffff81acb62c>] int_signal+0x12/0x17
[ 245.956098] Code: 48 89 e5 41 55 41 54 53 48 89 fb 48 83 ec 08 48 8b
47 68 48 85 c0 74 4a 48 8b 47 28 45 31 e4 49 89 f5 48 8b 40 68 48 85 c0
74 05 <ff> d0 41 89 c4 f6 43 75 40 75 16 4c 89 ee 48 89 df e8 f9 8e 04
[ 245.956098] RIP [<ffffffff811ae3c1>] filp_close+0x31/0x80
[ 245.956098] RSP <ffff8800576cfbf8>
[ 245.980303] ---[ end trace 9d29787e24defe8a ]---
[ 245.981531] Fixing recursive fault but reboot is needed!
[ 246.957011] general protection fault: 0000 [#2] PREEMPT SMP
DEBUG_PAGEALLOC
[ 246.957933] Modules linked in: nfsv3 rpcsec_gss_krb5 nfsv4 nfs nfsd
auth_rpcgss oid_registry nfs_acl lockd grace sunrpc
[ 246.959467] CPU: 1 PID: 6602 Comm: plock_tests Tainted: G D W
4.2.0-rc1-00004-gfbb2913 #222
[ 246.960075] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS 1.7.5-20140709_153950- 04/01/2014
[ 246.960075] task: ffff88004cf54380 ti: ffff88004cc40000 task.ti:
ffff88004cc40000
[ 246.960075] RIP: 0010:[<ffffffff811ae3c1>] [<ffffffff811ae3c1>]
filp_close+0x31/0x80
[ 246.960075] RSP: 0018:ffff88004cc43bf8 EFLAGS: 00010206
[ 246.960075] RAX: 5d5f415e415d415c RBX: ffff8800727dee40 RCX:
ffff88006c373dd0
[ 246.960075] RDX: 0000000080000000 RSI: ffff88006c373cc0 RDI:
ffff8800727dee40
[ 246.960075] RBP: ffff88004cc43c18 R08: 0000000000000001 R09:
0000000000000001
[ 246.960075] R10: 0000000000000000 R11: 0000000000000000 R12:
0000000000000000
[ 246.960075] R13: ffff88006c373cc0 R14: ffff88006c373d18 R15:
0000000000000007
[ 246.960075] FS: 00007f3a46d4a700(0000) GS:ffff88007f900000(0000)
knlGS:0000000000000000
[ 246.960075] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 246.960075] CR2: 00007fa219b62010 CR3: 000000000220b000 CR4:
00000000000406e0
[ 246.960075] Stack:
[ 246.960075] 000000004cc43c18 0000000000000001 0000000000000000
ffff88006c373cc0
[ 246.960075] ffff88004cc43c68 ffffffff811d001a ffff88006c373cc0
000000014cf54f70
[ 246.960075] 0000000000000008 ffff88004cf54380 ffff88006c373cc0
ffff88004cf54f70
[ 246.960075] Call Trace:
[ 246.960075] [<ffffffff811d001a>] put_files_struct+0x7a/0xf0
[ 246.960075] [<ffffffff811d013b>] exit_files+0x4b/0x60
[ 246.960075] [<ffffffff8107acc7>] do_exit+0x3a7/0xc90
[ 246.960075] [<ffffffff810c79dd>] ? trace_hardirqs_on+0xd/0x10
[ 246.960075] [<ffffffff8107c96e>] do_group_exit+0x4e/0xc0
[ 246.960075] [<ffffffff810891b8>] get_signal+0x238/0x9b0
[ 246.960075] [<ffffffff81003278>] do_signal+0x28/0x690
[ 246.960075] [<ffffffff811afbc7>] ? __vfs_read+0xa7/0xd0
[ 246.960075] [<ffffffff811b034a>] ? vfs_read+0x8a/0x110
[ 246.960075] [<ffffffff81003925>] do_notify_resume+0x45/0x60
[ 246.960075] [<ffffffff81acb62c>] int_signal+0x12/0x17
[ 246.960075] Code: 48 89 e5 41 55 41 54 53 48 89 fb 48 83 ec 08 48 8b
47 68 48 85 c0 74 4a 48 8b 47 28 45 31 e4 49 89 f5 48 8b 40 68 48 85 c0
74 05 <ff> d0 41 89 c4 f6 43 75 40 75 16 4c 89 ee 48 89 df e8 f9 8e 04
[ 246.960075] RIP [<ffffffff811ae3c1>] filp_close+0x31/0x80
[ 246.960075] RSP <ffff88004cc43bf8>
[ 246.978654] ---[ end trace 9d29787e24defe8b ]---
[ 246.979247] Fixing recursive fault but reboot is needed!
On Fri, 10 Jul 2015 17:10:54 -0400
"J. Bruce Fields" <[email protected]> wrote:
> On Fri, Jul 10, 2015 at 04:33:30PM -0400, Jeff Layton wrote:
> > William Dauchy reported some instability after trying to apply
> > db2efec0caba to the v3.14 stable series kernels. The problem was that
> > that patch had us taking an extra reference to the filp in the NFSv4
> > unlock code. If we're unlocking locks on task exit though, then we may
> > be taking a reference after the last reference had already been put.
> >
> > This fixes the problem in a different way. Most of the locking code is
> > not terribly concerned with the actual filp, but rather with the inode.
> > So we can fix this by adding helpers {flock|posix}_inode_lock_wait
> > helpers and using those from the NFSv4 code. We _know_ that we hold a
> > reference to the inode by virtue of the NFS open context, so this should
> > be safe.
> >
> > I intend to do some more testing of this set over the weekend, but wanted
> > to get this out there so we can go ahead and get the patch reverted and
> > get some early feedback.
> >
> > Jeff Layton (4):
> > Revert "nfs: take extra reference to fl->fl_file when running a LOCKU
> > operation"
> > locks: have flock_lock_file take an inode pointer instead of a filp
> > locks: new helpers - flock_lock_inode_wait and posix_lock_inode_wait
> > nfs4: have do_vfs_lock take an inode pointer
> >
> > fs/locks.c | 60 ++++++++++++++++++++++++++++++++++++++----------------
> > fs/nfs/nfs4proc.c | 18 ++++++++--------
> > include/linux/fs.h | 14 +++++++++++++
> > 3 files changed, 65 insertions(+), 27 deletions(-)
>
> By the way, I noticed just yesterday that my testing of recent upstream
> was failing--processes getting stuck in some lock tests, with log
> warnings as appended below. After applying these four patches it looks
> like that's no longer happening.
>
> I can run some more tests over the weekend if that'd help confirm.
>
> Definitely in favor of expediting these fixes if possible as it was
> blocking my testing and looks easy to reproduce. But I haven't reviewed
> them yet.
>
> --b.
>
> [ 158.972038] ------------[ cut here ]------------
> [ 158.972346] WARNING: CPU: 0 PID: 0 at kernel/rcu/tree.c:2694
> rcu_process_callbacks+0x6d1/0x980()
> [ 158.972893] Modules linked in: rpcsec_gss_krb5 nfsv4 nfs nfsd
> auth_rpcgss oid_registry nfs_acl lockd grace sunrpc
> [ 158.974257] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
> 4.2.0-rc1-00004-gfbb2913 #222
> [ 158.974714] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS 1.7.5-20140709_153950- 04/01/2014
> [ 158.975307] ffffffff81f0a3de ffff88007f803e28 ffffffff81ac1cf7
> 0000000000000102
> [ 158.976022] 0000000000000000 ffff88007f803e68 ffffffff81078ec6
> 0000000100000000
> [ 158.976022] ffff88007f8160c0 0000000000000002 0000000000000000
> 0000000000000246
> [ 158.976022] Call Trace:
> [ 158.976022] <IRQ> [<ffffffff81ac1cf7>] dump_stack+0x4f/0x7b
> [ 158.976022] [<ffffffff81078ec6>] warn_slowpath_common+0x86/0xc0
> [ 158.976022] [<ffffffff81078fba>] warn_slowpath_null+0x1a/0x20
> [ 158.976022] [<ffffffff810e1691>] rcu_process_callbacks+0x6d1/0x980
> [ 158.976022] [<ffffffff810e127d>] ? rcu_process_callbacks+0x2bd/0x980
> [ 158.976022] [<ffffffff810b6466>] ? run_rebalance_domains+0x156/0x3d0
> [ 158.976022] [<ffffffff810b6315>] ? run_rebalance_domains+0x5/0x3d0
> [ 158.976022] [<ffffffff8107d834>] __do_softirq+0xd4/0x5f0
> [ 158.976022] [<ffffffff8107de9c>] irq_exit+0x5c/0x60
> [ 158.976022] [<ffffffff81acdee6>] smp_apic_timer_interrupt+0x46/0x60
> [ 158.976022] [<ffffffff81acc2ad>] apic_timer_interrupt+0x6d/0x80
> [ 158.976022] <EOI> [<ffffffff8100dd55>] ? default_idle+0x25/0x210
> [ 158.976022] [<ffffffff8100dd53>] ? default_idle+0x23/0x210
> [ 158.976022] [<ffffffff8100e4ef>] arch_cpu_idle+0xf/0x20
> [ 158.976022] [<ffffffff810bd37a>] default_idle_call+0x2a/0x40
> [ 158.976022] [<ffffffff810bd607>] cpu_startup_entry+0x217/0x450
> [ 158.976022] [<ffffffff81abc074>] rest_init+0x134/0x140
> [ 158.976022] [<ffffffff82354ebe>] start_kernel+0x43c/0x449
> [ 158.976022] [<ffffffff82354495>] x86_64_start_reservations+0x2a/0x2c
> [ 158.976022] [<ffffffff8235457d>] x86_64_start_kernel+0xe6/0xea
> [ 158.976022] ---[ end trace 9d29787e24defe89 ]---
> [ 245.953371] general protection fault: 0000 [#1] PREEMPT SMP
> DEBUG_PAGEALLOC
> [ 245.954280] Modules linked in: nfsv3 rpcsec_gss_krb5 nfsv4 nfs nfsd
> auth_rpcgss oid_registry nfs_acl lockd grace sunrpc
> [ 245.955795] CPU: 1 PID: 6604 Comm: plock_tests Tainted: G W
> 4.2.0-rc1-00004-gfbb2913 #222
> [ 245.956098] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS 1.7.5-20140709_153950- 04/01/2014
> [ 245.956098] task: ffff8800502ec0c0 ti: ffff8800576cc000 task.ti:
> ffff8800576cc000
> [ 245.956098] RIP: 0010:[<ffffffff811ae3c1>] [<ffffffff811ae3c1>]
> filp_close+0x31/0x80
> [ 245.956098] RSP: 0018:ffff8800576cfbf8 EFLAGS: 00010206
> [ 245.956098] RAX: 5d5f415e415d415c RBX: ffff8800727dee40 RCX:
> ffff880063c00dd0
> [ 245.956098] RDX: 0000000080000000 RSI: ffff880063c00cc0 RDI:
> ffff8800727dee40
> [ 245.956098] RBP: ffff8800576cfc18 R08: 0000000000000000 R09:
> 0000000000000001
> [ 245.956098] R10: 0000000000000000 R11: 0000000000000000 R12:
> 0000000000000000
> [ 245.956098] R13: ffff880063c00cc0 R14: ffff880063c00d18 R15:
> 0000000000000007
> [ 245.956098] FS: 00007f3a4655d700(0000) GS:ffff88007f900000(0000)
> knlGS:0000000000000000
> [ 245.956098] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 245.956098] CR2: 00007f3a4655cff8 CR3: 0000000049c7b000 CR4:
> 00000000000406e0
> [ 245.956098] Stack:
> [ 245.956098] 00000000576cfc18 00000000000001ff 0000000000000000
> ffff880063c00cc0
> [ 245.956098] ffff8800576cfc68 ffffffff811d001a ffff880063c00cc0
> 00000001502eccb0
> [ 245.956098] 0000000000000008 ffff8800502ec0c0 ffff880063c00cc0
> ffff8800502eccb0
> [ 245.956098] Call Trace:
> [ 245.956098] [<ffffffff811d001a>] put_files_struct+0x7a/0xf0
> [ 245.956098] [<ffffffff811d013b>] exit_files+0x4b/0x60
> [ 245.956098] [<ffffffff8107acc7>] do_exit+0x3a7/0xc90
> [ 245.956098] [<ffffffff81acaaa0>] ? _raw_spin_unlock_irq+0x30/0x60
> [ 245.956098] [<ffffffff8107c96e>] do_group_exit+0x4e/0xc0
> [ 245.956098] [<ffffffff810891b8>] get_signal+0x238/0x9b0
> [ 245.956098] [<ffffffff81003278>] do_signal+0x28/0x690
> [ 245.956098] [<ffffffff81205082>] ? fcntl_setlk+0x72/0x430
> [ 245.956098] [<ffffffff81acb5e0>] ? int_very_careful+0x5/0x3f
> [ 245.956098] [<ffffffff810c7942>] ?
> trace_hardirqs_on_caller+0x122/0x1b0
> [ 245.956098] [<ffffffff81003925>] do_notify_resume+0x45/0x60
> [ 245.956098] [<ffffffff81acb62c>] int_signal+0x12/0x17
> [ 245.956098] Code: 48 89 e5 41 55 41 54 53 48 89 fb 48 83 ec 08 48 8b
> 47 68 48 85 c0 74 4a 48 8b 47 28 45 31 e4 49 89 f5 48 8b 40 68 48 85 c0
> 74 05 <ff> d0 41 89 c4 f6 43 75 40 75 16 4c 89 ee 48 89 df e8 f9 8e 04
> [ 245.956098] RIP [<ffffffff811ae3c1>] filp_close+0x31/0x80
> [ 245.956098] RSP <ffff8800576cfbf8>
> [ 245.980303] ---[ end trace 9d29787e24defe8a ]---
> [ 245.981531] Fixing recursive fault but reboot is needed!
> [ 246.957011] general protection fault: 0000 [#2] PREEMPT SMP
> DEBUG_PAGEALLOC
> [ 246.957933] Modules linked in: nfsv3 rpcsec_gss_krb5 nfsv4 nfs nfsd
> auth_rpcgss oid_registry nfs_acl lockd grace sunrpc
> [ 246.959467] CPU: 1 PID: 6602 Comm: plock_tests Tainted: G D W
> 4.2.0-rc1-00004-gfbb2913 #222
> [ 246.960075] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS 1.7.5-20140709_153950- 04/01/2014
> [ 246.960075] task: ffff88004cf54380 ti: ffff88004cc40000 task.ti:
> ffff88004cc40000
> [ 246.960075] RIP: 0010:[<ffffffff811ae3c1>] [<ffffffff811ae3c1>]
> filp_close+0x31/0x80
> [ 246.960075] RSP: 0018:ffff88004cc43bf8 EFLAGS: 00010206
> [ 246.960075] RAX: 5d5f415e415d415c RBX: ffff8800727dee40 RCX:
> ffff88006c373dd0
> [ 246.960075] RDX: 0000000080000000 RSI: ffff88006c373cc0 RDI:
> ffff8800727dee40
> [ 246.960075] RBP: ffff88004cc43c18 R08: 0000000000000001 R09:
> 0000000000000001
> [ 246.960075] R10: 0000000000000000 R11: 0000000000000000 R12:
> 0000000000000000
> [ 246.960075] R13: ffff88006c373cc0 R14: ffff88006c373d18 R15:
> 0000000000000007
> [ 246.960075] FS: 00007f3a46d4a700(0000) GS:ffff88007f900000(0000)
> knlGS:0000000000000000
> [ 246.960075] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 246.960075] CR2: 00007fa219b62010 CR3: 000000000220b000 CR4:
> 00000000000406e0
> [ 246.960075] Stack:
> [ 246.960075] 000000004cc43c18 0000000000000001 0000000000000000
> ffff88006c373cc0
> [ 246.960075] ffff88004cc43c68 ffffffff811d001a ffff88006c373cc0
> 000000014cf54f70
> [ 246.960075] 0000000000000008 ffff88004cf54380 ffff88006c373cc0
> ffff88004cf54f70
> [ 246.960075] Call Trace:
> [ 246.960075] [<ffffffff811d001a>] put_files_struct+0x7a/0xf0
> [ 246.960075] [<ffffffff811d013b>] exit_files+0x4b/0x60
> [ 246.960075] [<ffffffff8107acc7>] do_exit+0x3a7/0xc90
> [ 246.960075] [<ffffffff810c79dd>] ? trace_hardirqs_on+0xd/0x10
> [ 246.960075] [<ffffffff8107c96e>] do_group_exit+0x4e/0xc0
> [ 246.960075] [<ffffffff810891b8>] get_signal+0x238/0x9b0
> [ 246.960075] [<ffffffff81003278>] do_signal+0x28/0x690
> [ 246.960075] [<ffffffff811afbc7>] ? __vfs_read+0xa7/0xd0
> [ 246.960075] [<ffffffff811b034a>] ? vfs_read+0x8a/0x110
> [ 246.960075] [<ffffffff81003925>] do_notify_resume+0x45/0x60
> [ 246.960075] [<ffffffff81acb62c>] int_signal+0x12/0x17
> [ 246.960075] Code: 48 89 e5 41 55 41 54 53 48 89 fb 48 83 ec 08 48 8b
> 47 68 48 85 c0 74 4a 48 8b 47 28 45 31 e4 49 89 f5 48 8b 40 68 48 85 c0
> 74 05 <ff> d0 41 89 c4 f6 43 75 40 75 16 4c 89 ee 48 89 df e8 f9 8e 04
> [ 246.960075] RIP [<ffffffff811ae3c1>] filp_close+0x31/0x80
> [ 246.960075] RSP <ffff88004cc43bf8>
> [ 246.978654] ---[ end trace 9d29787e24defe8b ]---
> [ 246.979247] Fixing recursive fault but reboot is needed!
>
Thanks for testing, Bruce. Yeah at the very least we should get the
first one in ASAP. The rest can probably wait for more thorough review.
There is a real bug there, but I don't think people are hitting it
regularly.
--
Jeff Layton <[email protected]>
On Fri, Jul 10, 2015 at 05:14:26PM -0400, Jeff Layton wrote:
> Thanks for testing, Bruce. Yeah at the very least we should get the
> first one in ASAP. The rest can probably wait for more thorough review.
> There is a real bug there, but I don't think people are hitting it
> regularly.
OK. Free feel to add my tested-by: and/or reviewed-by: for those, they
are passing my tests and the patches look right to me....
--b.
They just call file_inode and then the corresponding *_inode_file_wait
function. Just make them static inlines instead.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/locks.c | 28 ----------------------------
include/linux/fs.h | 32 ++++++++++++++------------------
2 files changed, 14 insertions(+), 46 deletions(-)
diff --git a/fs/locks.c b/fs/locks.c
index ba268a503c1b..d3d558ba4da7 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1190,21 +1190,6 @@ int posix_lock_inode_wait(struct inode *inode, struct file_lock *fl)
EXPORT_SYMBOL(posix_lock_inode_wait);
/**
- * posix_lock_file_wait - Apply a POSIX-style lock to a file
- * @filp: The file to apply the lock to
- * @fl: The lock to be applied
- *
- * Add a POSIX style lock to a file.
- * We merge adjacent & overlapping locks whenever possible.
- * POSIX locks are sorted by owner task, then by starting address
- */
-int posix_lock_file_wait(struct file *filp, struct file_lock *fl)
-{
- return posix_lock_inode_wait(file_inode(filp), fl);
-}
-EXPORT_SYMBOL(posix_lock_file_wait);
-
-/**
* locks_mandatory_locked - Check for an active lock
* @file: the file to check
*
@@ -1890,19 +1875,6 @@ int flock_lock_inode_wait(struct inode *inode, struct file_lock *fl)
EXPORT_SYMBOL(flock_lock_inode_wait);
/**
- * flock_lock_file_wait - Apply a FLOCK-style lock to a file
- * @filp: The file to apply the lock to
- * @fl: The lock to be applied
- *
- * Add a FLOCK style lock to a file.
- */
-int flock_lock_file_wait(struct file *filp, struct file_lock *fl)
-{
- return flock_lock_inode_wait(file_inode(filp), fl);
-}
-EXPORT_SYMBOL(flock_lock_file_wait);
-
-/**
* sys_flock: - flock() system call.
* @fd: the file descriptor to lock.
* @cmd: the type of lock to apply.
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 4c990edd1377..cc008c338f5a 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1047,13 +1047,11 @@ extern void locks_release_private(struct file_lock *);
extern void posix_test_lock(struct file *, struct file_lock *);
extern int posix_lock_file(struct file *, struct file_lock *, struct file_lock *);
extern int posix_lock_inode_wait(struct inode *, struct file_lock *);
-extern int posix_lock_file_wait(struct file *, struct file_lock *);
extern int posix_unblock_lock(struct file_lock *);
extern int vfs_test_lock(struct file *, struct file_lock *);
extern int vfs_lock_file(struct file *, unsigned int, struct file_lock *, struct file_lock *);
extern int vfs_cancel_lock(struct file *filp, struct file_lock *fl);
extern int flock_lock_inode_wait(struct inode *inode, struct file_lock *fl);
-extern int flock_lock_file_wait(struct file *filp, struct file_lock *fl);
extern int __break_lease(struct inode *inode, unsigned int flags, unsigned int type);
extern void lease_get_mtime(struct inode *, struct timespec *time);
extern int generic_setlease(struct file *, long, struct file_lock **, void **priv);
@@ -1145,11 +1143,6 @@ static inline int posix_lock_inode_wait(struct inode *inode,
return -ENOLCK;
}
-static inline int posix_lock_file_wait(struct file *filp, struct file_lock *fl)
-{
- return -ENOLCK;
-}
-
static inline int posix_unblock_lock(struct file_lock *waiter)
{
return -ENOENT;
@@ -1177,12 +1170,6 @@ static inline int flock_lock_inode_wait(struct inode *inode,
return -ENOLCK;
}
-static inline int flock_lock_file_wait(struct file *filp,
- struct file_lock *request)
-{
- return -ENOLCK;
-}
-
static inline int __break_lease(struct inode *inode, unsigned int mode, unsigned int type)
{
return 0;
@@ -1216,6 +1203,20 @@ static inline void show_fd_locks(struct seq_file *f,
struct file *filp, struct files_struct *files) {}
#endif /* !CONFIG_FILE_LOCKING */
+static inline struct inode *file_inode(const struct file *f)
+{
+ return f->f_inode;
+}
+
+static inline int posix_lock_file_wait(struct file *filp, struct file_lock *fl)
+{
+ return posix_lock_inode_wait(file_inode(filp), fl);
+}
+
+static inline int flock_lock_file_wait(struct file *filp, struct file_lock *fl)
+{
+ return flock_lock_inode_wait(file_inode(filp), fl);
+}
struct fasync_struct {
spinlock_t fa_lock;
@@ -2025,11 +2026,6 @@ extern void ihold(struct inode * inode);
extern void iput(struct inode *);
extern int generic_update_time(struct inode *, struct timespec *, int);
-static inline struct inode *file_inode(const struct file *f)
-{
- return f->f_inode;
-}
-
/* /sys/fs */
extern struct kobject *fs_kobj;
--
2.4.3