Hello Namjae Jeon,
The patch b5e5f9dfc915: "ksmbd: check invalid FileOffset and
BeyondFinalZero in FSCTL_ZERO_DATA" from Jun 19, 2022, leads to the
following Smatch static checker warning:
fs/ksmbd/smb2pdu.c:7759 smb2_ioctl()
warn: no lower bound on 'off'
fs/ksmbd/smb2pdu.c
7573 int smb2_ioctl(struct ksmbd_work *work)
7574 {
7575 struct smb2_ioctl_req *req;
7576 struct smb2_ioctl_rsp *rsp;
7577 unsigned int cnt_code, nbytes = 0, out_buf_len, in_buf_len;
7578 u64 id = KSMBD_NO_FID;
7579 struct ksmbd_conn *conn = work->conn;
7580 int ret = 0;
7581
7582 if (work->next_smb2_rcv_hdr_off) {
7583 req = ksmbd_req_buf_next(work);
7584 rsp = ksmbd_resp_buf_next(work);
7585 if (!has_file_id(req->VolatileFileId)) {
7586 ksmbd_debug(SMB, "Compound request set FID = %llu\n",
7587 work->compound_fid);
7588 id = work->compound_fid;
7589 }
7590 } else {
7591 req = smb2_get_msg(work->request_buf);
7592 rsp = smb2_get_msg(work->response_buf);
7593 }
7594
7595 if (!has_file_id(id))
7596 id = req->VolatileFileId;
7597
7598 if (req->Flags != cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL)) {
7599 rsp->hdr.Status = STATUS_NOT_SUPPORTED;
7600 goto out;
7601 }
7602
7603 cnt_code = le32_to_cpu(req->CtlCode);
7604 ret = smb2_calc_max_out_buf_len(work, 48,
7605 le32_to_cpu(req->MaxOutputResponse));
7606 if (ret < 0) {
7607 rsp->hdr.Status = STATUS_INVALID_PARAMETER;
7608 goto out;
7609 }
7610 out_buf_len = (unsigned int)ret;
7611 in_buf_len = le32_to_cpu(req->InputCount);
7612
7613 switch (cnt_code) {
7614 case FSCTL_DFS_GET_REFERRALS:
7615 case FSCTL_DFS_GET_REFERRALS_EX:
7616 /* Not support DFS yet */
7617 rsp->hdr.Status = STATUS_FS_DRIVER_REQUIRED;
7618 goto out;
7619 case FSCTL_CREATE_OR_GET_OBJECT_ID:
7620 {
7621 struct file_object_buf_type1_ioctl_rsp *obj_buf;
7622
7623 nbytes = sizeof(struct file_object_buf_type1_ioctl_rsp);
7624 obj_buf = (struct file_object_buf_type1_ioctl_rsp *)
7625 &rsp->Buffer[0];
7626
7627 /*
7628 * TODO: This is dummy implementation to pass smbtorture
7629 * Need to check correct response later
7630 */
7631 memset(obj_buf->ObjectId, 0x0, 16);
7632 memset(obj_buf->BirthVolumeId, 0x0, 16);
7633 memset(obj_buf->BirthObjectId, 0x0, 16);
7634 memset(obj_buf->DomainId, 0x0, 16);
7635
7636 break;
7637 }
7638 case FSCTL_PIPE_TRANSCEIVE:
7639 out_buf_len = min_t(u32, KSMBD_IPC_MAX_PAYLOAD, out_buf_len);
7640 nbytes = fsctl_pipe_transceive(work, id, out_buf_len, req, rsp);
7641 break;
7642 case FSCTL_VALIDATE_NEGOTIATE_INFO:
7643 if (conn->dialect < SMB30_PROT_ID) {
7644 ret = -EOPNOTSUPP;
7645 goto out;
7646 }
7647
7648 if (in_buf_len < offsetof(struct validate_negotiate_info_req,
7649 Dialects)) {
7650 ret = -EINVAL;
7651 goto out;
7652 }
7653
7654 if (out_buf_len < sizeof(struct validate_negotiate_info_rsp)) {
7655 ret = -EINVAL;
7656 goto out;
7657 }
7658
7659 ret = fsctl_validate_negotiate_info(conn,
7660 (struct validate_negotiate_info_req *)&req->Buffer[0],
7661 (struct validate_negotiate_info_rsp *)&rsp->Buffer[0],
7662 in_buf_len);
7663 if (ret < 0)
7664 goto out;
7665
7666 nbytes = sizeof(struct validate_negotiate_info_rsp);
7667 rsp->PersistentFileId = SMB2_NO_FID;
7668 rsp->VolatileFileId = SMB2_NO_FID;
7669 break;
7670 case FSCTL_QUERY_NETWORK_INTERFACE_INFO:
7671 ret = fsctl_query_iface_info_ioctl(conn, rsp, out_buf_len);
7672 if (ret < 0)
7673 goto out;
7674 nbytes = ret;
7675 break;
7676 case FSCTL_REQUEST_RESUME_KEY:
7677 if (out_buf_len < sizeof(struct resume_key_ioctl_rsp)) {
7678 ret = -EINVAL;
7679 goto out;
7680 }
7681
7682 ret = fsctl_request_resume_key(work, req,
7683 (struct resume_key_ioctl_rsp *)&rsp->Buffer[0]);
7684 if (ret < 0)
7685 goto out;
7686 rsp->PersistentFileId = req->PersistentFileId;
7687 rsp->VolatileFileId = req->VolatileFileId;
7688 nbytes = sizeof(struct resume_key_ioctl_rsp);
7689 break;
7690 case FSCTL_COPYCHUNK:
7691 case FSCTL_COPYCHUNK_WRITE:
7692 if (!test_tree_conn_flag(work->tcon, KSMBD_TREE_CONN_FLAG_WRITABLE)) {
7693 ksmbd_debug(SMB,
7694 "User does not have write permission\n");
7695 ret = -EACCES;
7696 goto out;
7697 }
7698
7699 if (in_buf_len < sizeof(struct copychunk_ioctl_req)) {
7700 ret = -EINVAL;
7701 goto out;
7702 }
7703
7704 if (out_buf_len < sizeof(struct copychunk_ioctl_rsp)) {
7705 ret = -EINVAL;
7706 goto out;
7707 }
7708
7709 nbytes = sizeof(struct copychunk_ioctl_rsp);
7710 rsp->VolatileFileId = req->VolatileFileId;
7711 rsp->PersistentFileId = req->PersistentFileId;
7712 fsctl_copychunk(work,
7713 (struct copychunk_ioctl_req *)&req->Buffer[0],
7714 le32_to_cpu(req->CtlCode),
7715 le32_to_cpu(req->InputCount),
7716 req->VolatileFileId,
7717 req->PersistentFileId,
7718 rsp);
7719 break;
7720 case FSCTL_SET_SPARSE:
7721 if (in_buf_len < sizeof(struct file_sparse)) {
7722 ret = -EINVAL;
7723 goto out;
7724 }
7725
7726 ret = fsctl_set_sparse(work, id,
7727 (struct file_sparse *)&req->Buffer[0]);
7728 if (ret < 0)
7729 goto out;
7730 break;
7731 case FSCTL_SET_ZERO_DATA:
7732 {
7733 struct file_zero_data_information *zero_data;
7734 struct ksmbd_file *fp;
7735 loff_t off, len, bfz;
7736
7737 if (!test_tree_conn_flag(work->tcon, KSMBD_TREE_CONN_FLAG_WRITABLE)) {
7738 ksmbd_debug(SMB,
7739 "User does not have write permission\n");
7740 ret = -EACCES;
7741 goto out;
7742 }
7743
7744 if (in_buf_len < sizeof(struct file_zero_data_information)) {
7745 ret = -EINVAL;
7746 goto out;
7747 }
7748
7749 zero_data =
7750 (struct file_zero_data_information *)&req->Buffer[0];
7751
7752 off = le64_to_cpu(zero_data->FileOffset);
7753 bfz = le64_to_cpu(zero_data->BeyondFinalZero);
7754 if (off > bfz) {
Should we check for negative values of "off"? To be honest,
Smatch doesn't really trust either off or bfz...
7755 ret = -EINVAL;
7756 goto out;
7757 }
7758
--> 7759 len = bfz - off;
7760 if (len) {
7761 fp = ksmbd_lookup_fd_fast(work, id);
7762 if (!fp) {
7763 ret = -ENOENT;
7764 goto out;
7765 }
7766
7767 ret = ksmbd_vfs_zero_data(work, fp, off, len);
7768 ksmbd_fd_put(work, fp);
7769 if (ret < 0)
7770 goto out;
7771 }
7772 break;
7773 }
regards,
dan carpenter
