diff options
author | Amir Goldstein <amir73il@gmail.com> | 2018-07-13 10:22:24 -0400 |
---|---|---|
committer | J. Bruce Fields <bfields@redhat.com> | 2018-08-09 16:11:21 -0400 |
commit | 64bed6cbe38bc95689fb9399872d9ce250192f90 (patch) | |
tree | 6376a71e746622c32818abd785c05a09593a01a8 | |
parent | 8163496e78db100a6b5cfbdaece385686ae50129 (diff) |
nfsd: fix leaked file lock with nfs exported overlayfs
nfsd and lockd call vfs_lock_file() to lock/unlock the inode
returned by locks_inode(file).
Many places in nfsd/lockd code use the inode returned by
file_inode(file) for lock manipulation. With Overlayfs, file_inode()
(the underlying inode) is not the same object as locks_inode() (the
overlay inode). This can result in "Leaked POSIX lock" messages
and eventually to a kernel crash as reported by Eddie Horng:
https://marc.info/?l=linux-unionfs&m=153086643202072&w=2
Fix all the call sites in nfsd/lockd that should use locks_inode().
This is a correctness bug that manifested when overlayfs gained
NFS export support in v4.16.
Reported-by: Eddie Horng <eddiehorng.tw@gmail.com>
Tested-by: Eddie Horng <eddiehorng.tw@gmail.com>
Cc: Jeff Layton <jlayton@kernel.org>
Fixes: 8383f1748829 ("ovl: wire up NFS export operations")
Cc: stable@vger.kernel.org
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
-rw-r--r-- | fs/lockd/clntlock.c | 2 | ||||
-rw-r--r-- | fs/lockd/clntproc.c | 2 | ||||
-rw-r--r-- | fs/lockd/svclock.c | 16 | ||||
-rw-r--r-- | fs/lockd/svcsubs.c | 4 | ||||
-rw-r--r-- | fs/nfsd/nfs4state.c | 2 | ||||
-rw-r--r-- | include/linux/lockd/lockd.h | 4 |
6 files changed, 15 insertions, 15 deletions
diff --git a/fs/lockd/clntlock.c b/fs/lockd/clntlock.c index 96c1d14c18f1..c2a128678e6e 100644 --- a/fs/lockd/clntlock.c +++ b/fs/lockd/clntlock.c | |||
@@ -187,7 +187,7 @@ __be32 nlmclnt_grant(const struct sockaddr *addr, const struct nlm_lock *lock) | |||
187 | continue; | 187 | continue; |
188 | if (!rpc_cmp_addr(nlm_addr(block->b_host), addr)) | 188 | if (!rpc_cmp_addr(nlm_addr(block->b_host), addr)) |
189 | continue; | 189 | continue; |
190 | if (nfs_compare_fh(NFS_FH(file_inode(fl_blocked->fl_file)) ,fh) != 0) | 190 | if (nfs_compare_fh(NFS_FH(locks_inode(fl_blocked->fl_file)), fh) != 0) |
191 | continue; | 191 | continue; |
192 | /* Alright, we found a lock. Set the return status | 192 | /* Alright, we found a lock. Set the return status |
193 | * and wake up the caller | 193 | * and wake up the caller |
diff --git a/fs/lockd/clntproc.c b/fs/lockd/clntproc.c index a2c0dfc6fdc0..d20b92f271c2 100644 --- a/fs/lockd/clntproc.c +++ b/fs/lockd/clntproc.c | |||
@@ -128,7 +128,7 @@ static void nlmclnt_setlockargs(struct nlm_rqst *req, struct file_lock *fl) | |||
128 | char *nodename = req->a_host->h_rpcclnt->cl_nodename; | 128 | char *nodename = req->a_host->h_rpcclnt->cl_nodename; |
129 | 129 | ||
130 | nlmclnt_next_cookie(&argp->cookie); | 130 | nlmclnt_next_cookie(&argp->cookie); |
131 | memcpy(&lock->fh, NFS_FH(file_inode(fl->fl_file)), sizeof(struct nfs_fh)); | 131 | memcpy(&lock->fh, NFS_FH(locks_inode(fl->fl_file)), sizeof(struct nfs_fh)); |
132 | lock->caller = nodename; | 132 | lock->caller = nodename; |
133 | lock->oh.data = req->a_owner; | 133 | lock->oh.data = req->a_owner; |
134 | lock->oh.len = snprintf(req->a_owner, sizeof(req->a_owner), "%u@%s", | 134 | lock->oh.len = snprintf(req->a_owner, sizeof(req->a_owner), "%u@%s", |
diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c index 3701bccab478..74330daeab71 100644 --- a/fs/lockd/svclock.c +++ b/fs/lockd/svclock.c | |||
@@ -405,8 +405,8 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file, | |||
405 | __be32 ret; | 405 | __be32 ret; |
406 | 406 | ||
407 | dprintk("lockd: nlmsvc_lock(%s/%ld, ty=%d, pi=%d, %Ld-%Ld, bl=%d)\n", | 407 | dprintk("lockd: nlmsvc_lock(%s/%ld, ty=%d, pi=%d, %Ld-%Ld, bl=%d)\n", |
408 | file_inode(file->f_file)->i_sb->s_id, | 408 | locks_inode(file->f_file)->i_sb->s_id, |
409 | file_inode(file->f_file)->i_ino, | 409 | locks_inode(file->f_file)->i_ino, |
410 | lock->fl.fl_type, lock->fl.fl_pid, | 410 | lock->fl.fl_type, lock->fl.fl_pid, |
411 | (long long)lock->fl.fl_start, | 411 | (long long)lock->fl.fl_start, |
412 | (long long)lock->fl.fl_end, | 412 | (long long)lock->fl.fl_end, |
@@ -511,8 +511,8 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file, | |||
511 | __be32 ret; | 511 | __be32 ret; |
512 | 512 | ||
513 | dprintk("lockd: nlmsvc_testlock(%s/%ld, ty=%d, %Ld-%Ld)\n", | 513 | dprintk("lockd: nlmsvc_testlock(%s/%ld, ty=%d, %Ld-%Ld)\n", |
514 | file_inode(file->f_file)->i_sb->s_id, | 514 | locks_inode(file->f_file)->i_sb->s_id, |
515 | file_inode(file->f_file)->i_ino, | 515 | locks_inode(file->f_file)->i_ino, |
516 | lock->fl.fl_type, | 516 | lock->fl.fl_type, |
517 | (long long)lock->fl.fl_start, | 517 | (long long)lock->fl.fl_start, |
518 | (long long)lock->fl.fl_end); | 518 | (long long)lock->fl.fl_end); |
@@ -566,8 +566,8 @@ nlmsvc_unlock(struct net *net, struct nlm_file *file, struct nlm_lock *lock) | |||
566 | int error; | 566 | int error; |
567 | 567 | ||
568 | dprintk("lockd: nlmsvc_unlock(%s/%ld, pi=%d, %Ld-%Ld)\n", | 568 | dprintk("lockd: nlmsvc_unlock(%s/%ld, pi=%d, %Ld-%Ld)\n", |
569 | file_inode(file->f_file)->i_sb->s_id, | 569 | locks_inode(file->f_file)->i_sb->s_id, |
570 | file_inode(file->f_file)->i_ino, | 570 | locks_inode(file->f_file)->i_ino, |
571 | lock->fl.fl_pid, | 571 | lock->fl.fl_pid, |
572 | (long long)lock->fl.fl_start, | 572 | (long long)lock->fl.fl_start, |
573 | (long long)lock->fl.fl_end); | 573 | (long long)lock->fl.fl_end); |
@@ -595,8 +595,8 @@ nlmsvc_cancel_blocked(struct net *net, struct nlm_file *file, struct nlm_lock *l | |||
595 | int status = 0; | 595 | int status = 0; |
596 | 596 | ||
597 | dprintk("lockd: nlmsvc_cancel(%s/%ld, pi=%d, %Ld-%Ld)\n", | 597 | dprintk("lockd: nlmsvc_cancel(%s/%ld, pi=%d, %Ld-%Ld)\n", |
598 | file_inode(file->f_file)->i_sb->s_id, | 598 | locks_inode(file->f_file)->i_sb->s_id, |
599 | file_inode(file->f_file)->i_ino, | 599 | locks_inode(file->f_file)->i_ino, |
600 | lock->fl.fl_pid, | 600 | lock->fl.fl_pid, |
601 | (long long)lock->fl.fl_start, | 601 | (long long)lock->fl.fl_start, |
602 | (long long)lock->fl.fl_end); | 602 | (long long)lock->fl.fl_end); |
diff --git a/fs/lockd/svcsubs.c b/fs/lockd/svcsubs.c index 4ec3d6e03e76..899360ba3b84 100644 --- a/fs/lockd/svcsubs.c +++ b/fs/lockd/svcsubs.c | |||
@@ -44,7 +44,7 @@ static inline void nlm_debug_print_fh(char *msg, struct nfs_fh *f) | |||
44 | 44 | ||
45 | static inline void nlm_debug_print_file(char *msg, struct nlm_file *file) | 45 | static inline void nlm_debug_print_file(char *msg, struct nlm_file *file) |
46 | { | 46 | { |
47 | struct inode *inode = file_inode(file->f_file); | 47 | struct inode *inode = locks_inode(file->f_file); |
48 | 48 | ||
49 | dprintk("lockd: %s %s/%ld\n", | 49 | dprintk("lockd: %s %s/%ld\n", |
50 | msg, inode->i_sb->s_id, inode->i_ino); | 50 | msg, inode->i_sb->s_id, inode->i_ino); |
@@ -414,7 +414,7 @@ nlmsvc_match_sb(void *datap, struct nlm_file *file) | |||
414 | { | 414 | { |
415 | struct super_block *sb = datap; | 415 | struct super_block *sb = datap; |
416 | 416 | ||
417 | return sb == file_inode(file->f_file)->i_sb; | 417 | return sb == locks_inode(file->f_file)->i_sb; |
418 | } | 418 | } |
419 | 419 | ||
420 | /** | 420 | /** |
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 19c4d29917ee..c8e4a8f42bbe 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c | |||
@@ -6322,7 +6322,7 @@ check_for_locks(struct nfs4_file *fp, struct nfs4_lockowner *lowner) | |||
6322 | return status; | 6322 | return status; |
6323 | } | 6323 | } |
6324 | 6324 | ||
6325 | inode = file_inode(filp); | 6325 | inode = locks_inode(filp); |
6326 | flctx = inode->i_flctx; | 6326 | flctx = inode->i_flctx; |
6327 | 6327 | ||
6328 | if (flctx && !list_empty_careful(&flctx->flc_posix)) { | 6328 | if (flctx && !list_empty_careful(&flctx->flc_posix)) { |
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h index 4fd95dbeb52f..b065ef406770 100644 --- a/include/linux/lockd/lockd.h +++ b/include/linux/lockd/lockd.h | |||
@@ -299,7 +299,7 @@ int nlmsvc_unlock_all_by_ip(struct sockaddr *server_addr); | |||
299 | 299 | ||
300 | static inline struct inode *nlmsvc_file_inode(struct nlm_file *file) | 300 | static inline struct inode *nlmsvc_file_inode(struct nlm_file *file) |
301 | { | 301 | { |
302 | return file_inode(file->f_file); | 302 | return locks_inode(file->f_file); |
303 | } | 303 | } |
304 | 304 | ||
305 | static inline int __nlm_privileged_request4(const struct sockaddr *sap) | 305 | static inline int __nlm_privileged_request4(const struct sockaddr *sap) |
@@ -359,7 +359,7 @@ static inline int nlm_privileged_requester(const struct svc_rqst *rqstp) | |||
359 | static inline int nlm_compare_locks(const struct file_lock *fl1, | 359 | static inline int nlm_compare_locks(const struct file_lock *fl1, |
360 | const struct file_lock *fl2) | 360 | const struct file_lock *fl2) |
361 | { | 361 | { |
362 | return file_inode(fl1->fl_file) == file_inode(fl2->fl_file) | 362 | return locks_inode(fl1->fl_file) == locks_inode(fl2->fl_file) |
363 | && fl1->fl_pid == fl2->fl_pid | 363 | && fl1->fl_pid == fl2->fl_pid |
364 | && fl1->fl_owner == fl2->fl_owner | 364 | && fl1->fl_owner == fl2->fl_owner |
365 | && fl1->fl_start == fl2->fl_start | 365 | && fl1->fl_start == fl2->fl_start |