Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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>

authored by

Amir Goldstein and committed by
J. Bruce Fields
64bed6cb 8163496e

+15 -15
+1 -1
fs/lockd/clntlock.c
··· 187 187 continue; 188 188 if (!rpc_cmp_addr(nlm_addr(block->b_host), addr)) 189 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 191 continue; 192 192 /* Alright, we found a lock. Set the return status 193 193 * and wake up the caller
+1 -1
fs/lockd/clntproc.c
··· 128 128 char *nodename = req->a_host->h_rpcclnt->cl_nodename; 129 129 130 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 132 lock->caller = nodename; 133 133 lock->oh.data = req->a_owner; 134 134 lock->oh.len = snprintf(req->a_owner, sizeof(req->a_owner), "%u@%s",
+8 -8
fs/lockd/svclock.c
··· 405 405 __be32 ret; 406 406 407 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, 409 - file_inode(file->f_file)->i_ino, 408 + locks_inode(file->f_file)->i_sb->s_id, 409 + locks_inode(file->f_file)->i_ino, 410 410 lock->fl.fl_type, lock->fl.fl_pid, 411 411 (long long)lock->fl.fl_start, 412 412 (long long)lock->fl.fl_end, ··· 511 511 __be32 ret; 512 512 513 513 dprintk("lockd: nlmsvc_testlock(%s/%ld, ty=%d, %Ld-%Ld)\n", 514 - file_inode(file->f_file)->i_sb->s_id, 515 - file_inode(file->f_file)->i_ino, 514 + locks_inode(file->f_file)->i_sb->s_id, 515 + locks_inode(file->f_file)->i_ino, 516 516 lock->fl.fl_type, 517 517 (long long)lock->fl.fl_start, 518 518 (long long)lock->fl.fl_end); ··· 566 566 int error; 567 567 568 568 dprintk("lockd: nlmsvc_unlock(%s/%ld, pi=%d, %Ld-%Ld)\n", 569 - file_inode(file->f_file)->i_sb->s_id, 570 - file_inode(file->f_file)->i_ino, 569 + locks_inode(file->f_file)->i_sb->s_id, 570 + locks_inode(file->f_file)->i_ino, 571 571 lock->fl.fl_pid, 572 572 (long long)lock->fl.fl_start, 573 573 (long long)lock->fl.fl_end); ··· 595 595 int status = 0; 596 596 597 597 dprintk("lockd: nlmsvc_cancel(%s/%ld, pi=%d, %Ld-%Ld)\n", 598 - file_inode(file->f_file)->i_sb->s_id, 599 - file_inode(file->f_file)->i_ino, 598 + locks_inode(file->f_file)->i_sb->s_id, 599 + locks_inode(file->f_file)->i_ino, 600 600 lock->fl.fl_pid, 601 601 (long long)lock->fl.fl_start, 602 602 (long long)lock->fl.fl_end);
+2 -2
fs/lockd/svcsubs.c
··· 44 44 45 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 49 dprintk("lockd: %s %s/%ld\n", 50 50 msg, inode->i_sb->s_id, inode->i_ino); ··· 414 414 { 415 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 /**
+1 -1
fs/nfsd/nfs4state.c
··· 6322 6322 return status; 6323 6323 } 6324 6324 6325 - inode = file_inode(filp); 6325 + inode = locks_inode(filp); 6326 6326 flctx = inode->i_flctx; 6327 6327 6328 6328 if (flctx && !list_empty_careful(&flctx->flc_posix)) {
+2 -2
include/linux/lockd/lockd.h
··· 299 299 300 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 305 static inline int __nlm_privileged_request4(const struct sockaddr *sap) ··· 359 359 static inline int nlm_compare_locks(const struct file_lock *fl1, 360 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 363 && fl1->fl_pid == fl2->fl_pid 364 364 && fl1->fl_owner == fl2->fl_owner 365 365 && fl1->fl_start == fl2->fl_start