* SG_DXFER_TO_FROM_DEV does not copy user buffer to driver buffer in linux 2.6.28 and later?
@ 2009-06-29 0:50 zhou sf
2009-06-29 11:41 ` Douglas Gilbert
0 siblings, 1 reply; 4+ messages in thread
From: zhou sf @ 2009-06-29 0:50 UTC (permalink / raw)
To: linux-scsi
Test with the following program, and dump the data buf at queuecommand
of the driver, found the data is something like:
"00 00 00 00 ff 53 4d 42 2e 00 00 00 00 80 01 c0 00 00 00 00 00 00 00
00 00 00 00 00 02 20 4a ..."
While before 2.6.28, it is "ec ec ec ec ..." as expected.
#include <string.h>
#include <fcntl.h>
#include <sys/ioctl.h>
#include <scsi/sg.h>
int main(int argc, char* argv[])
{
int fd;
sg_io_hdr_t io_hdr;
int ret;
unsigned char sensebuf[32], cdb[16], outbuf[256];
if ((argc!=2) || (fd = open(argv[1], O_RDWR)) < 0) {
return -1;
}
memset(&io_hdr, 0, sizeof(sg_io_hdr_t));
memset(cdb, 0, sizeof(cdb));
memset(outbuf, 0xec, sizeof(outbuf));
memset(sensebuf, 0xec, sizeof(sensebuf));
io_hdr.interface_id = 'S';
io_hdr.cmdp = cdb;
io_hdr.cmd_len = sizeof(cdb);
io_hdr.sbp = sensebuf;
io_hdr.mx_sb_len = sizeof(sensebuf);
io_hdr.dxferp = outbuf;
io_hdr.dxfer_len = sizeof(outbuf);
io_hdr.dxfer_direction = SG_DXFER_TO_FROM_DEV;
io_hdr.timeout = 20000;
ret = ioctl(fd, SG_IO, &io_hdr);
return 0;
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: SG_DXFER_TO_FROM_DEV does not copy user buffer to driver buffer in linux 2.6.28 and later?
2009-06-29 0:50 SG_DXFER_TO_FROM_DEV does not copy user buffer to driver buffer in linux 2.6.28 and later? zhou sf
@ 2009-06-29 11:41 ` Douglas Gilbert
2009-06-30 6:45 ` FUJITA Tomonori
0 siblings, 1 reply; 4+ messages in thread
From: Douglas Gilbert @ 2009-06-29 11:41 UTC (permalink / raw)
To: zhou sf; +Cc: linux-scsi, FUJITA Tomonori
The semantics of SG_DXFER_TO_FROM_DEV were:
- copy user space buffer to kernel (LLD) buffer
- do SCSI command which is assumed to be of the DATA_IN
(data from device) variety. This would overwrite
some or all of the kernel buffer
- copy kernel (LLD) buffer back to the user space.
The idea was to detect short reads by filling the original
user space buffer with some marker bytes ("0xec" it would
seem in this report). The "resid" value is a better way
of detecting short reads but that was only added this century
and requires co-operation from the LLD.
There were major plumbing changes in the sg driver around
lk 2.6.28 and it looks like the above nuance was lost.
Perhaps the block layer does support the above "double
shuffle".
Doug Gilbert
cc-ed to Tomo for comments
zhou sf wrote:
> Test with the following program, and dump the data buf at queuecommand
> of the driver, found the data is something like:
> "00 00 00 00 ff 53 4d 42 2e 00 00 00 00 80 01 c0 00 00 00 00 00 00 00
> 00 00 00 00 00 02 20 4a ..."
> While before 2.6.28, it is "ec ec ec ec ..." as expected.
>
>
> #include <string.h>
> #include <fcntl.h>
> #include <sys/ioctl.h>
> #include <scsi/sg.h>
>
> int main(int argc, char* argv[])
> {
> int fd;
> sg_io_hdr_t io_hdr;
> int ret;
>
> unsigned char sensebuf[32], cdb[16], outbuf[256];
>
> if ((argc!=2) || (fd = open(argv[1], O_RDWR)) < 0) {
> return -1;
> }
>
> memset(&io_hdr, 0, sizeof(sg_io_hdr_t));
> memset(cdb, 0, sizeof(cdb));
> memset(outbuf, 0xec, sizeof(outbuf));
> memset(sensebuf, 0xec, sizeof(sensebuf));
>
> io_hdr.interface_id = 'S';
>
> io_hdr.cmdp = cdb;
> io_hdr.cmd_len = sizeof(cdb);
>
> io_hdr.sbp = sensebuf;
> io_hdr.mx_sb_len = sizeof(sensebuf);
>
> io_hdr.dxferp = outbuf;
> io_hdr.dxfer_len = sizeof(outbuf);
> io_hdr.dxfer_direction = SG_DXFER_TO_FROM_DEV;
>
> io_hdr.timeout = 20000;
>
> ret = ioctl(fd, SG_IO, &io_hdr);
> return 0;
> }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: SG_DXFER_TO_FROM_DEV does not copy user buffer to driver buffer in linux 2.6.28 and later?
2009-06-29 11:41 ` Douglas Gilbert
@ 2009-06-30 6:45 ` FUJITA Tomonori
2009-06-30 7:56 ` zhou sf
0 siblings, 1 reply; 4+ messages in thread
From: FUJITA Tomonori @ 2009-06-30 6:45 UTC (permalink / raw)
To: dgilbert; +Cc: sxzzsf, linux-scsi, fujita.tomonori
On Mon, 29 Jun 2009 13:41:20 +0200
Douglas Gilbert <dgilbert@interlog.com> wrote:
> The semantics of SG_DXFER_TO_FROM_DEV were:
> - copy user space buffer to kernel (LLD) buffer
> - do SCSI command which is assumed to be of the DATA_IN
> (data from device) variety. This would overwrite
> some or all of the kernel buffer
> - copy kernel (LLD) buffer back to the user space.
>
> The idea was to detect short reads by filling the original
> user space buffer with some marker bytes ("0xec" it would
> seem in this report). The "resid" value is a better way
> of detecting short reads but that was only added this century
> and requires co-operation from the LLD.
>
> There were major plumbing changes in the sg driver around
> lk 2.6.28 and it looks like the above nuance was lost.
> Perhaps the block layer does support the above "double
> shuffle".
Sorry, I overlooked SG_DXFER_TO_FROM_DEV.
Does this patch work?
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 8201387..8df3b6a 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1658,6 +1658,10 @@ static int sg_start_req(Sg_request *srp, unsigned char *cmd)
md->nr_entries = req_schp->k_use_sg;
md->offset = 0;
md->null_mapped = hp->dxferp ? 0 : 1;
+ if (dxfer_dir == SG_DXFER_TO_FROM_DEV)
+ md->from_user = 1;
+ else
+ md->from_user = 0;
}
if (iov_count) {
diff --git a/fs/bio.c b/fs/bio.c
index 24c9140..e467dd8 100644
--- a/fs/bio.c
+++ b/fs/bio.c
@@ -705,14 +705,13 @@ static struct bio_map_data *bio_alloc_map_data(int nr_segs, int iov_count,
}
static int __bio_copy_iov(struct bio *bio, struct bio_vec *iovecs,
- struct sg_iovec *iov, int iov_count, int uncopy,
- int do_free_page)
+ struct sg_iovec *iov, int iov_count,
+ int to_user, int from_user, int do_free_page)
{
int ret = 0, i;
struct bio_vec *bvec;
int iov_idx = 0;
unsigned int iov_off = 0;
- int read = bio_data_dir(bio) == READ;
__bio_for_each_segment(bvec, bio, i, 0) {
char *bv_addr = page_address(bvec->bv_page);
@@ -727,13 +726,14 @@ static int __bio_copy_iov(struct bio *bio, struct bio_vec *iovecs,
iov_addr = iov[iov_idx].iov_base + iov_off;
if (!ret) {
- if (!read && !uncopy)
- ret = copy_from_user(bv_addr, iov_addr,
- bytes);
- if (read && uncopy)
+ if (to_user)
ret = copy_to_user(iov_addr, bv_addr,
bytes);
+ if (from_user)
+ ret = copy_from_user(bv_addr, iov_addr,
+ bytes);
+
if (ret)
ret = -EFAULT;
}
@@ -770,7 +770,8 @@ int bio_uncopy_user(struct bio *bio)
if (!bio_flagged(bio, BIO_NULL_MAPPED))
ret = __bio_copy_iov(bio, bmd->iovecs, bmd->sgvecs,
- bmd->nr_sgvecs, 1, bmd->is_our_pages);
+ bmd->nr_sgvecs, bio_data_dir(bio) == READ,
+ 0, bmd->is_our_pages);
bio_free_map_data(bmd);
bio_put(bio);
return ret;
@@ -875,8 +876,9 @@ struct bio *bio_copy_user_iov(struct request_queue *q,
/*
* success
*/
- if (!write_to_vm && (!map_data || !map_data->null_mapped)) {
- ret = __bio_copy_iov(bio, bio->bi_io_vec, iov, iov_count, 0, 0);
+ if ((!write_to_vm && (!map_data || !map_data->null_mapped)) ||
+ (map_data && map_data->from_user)) {
+ ret = __bio_copy_iov(bio, bio->bi_io_vec, iov, iov_count, 0, 1, 0);
if (ret)
goto cleanup;
}
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 8963d91..36c5ff8 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -730,6 +730,7 @@ struct rq_map_data {
int nr_entries;
unsigned long offset;
int null_mapped;
+ int from_user;
};
struct req_iterator {
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: SG_DXFER_TO_FROM_DEV does not copy user buffer to driver buffer in linux 2.6.28 and later?
2009-06-30 6:45 ` FUJITA Tomonori
@ 2009-06-30 7:56 ` zhou sf
0 siblings, 0 replies; 4+ messages in thread
From: zhou sf @ 2009-06-30 7:56 UTC (permalink / raw)
To: FUJITA Tomonori; +Cc: dgilbert, linux-scsi
It works under 2.6.28.
2009/6/30 FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>:
> On Mon, 29 Jun 2009 13:41:20 +0200
> Douglas Gilbert <dgilbert@interlog.com> wrote:
>
>> The semantics of SG_DXFER_TO_FROM_DEV were:
>> - copy user space buffer to kernel (LLD) buffer
>> - do SCSI command which is assumed to be of the DATA_IN
>> (data from device) variety. This would overwrite
>> some or all of the kernel buffer
>> - copy kernel (LLD) buffer back to the user space.
>>
>> The idea was to detect short reads by filling the original
>> user space buffer with some marker bytes ("0xec" it would
>> seem in this report). The "resid" value is a better way
>> of detecting short reads but that was only added this century
>> and requires co-operation from the LLD.
>>
>> There were major plumbing changes in the sg driver around
>> lk 2.6.28 and it looks like the above nuance was lost.
>> Perhaps the block layer does support the above "double
>> shuffle".
>
> Sorry, I overlooked SG_DXFER_TO_FROM_DEV.
>
> Does this patch work?
>
>
> diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
> index 8201387..8df3b6a 100644
> --- a/drivers/scsi/sg.c
> +++ b/drivers/scsi/sg.c
> @@ -1658,6 +1658,10 @@ static int sg_start_req(Sg_request *srp, unsigned char *cmd)
> md->nr_entries = req_schp->k_use_sg;
> md->offset = 0;
> md->null_mapped = hp->dxferp ? 0 : 1;
> + if (dxfer_dir == SG_DXFER_TO_FROM_DEV)
> + md->from_user = 1;
> + else
> + md->from_user = 0;
> }
>
> if (iov_count) {
> diff --git a/fs/bio.c b/fs/bio.c
> index 24c9140..e467dd8 100644
> --- a/fs/bio.c
> +++ b/fs/bio.c
> @@ -705,14 +705,13 @@ static struct bio_map_data *bio_alloc_map_data(int nr_segs, int iov_count,
> }
>
> static int __bio_copy_iov(struct bio *bio, struct bio_vec *iovecs,
> - struct sg_iovec *iov, int iov_count, int uncopy,
> - int do_free_page)
> + struct sg_iovec *iov, int iov_count,
> + int to_user, int from_user, int do_free_page)
> {
> int ret = 0, i;
> struct bio_vec *bvec;
> int iov_idx = 0;
> unsigned int iov_off = 0;
> - int read = bio_data_dir(bio) == READ;
>
> __bio_for_each_segment(bvec, bio, i, 0) {
> char *bv_addr = page_address(bvec->bv_page);
> @@ -727,13 +726,14 @@ static int __bio_copy_iov(struct bio *bio, struct bio_vec *iovecs,
> iov_addr = iov[iov_idx].iov_base + iov_off;
>
> if (!ret) {
> - if (!read && !uncopy)
> - ret = copy_from_user(bv_addr, iov_addr,
> - bytes);
> - if (read && uncopy)
> + if (to_user)
> ret = copy_to_user(iov_addr, bv_addr,
> bytes);
>
> + if (from_user)
> + ret = copy_from_user(bv_addr, iov_addr,
> + bytes);
> +
> if (ret)
> ret = -EFAULT;
> }
> @@ -770,7 +770,8 @@ int bio_uncopy_user(struct bio *bio)
>
> if (!bio_flagged(bio, BIO_NULL_MAPPED))
> ret = __bio_copy_iov(bio, bmd->iovecs, bmd->sgvecs,
> - bmd->nr_sgvecs, 1, bmd->is_our_pages);
> + bmd->nr_sgvecs, bio_data_dir(bio) == READ,
> + 0, bmd->is_our_pages);
> bio_free_map_data(bmd);
> bio_put(bio);
> return ret;
> @@ -875,8 +876,9 @@ struct bio *bio_copy_user_iov(struct request_queue *q,
> /*
> * success
> */
> - if (!write_to_vm && (!map_data || !map_data->null_mapped)) {
> - ret = __bio_copy_iov(bio, bio->bi_io_vec, iov, iov_count, 0, 0);
> + if ((!write_to_vm && (!map_data || !map_data->null_mapped)) ||
> + (map_data && map_data->from_user)) {
> + ret = __bio_copy_iov(bio, bio->bi_io_vec, iov, iov_count, 0, 1, 0);
> if (ret)
> goto cleanup;
> }
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 8963d91..36c5ff8 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -730,6 +730,7 @@ struct rq_map_data {
> int nr_entries;
> unsigned long offset;
> int null_mapped;
> + int from_user;
> };
>
> struct req_iterator {
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-06-30 7:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-06-29 0:50 SG_DXFER_TO_FROM_DEV does not copy user buffer to driver buffer in linux 2.6.28 and later? zhou sf
2009-06-29 11:41 ` Douglas Gilbert
2009-06-30 6:45 ` FUJITA Tomonori
2009-06-30 7:56 ` zhou sf
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).