* 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; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ messages in thread
[parent not found: <42a6d9690906260128q46c092aqf29aa6d6e77b9ccb@mail.gmail.com>]
* SG_DXFER_TO_FROM_DEV does not copy user buffer to driver buffer in linux 2.6.28 and later? [not found] <42a6d9690906260128q46c092aqf29aa6d6e77b9ccb@mail.gmail.com> @ 2009-06-26 8:39 ` zhou sf 2009-06-26 8:40 ` zhou sf 1 sibling, 0 replies; 6+ messages in thread From: zhou sf @ 2009-06-26 8:39 UTC (permalink / raw) To: linux-kernel ---------- Forwarded message ---------- From: zhou sf <sxzzsf@gmail.com> Date: 2009/6/26 Subject: SG_DXFER_TO_FROM_DEV does not copy user buffer to driver buffer in linux 2.6.28 and later? To: majordomo@vger.kernel.org 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] 6+ messages in thread
* SG_DXFER_TO_FROM_DEV does not copy user buffer to driver buffer in linux 2.6.28 and later? [not found] <42a6d9690906260128q46c092aqf29aa6d6e77b9ccb@mail.gmail.com> 2009-06-26 8:39 ` zhou sf @ 2009-06-26 8:40 ` zhou sf 1 sibling, 0 replies; 6+ messages in thread From: zhou sf @ 2009-06-26 8:40 UTC (permalink / raw) To: linux-kernel 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] 6+ messages in thread
end of thread, other threads:[~2009-06-30 7:56 UTC | newest]
Thread overview: 6+ 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
[not found] <42a6d9690906260128q46c092aqf29aa6d6e77b9ccb@mail.gmail.com>
2009-06-26 8:39 ` zhou sf
2009-06-26 8:40 ` zhou sf
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.