From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoph Hellwig Subject: [PATCH 5/5] target: rewrite fd_execute_write_same Date: Sun, 25 Jan 2015 21:12:02 +0100 Message-ID: <1422216722-27786-6-git-send-email-hch@lst.de> References: <1422216722-27786-1-git-send-email-hch@lst.de> Cc: Jens Axboe , Nicholas Bellinger , Ming Lei , linux-fsdevel@vger.kernel.org, target-devel@vger.kernel.org To: Al Viro Return-path: Received: from casper.infradead.org ([85.118.1.10]:57466 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932395AbbAYUNR (ORCPT ); Sun, 25 Jan 2015 15:13:17 -0500 In-Reply-To: <1422216722-27786-1-git-send-email-hch@lst.de> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: With the new bio_vec backed iov_iter helpers we can simply set up on bio_vec per LBA, all pointing to the same initiator-supplied buffer. Btw, can someone check if the potential overflow for the total length is real, or if there is something higher level preventing it? This sounds like it might be exploitable. Signed-off-by: Christoph Hellwig --- drivers/target/target_core_file.c | 99 +++++++++------------------------------ 1 file changed, 22 insertions(+), 77 deletions(-) diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c index 36ea19a..c8fa4a1 100644 --- a/drivers/target/target_core_file.c +++ b/drivers/target/target_core_file.c @@ -433,107 +433,52 @@ fd_execute_sync_cache(struct se_cmd *cmd) return 0; } -static unsigned char * -fd_setup_write_same_buf(struct se_cmd *cmd, struct scatterlist *sg, - unsigned int len) -{ - struct se_device *se_dev = cmd->se_dev; - unsigned int block_size = se_dev->dev_attrib.block_size; - unsigned int i = 0, end; - unsigned char *buf, *p, *kmap_buf; - - buf = kzalloc(min_t(unsigned int, len, PAGE_SIZE), GFP_KERNEL); - if (!buf) { - pr_err("Unable to allocate fd_execute_write_same buf\n"); - return NULL; - } - - kmap_buf = kmap(sg_page(sg)) + sg->offset; - if (!kmap_buf) { - pr_err("kmap() failed in fd_setup_write_same\n"); - kfree(buf); - return NULL; - } - /* - * Fill local *buf to contain multiple WRITE_SAME blocks up to - * min(len, PAGE_SIZE) - */ - p = buf; - end = min_t(unsigned int, len, PAGE_SIZE); - - while (i < end) { - memcpy(p, kmap_buf, block_size); - - i += block_size; - p += block_size; - } - kunmap(sg_page(sg)); - - return buf; -} - static sense_reason_t fd_execute_write_same(struct se_cmd *cmd) { struct se_device *se_dev = cmd->se_dev; struct fd_dev *fd_dev = FD_DEV(se_dev); - struct file *f = fd_dev->fd_file; - struct scatterlist *sg; - struct iovec *iov; - mm_segment_t old_fs; - sector_t nolb = sbc_get_write_same_sectors(cmd); loff_t pos = cmd->t_task_lba * se_dev->dev_attrib.block_size; - unsigned int len, len_tmp, iov_num; - int i, rc; - unsigned char *buf; + sector_t nolb = sbc_get_write_same_sectors(cmd); + struct iov_iter iter; + struct bio_vec *bvec; + unsigned int len, i; + ssize_t ret; if (!nolb) { target_complete_cmd(cmd, SAM_STAT_GOOD); return 0; } - sg = &cmd->t_data_sg[0]; if (cmd->t_data_nents > 1 || - sg->length != cmd->se_dev->dev_attrib.block_size) { + cmd->t_data_sg[0].length != cmd->se_dev->dev_attrib.block_size) { pr_err("WRITE_SAME: Illegal SGL t_data_nents: %u length: %u" - " block_size: %u\n", cmd->t_data_nents, sg->length, + " block_size: %u\n", + cmd->t_data_nents, + cmd->t_data_sg[0].length, cmd->se_dev->dev_attrib.block_size); return TCM_INVALID_CDB_FIELD; } - len = len_tmp = nolb * se_dev->dev_attrib.block_size; - iov_num = DIV_ROUND_UP(len, PAGE_SIZE); + /* XXX: looks like this could overflow */ + len = nolb * se_dev->dev_attrib.block_size; - buf = fd_setup_write_same_buf(cmd, sg, len); - if (!buf) + bvec = kcalloc(nolb, sizeof(struct bio_vec), GFP_KERNEL); + if (!bvec) return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE; - iov = vzalloc(sizeof(struct iovec) * iov_num); - if (!iov) { - pr_err("Unable to allocate fd_execute_write_same iovecs\n"); - kfree(buf); - return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE; + for (i = 0; i < nolb; i++) { + bvec[i].bv_page = sg_page(&cmd->t_data_sg[0]); + bvec[i].bv_len = cmd->t_data_sg[0].length; + bvec[i].bv_offset = cmd->t_data_sg[0].offset; } - /* - * Map the single fabric received scatterlist block now populated - * in *buf into each iovec for I/O submission. - */ - for (i = 0; i < iov_num; i++) { - iov[i].iov_base = buf; - iov[i].iov_len = min_t(unsigned int, len_tmp, PAGE_SIZE); - len_tmp -= iov[i].iov_len; - } - - old_fs = get_fs(); - set_fs(get_ds()); - rc = vfs_writev(f, &iov[0], iov_num, &pos); - set_fs(old_fs); - vfree(iov); - kfree(buf); + iov_iter_bvec(&iter, ITER_BVEC, bvec, nolb, len); + ret = vfs_iter_write(fd_dev->fd_file, &iter, &pos); - if (rc < 0 || rc != len) { - pr_err("vfs_writev() returned %d for write same\n", rc); + kfree(bvec); + if (ret < 0 || ret != len) { + pr_err("vfs_iter_write() returned %zd for write same\n", ret); return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE; } -- 1.9.1