From: Dan Carpenter <dan.carpenter@oracle.com>
To: namjae.jeon@samsung.com
Cc: linux-cifs@vger.kernel.org
Subject: [bug report] ksmbd: fix unused err value in smb2_lock
Date: Mon, 26 Jul 2021 14:41:37 +0300 [thread overview]
Message-ID: <20210726114137.GA30721@kili> (raw)
Hello Namjae Jeon,
The patch 96ad4ec51c06: "ksmbd: fix unused err value in smb2_lock"
from Jul 13, 2021, leads to the following static checker warning:
fs/ksmbd/smb2pdu.c:6565 smb2_lock()
warn: missing error code here? 'smb_flock_init()' failed.
fs/ksmbd/smb2pdu.c
6518 int smb2_lock(struct ksmbd_work *work)
6519 {
6520 struct smb2_lock_req *req = work->request_buf;
6521 struct smb2_lock_rsp *rsp = work->response_buf;
6522 struct smb2_lock_element *lock_ele;
6523 struct ksmbd_file *fp = NULL;
6524 struct file_lock *flock = NULL;
6525 struct file *filp = NULL;
6526 int lock_count;
6527 int flags = 0;
6528 int cmd = 0;
6529 int err = 0, i;
6530 u64 lock_start, lock_length;
6531 struct ksmbd_lock *smb_lock = NULL, *cmp_lock, *tmp, *tmp2;
6532 struct ksmbd_conn *conn;
6533 int nolock = 0;
6534 LIST_HEAD(lock_list);
6535 LIST_HEAD(rollback_list);
6536 int prior_lock = 0;
6537
6538 ksmbd_debug(SMB, "Received lock request\n");
6539 fp = ksmbd_lookup_fd_slow(work,
6540 le64_to_cpu(req->VolatileFileId),
6541 le64_to_cpu(req->PersistentFileId));
6542 if (!fp) {
6543 ksmbd_debug(SMB, "Invalid file id for lock : %llu\n",
6544 le64_to_cpu(req->VolatileFileId));
6545 rsp->hdr.Status = STATUS_FILE_CLOSED;
6546 goto out2;
6547 }
6548
6549 filp = fp->filp;
6550 lock_count = le16_to_cpu(req->LockCount);
6551 lock_ele = req->locks;
6552
6553 ksmbd_debug(SMB, "lock count is %d\n", lock_count);
6554 if (!lock_count) {
6555 rsp->hdr.Status = STATUS_INVALID_PARAMETER;
6556 goto out2;
6557 }
6558
6559 for (i = 0; i < lock_count; i++) {
6560 flags = le32_to_cpu(lock_ele[i].Flags);
6561
6562 flock = smb_flock_init(filp);
6563 if (!flock) {
6564 rsp->hdr.Status = STATUS_LOCK_NOT_GRANTED;
--> 6565 goto out;
^^^^^^^^
Should this be an error code?
6566 }
6567
6568 cmd = smb2_set_flock_flags(flock, flags);
6569
6570 lock_start = le64_to_cpu(lock_ele[i].Offset);
6571 lock_length = le64_to_cpu(lock_ele[i].Length);
6572 if (lock_start > U64_MAX - lock_length) {
6573 pr_err("Invalid lock range requested\n");
6574 rsp->hdr.Status = STATUS_INVALID_LOCK_RANGE;
6575 goto out;
Same for a bunch of these early gotos as well.
6576 }
6577
6578 if (lock_start > OFFSET_MAX)
6579 flock->fl_start = OFFSET_MAX;
6580 else
6581 flock->fl_start = lock_start;
6582
6583 lock_length = le64_to_cpu(lock_ele[i].Length);
6584 if (lock_length > OFFSET_MAX - flock->fl_start)
6585 lock_length = OFFSET_MAX - flock->fl_start;
6586
6587 flock->fl_end = flock->fl_start + lock_length;
6588
6589 if (flock->fl_end < flock->fl_start) {
6590 ksmbd_debug(SMB,
6591 "the end offset(%llx) is smaller than the start offset(%llx)\n",
6592 flock->fl_end, flock->fl_start);
6593 rsp->hdr.Status = STATUS_INVALID_LOCK_RANGE;
6594 goto out;
6595 }
6596
6597 /* Check conflict locks in one request */
6598 list_for_each_entry(cmp_lock, &lock_list, llist) {
6599 if (cmp_lock->fl->fl_start <= flock->fl_start &&
6600 cmp_lock->fl->fl_end >= flock->fl_end) {
6601 if (cmp_lock->fl->fl_type != F_UNLCK &&
6602 flock->fl_type != F_UNLCK) {
6603 pr_err("conflict two locks in one request\n");
6604 rsp->hdr.Status =
6605 STATUS_INVALID_PARAMETER;
6606 goto out;
6607 }
6608 }
6609 }
6610
6611 smb_lock = smb2_lock_init(flock, cmd, flags, &lock_list);
6612 if (!smb_lock) {
6613 rsp->hdr.Status = STATUS_INVALID_PARAMETER;
6614 goto out;
6615 }
6616 }
6617
6618 list_for_each_entry_safe(smb_lock, tmp, &lock_list, llist) {
6619 if (smb_lock->cmd < 0) {
6620 rsp->hdr.Status = STATUS_INVALID_PARAMETER;
6621 goto out;
6622 }
6623
6624 if (!(smb_lock->flags & SMB2_LOCKFLAG_MASK)) {
6625 rsp->hdr.Status = STATUS_INVALID_PARAMETER;
6626 goto out;
6627 }
6628
6629 if ((prior_lock & (SMB2_LOCKFLAG_EXCLUSIVE | SMB2_LOCKFLAG_SHARED) &&
6630 smb_lock->flags & SMB2_LOCKFLAG_UNLOCK) ||
6631 (prior_lock == SMB2_LOCKFLAG_UNLOCK &&
6632 !(smb_lock->flags & SMB2_LOCKFLAG_UNLOCK))) {
6633 rsp->hdr.Status = STATUS_INVALID_PARAMETER;
6634 goto out;
6635 }
6636
6637 prior_lock = smb_lock->flags;
6638
6639 if (!(smb_lock->flags & SMB2_LOCKFLAG_UNLOCK) &&
6640 !(smb_lock->flags & SMB2_LOCKFLAG_FAIL_IMMEDIATELY))
6641 goto no_check_cl;
6642
6643 nolock = 1;
6644 /* check locks in connection list */
6645 read_lock(&conn_list_lock);
6646 list_for_each_entry(conn, &conn_list, conns_list) {
6647 spin_lock(&conn->llist_lock);
6648 list_for_each_entry_safe(cmp_lock, tmp2, &conn->lock_list, clist) {
6649 if (file_inode(cmp_lock->fl->fl_file) !=
6650 file_inode(smb_lock->fl->fl_file))
6651 continue;
6652
6653 if (smb_lock->fl->fl_type == F_UNLCK) {
6654 if (cmp_lock->fl->fl_file == smb_lock->fl->fl_file &&
6655 cmp_lock->start == smb_lock->start &&
6656 cmp_lock->end == smb_lock->end &&
6657 !lock_defer_pending(cmp_lock->fl)) {
6658 nolock = 0;
6659 list_del(&cmp_lock->flist);
6660 list_del(&cmp_lock->clist);
6661 spin_unlock(&conn->llist_lock);
6662 read_unlock(&conn_list_lock);
6663
6664 locks_free_lock(cmp_lock->fl);
6665 kfree(cmp_lock);
6666 goto out_check_cl;
6667 }
6668 continue;
6669 }
6670
6671 if (cmp_lock->fl->fl_file == smb_lock->fl->fl_file) {
6672 if (smb_lock->flags & SMB2_LOCKFLAG_SHARED)
6673 continue;
6674 } else {
6675 if (cmp_lock->flags & SMB2_LOCKFLAG_SHARED)
6676 continue;
6677 }
6678
6679 /* check zero byte lock range */
6680 if (cmp_lock->zero_len && !smb_lock->zero_len &&
6681 cmp_lock->start > smb_lock->start &&
6682 cmp_lock->start < smb_lock->end) {
6683 spin_unlock(&conn->llist_lock);
6684 read_unlock(&conn_list_lock);
6685 pr_err("previous lock conflict with zero byte lock range\n");
6686 rsp->hdr.Status = STATUS_LOCK_NOT_GRANTED;
6687 goto out;
6688 }
6689
6690 if (smb_lock->zero_len && !cmp_lock->zero_len &&
6691 smb_lock->start > cmp_lock->start &&
6692 smb_lock->start < cmp_lock->end) {
6693 spin_unlock(&conn->llist_lock);
6694 read_unlock(&conn_list_lock);
6695 pr_err("current lock conflict with zero byte lock range\n");
6696 rsp->hdr.Status = STATUS_LOCK_NOT_GRANTED;
6697 goto out;
6698 }
6699
6700 if (((cmp_lock->start <= smb_lock->start &&
6701 cmp_lock->end > smb_lock->start) ||
6702 (cmp_lock->start < smb_lock->end &&
6703 cmp_lock->end >= smb_lock->end)) &&
6704 !cmp_lock->zero_len && !smb_lock->zero_len) {
6705 spin_unlock(&conn->llist_lock);
6706 read_unlock(&conn_list_lock);
6707 pr_err("Not allow lock operation on exclusive lock range\n");
6708 rsp->hdr.Status =
6709 STATUS_LOCK_NOT_GRANTED;
6710 goto out;
6711 }
6712 }
6713 spin_unlock(&conn->llist_lock);
6714 }
6715 read_unlock(&conn_list_lock);
6716 out_check_cl:
6717 if (smb_lock->fl->fl_type == F_UNLCK && nolock) {
6718 pr_err("Try to unlock nolocked range\n");
6719 rsp->hdr.Status = STATUS_RANGE_NOT_LOCKED;
6720 goto out;
6721 }
6722
6723 no_check_cl:
6724 if (smb_lock->zero_len) {
6725 err = 0;
6726 goto skip;
6727 }
6728
6729 flock = smb_lock->fl;
6730 list_del(&smb_lock->llist);
6731 retry:
6732 err = vfs_lock_file(filp, smb_lock->cmd, flock, NULL);
6733 skip:
6734 if (flags & SMB2_LOCKFLAG_UNLOCK) {
6735 if (!err) {
6736 ksmbd_debug(SMB, "File unlocked\n");
6737 } else if (err == -ENOENT) {
6738 rsp->hdr.Status = STATUS_NOT_LOCKED;
6739 goto out;
6740 }
6741 locks_free_lock(flock);
6742 kfree(smb_lock);
6743 } else {
6744 if (err == FILE_LOCK_DEFERRED) {
6745 void **argv;
6746
6747 ksmbd_debug(SMB,
6748 "would have to wait for getting lock\n");
6749 spin_lock(&work->conn->llist_lock);
6750 list_add_tail(&smb_lock->clist,
6751 &work->conn->lock_list);
6752 spin_unlock(&work->conn->llist_lock);
6753 list_add(&smb_lock->llist, &rollback_list);
6754
6755 argv = kmalloc(sizeof(void *), GFP_KERNEL);
6756 if (!argv) {
6757 err = -ENOMEM;
6758 goto out;
6759 }
6760 argv[0] = flock;
6761
6762 err = setup_async_work(work,
6763 smb2_remove_blocked_lock,
6764 argv);
6765 if (err) {
6766 rsp->hdr.Status =
6767 STATUS_INSUFFICIENT_RESOURCES;
6768 goto out;
6769 }
6770 spin_lock(&fp->f_lock);
6771 list_add(&work->fp_entry, &fp->blocked_works);
6772 spin_unlock(&fp->f_lock);
6773
6774 smb2_send_interim_resp(work, STATUS_PENDING);
6775
6776 ksmbd_vfs_posix_lock_wait(flock);
6777
6778 if (work->state != KSMBD_WORK_ACTIVE) {
6779 list_del(&smb_lock->llist);
6780 spin_lock(&work->conn->llist_lock);
6781 list_del(&smb_lock->clist);
6782 spin_unlock(&work->conn->llist_lock);
6783 locks_free_lock(flock);
6784
6785 if (work->state == KSMBD_WORK_CANCELLED) {
6786 spin_lock(&fp->f_lock);
6787 list_del(&work->fp_entry);
6788 spin_unlock(&fp->f_lock);
6789 rsp->hdr.Status =
6790 STATUS_CANCELLED;
6791 kfree(smb_lock);
6792 smb2_send_interim_resp(work,
6793 STATUS_CANCELLED);
6794 work->send_no_response = 1;
6795 goto out;
6796 }
6797 init_smb2_rsp_hdr(work);
6798 smb2_set_err_rsp(work);
6799 rsp->hdr.Status =
6800 STATUS_RANGE_NOT_LOCKED;
6801 kfree(smb_lock);
6802 goto out2;
6803 }
6804
6805 list_del(&smb_lock->llist);
6806 spin_lock(&work->conn->llist_lock);
6807 list_del(&smb_lock->clist);
6808 spin_unlock(&work->conn->llist_lock);
6809
6810 spin_lock(&fp->f_lock);
6811 list_del(&work->fp_entry);
6812 spin_unlock(&fp->f_lock);
6813 goto retry;
6814 } else if (!err) {
6815 spin_lock(&work->conn->llist_lock);
6816 list_add_tail(&smb_lock->clist,
6817 &work->conn->lock_list);
6818 list_add_tail(&smb_lock->flist,
6819 &fp->lock_list);
6820 spin_unlock(&work->conn->llist_lock);
6821 list_add(&smb_lock->llist, &rollback_list);
6822 ksmbd_debug(SMB, "successful in taking lock\n");
6823 } else {
6824 rsp->hdr.Status = STATUS_LOCK_NOT_GRANTED;
6825 goto out;
6826 }
6827 }
6828 }
6829
6830 if (atomic_read(&fp->f_ci->op_count) > 1)
6831 smb_break_all_oplock(work, fp);
6832
6833 rsp->StructureSize = cpu_to_le16(4);
6834 ksmbd_debug(SMB, "successful in taking lock\n");
6835 rsp->hdr.Status = STATUS_SUCCESS;
6836 rsp->Reserved = 0;
6837 inc_rfc1001_len(rsp, 4);
6838 ksmbd_fd_put(work, fp);
6839 return 0;
6840
6841 out:
6842 list_for_each_entry_safe(smb_lock, tmp, &lock_list, llist) {
6843 locks_free_lock(smb_lock->fl);
6844 list_del(&smb_lock->llist);
6845 kfree(smb_lock);
6846 }
6847
6848 list_for_each_entry_safe(smb_lock, tmp, &rollback_list, llist) {
6849 struct file_lock *rlock = NULL;
6850 int rc;
6851
6852 rlock = smb_flock_init(filp);
6853 rlock->fl_type = F_UNLCK;
6854 rlock->fl_start = smb_lock->start;
6855 rlock->fl_end = smb_lock->end;
6856
6857 rc = vfs_lock_file(filp, 0, rlock, NULL);
6858 if (rc)
6859 pr_err("rollback unlock fail : %d\n", rc);
6860
6861 list_del(&smb_lock->llist);
6862 spin_lock(&work->conn->llist_lock);
6863 if (!list_empty(&smb_lock->flist))
6864 list_del(&smb_lock->flist);
6865 list_del(&smb_lock->clist);
6866 spin_unlock(&work->conn->llist_lock);
6867
6868 locks_free_lock(smb_lock->fl);
6869 locks_free_lock(rlock);
6870 kfree(smb_lock);
6871 }
6872 out2:
6873 ksmbd_debug(SMB, "failed in taking lock(flags : %x)\n", flags);
6874 smb2_set_err_rsp(work);
6875 ksmbd_fd_put(work, fp);
6876 return err;
6877 }
regards,
dan carpenter
next reply other threads:[~2021-07-26 11:41 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20210726114156epcas1p4e34eb25899ecee5d6ba97d1b01ddda4e@epcas1p4.samsung.com>
2021-07-26 11:41 ` Dan Carpenter [this message]
2021-07-26 23:05 ` [bug report] ksmbd: fix unused err value in smb2_lock Namjae Jeon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210726114137.GA30721@kili \
--to=dan.carpenter@oracle.com \
--cc=linux-cifs@vger.kernel.org \
--cc=namjae.jeon@samsung.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).