All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Al Viro <viro@zeniv.linux.org.uk>
Cc: Jens Axboe <axboe@fb.com>,
	Nicholas Bellinger <nab@linux-iscsi.org>,
	Ming Lei <ming.lei@canonical.com>,
	linux-fsdevel@vger.kernel.org, target-devel@vger.kernel.org
Subject: [PATCH 5/5] target: rewrite fd_execute_write_same
Date: Sun, 25 Jan 2015 21:12:02 +0100	[thread overview]
Message-ID: <1422216722-27786-6-git-send-email-hch@lst.de> (raw)
In-Reply-To: <1422216722-27786-1-git-send-email-hch@lst.de>

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 <hch@lst.de>
---
 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


  parent reply	other threads:[~2015-01-25 20:13 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-25 20:11 switch loop and target to use ITER_BVEC iov_iter V2 Christoph Hellwig
2015-01-25 20:11 ` [PATCH 1/5] new helper: iov_iter_bvec() Christoph Hellwig
2015-01-25 20:11 ` [PATCH 2/5] fs: add vfs_iter_{read,write} helpers Christoph Hellwig
2015-01-25 20:12 ` [PATCH 3/5] loop: convert to vfs_iter_read/write Christoph Hellwig
2015-01-25 20:12 ` [PATCH 4/5] target: use vfs_iter_read/write in fd_do_rw Christoph Hellwig
2015-01-25 20:12 ` Christoph Hellwig [this message]
2015-01-26  5:10 ` switch loop and target to use ITER_BVEC iov_iter V2 Al Viro

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=1422216722-27786-6-git-send-email-hch@lst.de \
    --to=hch@lst.de \
    --cc=axboe@fb.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=ming.lei@canonical.com \
    --cc=nab@linux-iscsi.org \
    --cc=target-devel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    --subject='Re: [PATCH 5/5] target: rewrite fd_execute_write_same' \
    /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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.