lockd: fix races in client GRANTED_MSG wait logic

After the wait for a grant is done (for whatever reason), nlmclnt_block
updates the status of the nlm_rqst with the status of the block. At the
point it does this, however, the block is still queued its status could
change at any time.

This is particularly a problem when the waiting task is signaled during
the wait. We can end up giving up on the lock just before the GRANTED_MSG
callback comes in, and accept it even though the lock request gets back
an error, leaving a dangling lock on the server.

Since the nlm_wait never lives beyond the end of nlmclnt_lock, put it on
the stack and add functions to allow us to enqueue and dequeue the
block. Enqueue it just before the lock/wait loop, and dequeue it
just after we exit the loop instead of waiting until the end of
the function. Also, scrape the status at the time that we dequeue it to
ensure that it's final.

Reported-by: Yongcheng Yang <yoyang@redhat.com>
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2063818
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

authored by

Jeff Layton and committed by

Chuck Lever 3 years ago 2005f5b9 f0aa4852

+44 -34

3 changed files

expand all

lockd

clntlock.c

clntproc.c

include

linux

lockd

lockd.h

+21 -21

fs/lockd/clntlock.c

··· 82 82 } 83 83 EXPORT_SYMBOL_GPL(nlmclnt_done); 84 84 85 + void nlmclnt_prepare_block(struct nlm_wait *block, struct nlm_host *host, struct file_lock *fl) 86 + { 87 + block->b_host = host; 88 + block->b_lock = fl; 89 + init_waitqueue_head(&block->b_wait); 90 + block->b_status = nlm_lck_blocked; 91 + } 92 + 85 93 /* 86 94 * Queue up a lock for blocking so that the GRANTED request can see it 87 95 */ 88 - struct nlm_wait *nlmclnt_prepare_block(struct nlm_host *host, struct file_lock *fl) 96 + void nlmclnt_queue_block(struct nlm_wait *block) 89 97 { 90 - struct nlm_wait *block; 91 - 92 - block = kmalloc(sizeof(*block), GFP_KERNEL); 93 - if (block != NULL) { 94 - block->b_host = host; 95 - block->b_lock = fl; 96 - init_waitqueue_head(&block->b_wait); 97 - block->b_status = nlm_lck_blocked; 98 - 99 - spin_lock(&nlm_blocked_lock); 100 - list_add(&block->b_list, &nlm_blocked); 101 - spin_unlock(&nlm_blocked_lock); 102 - } 103 - return block; 98 + spin_lock(&nlm_blocked_lock); 99 + list_add(&block->b_list, &nlm_blocked); 100 + spin_unlock(&nlm_blocked_lock); 104 101 } 105 102 106 - void nlmclnt_finish_block(struct nlm_wait *block) 103 + /* 104 + * Dequeue the block and return its final status 105 + */ 106 + __be32 nlmclnt_dequeue_block(struct nlm_wait *block) 107 107 { 108 - if (block == NULL) 109 - return; 108 + __be32 status; 109 + 110 110 spin_lock(&nlm_blocked_lock); 111 111 list_del(&block->b_list); 112 + status = block->b_status; 112 113 spin_unlock(&nlm_blocked_lock); 113 - kfree(block); 114 + return status; 114 115 } 115 116 116 117 /* 117 118 * Block on a lock 118 119 */ 119 - int nlmclnt_block(struct nlm_wait *block, struct nlm_rqst *req, long timeout) 120 + int nlmclnt_wait(struct nlm_wait *block, struct nlm_rqst *req, long timeout) 120 121 { 121 122 long ret; 122 123 ··· 143 142 /* Reset the lock status after a server reboot so we resend */ 144 143 if (block->b_status == nlm_lck_denied_grace_period) 145 144 block->b_status = nlm_lck_blocked; 146 - req->a_res.status = block->b_status; 147 145 return 0; 148 146 } 149 147

+18 -10

fs/lockd/clntproc.c

··· 516 516 const struct cred *cred = nfs_file_cred(fl->fl_file); 517 517 struct nlm_host *host = req->a_host; 518 518 struct nlm_res *resp = &req->a_res; 519 - struct nlm_wait *block = NULL; 519 + struct nlm_wait block; 520 520 unsigned char fl_flags = fl->fl_flags; 521 521 unsigned char fl_type; 522 + __be32 b_status; 522 523 int status = -ENOLCK; 523 524 524 525 if (nsm_monitor(host) < 0) ··· 532 531 if (status < 0) 533 532 goto out; 534 533 535 - block = nlmclnt_prepare_block(host, fl); 534 + nlmclnt_prepare_block(&block, host, fl); 536 535 again: 537 536 /* 538 537 * Initialise resp->status to a valid non-zero value, 539 538 * since 0 == nlm_lck_granted 540 539 */ 541 540 resp->status = nlm_lck_blocked; 542 - for(;;) { 541 + 542 + /* 543 + * A GRANTED callback can come at any time -- even before the reply 544 + * to the LOCK request arrives, so we queue the wait before 545 + * requesting the lock. 546 + */ 547 + nlmclnt_queue_block(&block); 548 + for (;;) { 543 549 /* Reboot protection */ 544 550 fl->fl_u.nfs_fl.state = host->h_state; 545 551 status = nlmclnt_call(cred, req, NLMPROC_LOCK); 546 552 if (status < 0) 547 553 break; 548 554 /* Did a reclaimer thread notify us of a server reboot? */ 549 - if (resp->status == nlm_lck_denied_grace_period) 555 + if (resp->status == nlm_lck_denied_grace_period) 550 556 continue; 551 557 if (resp->status != nlm_lck_blocked) 552 558 break; 553 559 /* Wait on an NLM blocking lock */ 554 - status = nlmclnt_block(block, req, NLMCLNT_POLL_TIMEOUT); 560 + status = nlmclnt_wait(&block, req, NLMCLNT_POLL_TIMEOUT); 555 561 if (status < 0) 556 562 break; 557 - if (resp->status != nlm_lck_blocked) 563 + if (block.b_status != nlm_lck_blocked) 558 564 break; 559 565 } 566 + b_status = nlmclnt_dequeue_block(&block); 567 + if (resp->status == nlm_lck_blocked) 568 + resp->status = b_status; 560 569 561 570 /* if we were interrupted while blocking, then cancel the lock request 562 571 * and exit ··· 575 564 if (!req->a_args.block) 576 565 goto out_unlock; 577 566 if (nlmclnt_cancel(host, req->a_args.block, fl) == 0) 578 - goto out_unblock; 567 + goto out; 579 568 } 580 569 581 570 if (resp->status == nlm_granted) { ··· 604 593 status = -ENOLCK; 605 594 else 606 595 status = nlm_stat_to_errno(resp->status); 607 - out_unblock: 608 - nlmclnt_finish_block(block); 609 596 out: 610 597 nlmclnt_release_call(req); 611 598 return status; ··· 611 602 /* Fatal error: ensure that we remove the lock altogether */ 612 603 dprintk("lockd: lock attempt ended in fatal error.\n" 613 604 " Attempting to unlock.\n"); 614 - nlmclnt_finish_block(block); 615 605 fl_type = fl->fl_type; 616 606 fl->fl_type = F_UNLCK; 617 607 down_read(&host->h_rwsem);

+5 -3

include/linux/lockd/lockd.h

··· 211 211 int nlm_async_call(struct nlm_rqst *, u32, const struct rpc_call_ops *); 212 212 int nlm_async_reply(struct nlm_rqst *, u32, const struct rpc_call_ops *); 213 213 void nlmclnt_release_call(struct nlm_rqst *); 214 - struct nlm_wait * nlmclnt_prepare_block(struct nlm_host *host, struct file_lock *fl); 215 - void nlmclnt_finish_block(struct nlm_wait *block); 216 - int nlmclnt_block(struct nlm_wait *block, struct nlm_rqst *req, long timeout); 214 + void nlmclnt_prepare_block(struct nlm_wait *block, struct nlm_host *host, 215 + struct file_lock *fl); 216 + void nlmclnt_queue_block(struct nlm_wait *block); 217 + __be32 nlmclnt_dequeue_block(struct nlm_wait *block); 218 + int nlmclnt_wait(struct nlm_wait *block, struct nlm_rqst *req, long timeout); 217 219 __be32 nlmclnt_grant(const struct sockaddr *addr, 218 220 const struct nlm_lock *lock); 219 221 void nlmclnt_recovery(struct nlm_host *);

Configure Feed

Configure Feed