Skip to contentSkip to conversation

locks: fix NULL-deref in generic_delete_lease

PATCH In review NeilBrown 2 participants 2 messages Jan 13, 2015

Subsystems (4)
lore
Share & tools
lore

NeilBrown

author
#0 lore raw

PATCH 1/1 locks: fix NULL-deref in generic_delete_lease

commit 0efaa7e82f02fe69c05ad28e905f31fc86e6f08e
locks: generic_delete_lease doesn't need a file_lock at all

moves the call to fl->fl_lmops->lm_change() to a place in the code where fl might be a non-lease lock.
When that happens, fl_lmops is NULL and an Oops ensures.

So add an extra test to restore correct functioning.

Fixes 0efaa7e82f02fe69c05ad28e905f31fc86e6f08e
---
 fs/locks.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/locks.c +1 -1
index 735b8d3fa78c..59e2f905e4ff 100644
@@ -1702,7 +1702,7 @@ static int generic_delete_lease(struct file *filp)
 			break;
 	}
 	trace_generic_delete_lease(inode, fl);
-	if (fl)
+	if (fl && IS_LEASE(fl))
 		error = fl->fl_lmops->lm_change(before, F_UNLCK, &dispose);
 	spin_unlock(&inode->i_lock);
 	locks_dispose_list(&dispose);
Signature · 1 line
2.1.1
#1 lore raw
Quoted 25 lines · On Tue, 13 Jan 2015 15:17:43 +1300 NeilBrown <[email protected]> wrote:

commit 0efaa7e82f02fe69c05ad28e905f31fc86e6f08e
locks: generic_delete_lease doesn't need a file_lock at all

moves the call to fl->fl_lmops->lm_change() to a place in the code where fl might be a non-lease lock.
When that happens, fl_lmops is NULL and an Oops ensures.

So add an extra test to restore correct functioning.

Fixes 0efaa7e82f02fe69c05ad28e905f31fc86e6f08e
---
 fs/locks.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/locks.c +0 -0
index 735b8d3fa78c..59e2f905e4ff 100644

fs/locks.c static int generic_delete_lease(struct file *filp) · line 1708

Quoted 8 lines
fs/locks.c +1 -1
@@ -1702,7 +1702,7 @@ static int generic_delete_lease(struct file *filp)
 			break;
 	}
 	trace_generic_delete_lease(inode, fl);
-	if (fl)
+	if (fl && IS_LEASE(fl))
 		error = fl->fl_lmops->lm_change(before, F_UNLCK, &dispose);
 	spin_unlock(&inode->i_lock);
 	locks_dispose_list(&dispose);

Looks good. I'll merge and do a bit of testing on it today and send it along to Linus in the next day or so.

Thanks!

Signature · 1 line
Jeff Layton <[email protected]>