Re: [RFC PATCH 1/6] bsg: fix kernel panic resulting from missing allocation of a reply-buffer
On Fri, Aug 11, 2017 at 05:35:53PM +0200, Christoph Hellwig wrote:
> On Fri, Aug 11, 2017 at 05:32:03PM +0200, Benjamin Block wrote:
> > So when the bsg interface is used with something different than the
> > bsg-lib request queue?
>
> Yes.
>
> > I haven't actually thought about that (presuming
> > the bsg-lib queue was the only one being used). Fair enough, I haven't
> > completely read that code now, but that seems bad then, to reassign a
> > space allocated in someone else's request queue.
> >
> > That still leaves open that we now over-allocate space in bsg-lib, or?
>
> Which space do we over-allocate?
When the BSG interface is used with bsg-lib, and the user sends a
Bidirectional command - so when he gives an input- and output-buffer
(most users of our interface will likely do that, if they wanna get the
transport-level response data) - bsg will allocate two requests from the
queue. The first request's bio is used to map the input and the second
request's bio for the output (see bsg_map_hdr() in the if-statement with
(op == REQ_OP_SCSI_OUT && hdr->din_xfer_len)).
When we now allocate the full space of bsg_job, sense, dd_data for each
request, these will be wasted on the (linked) second request. They will
go unused all the time, as only the first request's bsg_job, sense and
dd_data is used by the LLDs and BSG itself.
Right now, because we don't allocate this on each request, those spaces
are only allocated for the first request in bsg-lib.
Maybe we can ignore this, if it gets to complicated, I don't wanne
prolong this unnecessary.
>
> > My diff tells that this was the same patch as before.
>
> Next try:
I will have a look when I am back in the office next week.
Beste Grüße / Best regards,
- Benjamin Block
>
> ---
> From f5b03b82df0569c035022c1c2535696186907f1a Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig
> Date: Fri, 11 Aug 2017 11:03:29 +0200
> Subject: bsg-lib: allocate sense data for each request
>
> Since we split the scsi_request out of the request the driver is supposed
> to provide storage for the sense buffer. The bsg-lib code failed to do so,
> though and will crash anytime it is used.
>
> This patch moves bsg-lib to allocate and setup the bsg_job ahead of time,
> and allocate the sense data, which is used as reply buffer in bsg.
>
> Reported-by: Steffen Maier
> Signed-off-by: Benjamin Block
> Fixes: 82ed4db499b8 ("block: split scsi_request out of struct request")
> Cc: #4.11+
> ---
> block/bsg-lib.c | 51
> +++--
> include/linux/blkdev.h | 1 -
> include/linux/bsg-lib.h | 2 ++
> 3 files changed, 30 insertions(+), 24 deletions(-)
>
> diff --git a/block/bsg-lib.c b/block/bsg-lib.c
> index c4513b23f57a..c07333c3b785 100644
> --- a/block/bsg-lib.c
> +++ b/block/bsg-lib.c
> @@ -37,13 +37,11 @@ static void bsg_destroy_job(struct kref *kref)
> struct bsg_job *job = container_of(kref, struct bsg_job, kref);
> struct request *rq = job->req;
>
> - blk_end_request_all(rq, BLK_STS_OK);
> -
> put_device(job->dev); /* release reference for the request */
>
> kfree(job->request_payload.sg_list);
> kfree(job->reply_payload.sg_list);
> - kfree(job);
> + blk_end_request_all(rq, BLK_STS_OK);
> }
>
> void bsg_job_put(struct bsg_job *job)
> @@ -100,7 +98,7 @@ EXPORT_SYMBOL_GPL(bsg_job_done);
> */
> static void bsg_softirq_done(struct request *rq)
> {
> - struct bsg_job *job = rq->special;
> + struct bsg_job *job = blk_mq_rq_to_pdu(rq);
>
> bsg_job_put(job);
> }
> @@ -129,24 +127,11 @@ static int bsg_map_buffer(struct bsg_buffer *buf,
> struct request *req)
> static int bsg_create_job(struct device *dev, struct request *req)
> {
> struct request *rsp = req->next_rq;
> - struct request_queue *q = req->q;
> + struct bsg_job *job = blk_mq_rq_to_pdu(req);
> struct scsi_request *rq = scsi_req(req);
> - struct bsg_job *job;
> int ret;
>
> - BUG_ON(req->special);
> -
> - job = kzalloc(sizeof(struct bsg_job) + q->bsg_job_size, GFP_KERNEL);
> - if (!job)
> - return -ENOMEM;
> -
> - req->special = job;
> - job->req = req;
> - if (q->bsg_job_size)
> - job->dd_data = (void *)&job[1];
> - job->request = rq->cmd;
> job->request_len = rq->cmd_len;
> - job->reply = rq->sense;
> job->reply_len = SCSI_SENSE_BUFFERSIZE; /* Size of sense buffer
>* allocated */
> if (req->bio) {
> @@ -187,7 +172,6 @@ static void bsg_request_fn(struct request_queue *q)
> {
> struct device *dev = q->queuedata;
> struct request *req;
> - struct bsg_job *job;
> int ret;
>
> if (!get_device(dev))
> @@ -207,8 +191,7 @@ static void bsg_request_fn(struct request_queue *q)
> continue;
>
Re: [RFC PATCH 1/6] bsg: fix kernel panic resulting from missing allocation of a reply-buffer
On Fri, Aug 11, 2017 at 05:32:03PM +0200, Benjamin Block wrote:
> So when the bsg interface is used with something different than the
> bsg-lib request queue?
Yes.
> I haven't actually thought about that (presuming
> the bsg-lib queue was the only one being used). Fair enough, I haven't
> completely read that code now, but that seems bad then, to reassign a
> space allocated in someone else's request queue.
>
> That still leaves open that we now over-allocate space in bsg-lib, or?
Which space do we over-allocate?
> My diff tells that this was the same patch as before.
Next try:
---
>From f5b03b82df0569c035022c1c2535696186907f1a Mon Sep 17 00:00:00 2001
From: Christoph Hellwig
Date: Fri, 11 Aug 2017 11:03:29 +0200
Subject: bsg-lib: allocate sense data for each request
Since we split the scsi_request out of the request the driver is supposed
to provide storage for the sense buffer. The bsg-lib code failed to do so,
though and will crash anytime it is used.
This patch moves bsg-lib to allocate and setup the bsg_job ahead of time,
and allocate the sense data, which is used as reply buffer in bsg.
Reported-by: Steffen Maier
Signed-off-by: Benjamin Block
Fixes: 82ed4db499b8 ("block: split scsi_request out of struct request")
Cc: #4.11+
---
block/bsg-lib.c | 51 +++--
include/linux/blkdev.h | 1 -
include/linux/bsg-lib.h | 2 ++
3 files changed, 30 insertions(+), 24 deletions(-)
diff --git a/block/bsg-lib.c b/block/bsg-lib.c
index c4513b23f57a..c07333c3b785 100644
--- a/block/bsg-lib.c
+++ b/block/bsg-lib.c
@@ -37,13 +37,11 @@ static void bsg_destroy_job(struct kref *kref)
struct bsg_job *job = container_of(kref, struct bsg_job, kref);
struct request *rq = job->req;
- blk_end_request_all(rq, BLK_STS_OK);
-
put_device(job->dev); /* release reference for the request */
kfree(job->request_payload.sg_list);
kfree(job->reply_payload.sg_list);
- kfree(job);
+ blk_end_request_all(rq, BLK_STS_OK);
}
void bsg_job_put(struct bsg_job *job)
@@ -100,7 +98,7 @@ EXPORT_SYMBOL_GPL(bsg_job_done);
*/
static void bsg_softirq_done(struct request *rq)
{
- struct bsg_job *job = rq->special;
+ struct bsg_job *job = blk_mq_rq_to_pdu(rq);
bsg_job_put(job);
}
@@ -129,24 +127,11 @@ static int bsg_map_buffer(struct bsg_buffer *buf, struct
request *req)
static int bsg_create_job(struct device *dev, struct request *req)
{
struct request *rsp = req->next_rq;
- struct request_queue *q = req->q;
+ struct bsg_job *job = blk_mq_rq_to_pdu(req);
struct scsi_request *rq = scsi_req(req);
- struct bsg_job *job;
int ret;
- BUG_ON(req->special);
-
- job = kzalloc(sizeof(struct bsg_job) + q->bsg_job_size, GFP_KERNEL);
- if (!job)
- return -ENOMEM;
-
- req->special = job;
- job->req = req;
- if (q->bsg_job_size)
- job->dd_data = (void *)&job[1];
- job->request = rq->cmd;
job->request_len = rq->cmd_len;
- job->reply = rq->sense;
job->reply_len = SCSI_SENSE_BUFFERSIZE; /* Size of sense buffer
* allocated */
if (req->bio) {
@@ -187,7 +172,6 @@ static void bsg_request_fn(struct request_queue *q)
{
struct device *dev = q->queuedata;
struct request *req;
- struct bsg_job *job;
int ret;
if (!get_device(dev))
@@ -207,8 +191,7 @@ static void bsg_request_fn(struct request_queue *q)
continue;
}
- job = req->special;
- ret = q->bsg_job_fn(job);
+ ret = q->bsg_job_fn(blk_mq_rq_to_pdu(req));
spin_lock_irq(q->queue_lock);
if (ret)
break;
@@ -219,6 +202,27 @@ static void bsg_request_fn(struct request_queue *q)
spin_lock_irq(q->queue_lock);
}
+static int bsg_init_rq(struct request_queue *q, struct request *req, gfp_t gfp)
+{
+ struct bsg_job *job = blk_mq_rq_to_pdu(req);
+
+ memset(job, 0, sizeof(*job));
+ job->req = req;
+ job->request = job->sreq.cmd;
+ job->dd_data = job + 1;
+ job->reply = job->sreq.sense = kzalloc(job->reply_len, gfp);
+ if (!job->reply)
+ return -ENOMEM;
+ return 0;
+}
+
+static void bsg_exit_rq(struct request_queue *q, struct request *req)
+{
+ struct bsg_job *job = blk_mq_rq_to_pdu(req);
+
+ kfree(job->reply);
+}
+
/**
* bsg_setup_queue - Create and add the bsg hooks so we can receive requests
* @dev: device to attach bsg device to
@@ -235,7 +239,9 @@ struct request_queue *bsg_setup_queue(struct device *dev,
char *name,
q = blk_alloc_queue(GFP_KERNEL);
if (!q)
return ERR_PTR(-ENOMEM);
- q->cmd_size = sizeof(struct scsi_request);
+ q->cmd_size = sizeof(struct bsg_job) + dd_job_size;
Re: [RFC PATCH 1/6] bsg: fix kernel panic resulting from missing allocation of a reply-buffer
On Fri, Aug 11, 2017 at 04:36:49PM +0200, Christoph Hellwig wrote: > On Fri, Aug 11, 2017 at 03:49:29PM +0200, Benjamin Block wrote: > > On Fri, Aug 11, 2017 at 11:14:15AM +0200, Christoph Hellwig wrote: > > > But patch 1 still creates an additional copy of the sense data for > > > all bsg users. > > > > > > > Huh? What additional copy? There is one reply-buffer and that is copied > > into the user-buffer should it contain valid data. Just like in your > > patch, neither you, nor me touches any of the copy-code. There is also > > no changes to how the driver get their data into that buffer, it will > > still be copied in both cases. > > You're right - I misread your patch. But that does make it worse as > this means that with your patch we re-assign the scsi_request.sense > pointer when using bsg. That will lead to crashes if using the bsg > code against e.g. a normal scsi device using bsg when that request > later gets reused for something that is not bsg. > So when the bsg interface is used with something different than the bsg-lib request queue? I haven't actually thought about that (presuming the bsg-lib queue was the only one being used). Fair enough, I haven't completely read that code now, but that seems bad then, to reassign a space allocated in someone else's request queue. That still leaves open that we now over-allocate space in bsg-lib, or? > > > > > > > > > Can you test the patch below which implements my suggestion? Your > > > other patches should still apply fine on top modulo minor context > > > changes. > > > > Only your patch on top of 4.13-rc4. din_xferp (D) is also empty, which is > > not taken from the sense-buffer. > > Can't parse this. > > > = > > BUG kmalloc-1024 (Not tainted): Invalid object pointer 0x4ad9e0f0 > > - > > Oops - if we don't allocate the job separately we should not free it either. > Updated patch for that below: > My diff tells that this was the same patch as before. Beste Grüße / Best regards, - Benjamin Block -- Linux on z Systems Development / IBM Systems & Technology Group IBM Deutschland Research & Development GmbH Vorsitz. AufsR.: Martina Koederitz /Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen / Registergericht: AmtsG Stuttgart, HRB 243294 -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To post to this group, send email to open-iscsi@googlegroups.com. Visit this group at https://groups.google.com/group/open-iscsi. For more options, visit https://groups.google.com/d/optout.
Re: [RFC PATCH 1/6] bsg: fix kernel panic resulting from missing allocation of a reply-buffer
On Fri, Aug 11, 2017 at 03:49:29PM +0200, Benjamin Block wrote:
> On Fri, Aug 11, 2017 at 11:14:15AM +0200, Christoph Hellwig wrote:
> > But patch 1 still creates an additional copy of the sense data for
> > all bsg users.
> >
>
> Huh? What additional copy? There is one reply-buffer and that is copied
> into the user-buffer should it contain valid data. Just like in your
> patch, neither you, nor me touches any of the copy-code. There is also
> no changes to how the driver get their data into that buffer, it will
> still be copied in both cases.
You're right - I misread your patch. But that does make it worse as
this means that with your patch we re-assign the scsi_request.sense
pointer when using bsg. That will lead to crashes if using the bsg
code against e.g. a normal scsi device using bsg when that request
later gets reused for something that is not bsg.
>
> >
> > Can you test the patch below which implements my suggestion? Your
> > other patches should still apply fine on top modulo minor context
> > changes.
>
> Only your patch on top of 4.13-rc4. din_xferp (D) is also empty, which is
> not taken from the sense-buffer.
Can't parse this.
> =
> BUG kmalloc-1024 (Not tainted): Invalid object pointer 0x4ad9e0f0
> -
Oops - if we don't allocate the job separately we should not free it either.
Updated patch for that below:
---
>From 4cd32ee48e334b62b55bff0d380833b978454040 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig
Date: Fri, 11 Aug 2017 11:03:29 +0200
Subject: bsg-lib: allocate sense data for each request
Since we split the scsi_request out of the request the driver is supposed
to provide storage for the sense buffer. The bsg-lib code failed to do so,
though and will crash anytime it is used.
This patch moves bsg-lib to allocate and setup the bsg_job ahead of time,
and allocate the sense data, which is used as reply buffer in bsg.
Reported-by: Steffen Maier
Signed-off-by: Benjamin Block
Fixes: 82ed4db499b8 ("block: split scsi_request out of struct request")
Cc: #4.11+
---
block/bsg-lib.c | 53 +++--
include/linux/blkdev.h | 1 -
include/linux/bsg-lib.h | 2 ++
3 files changed, 31 insertions(+), 25 deletions(-)
diff --git a/block/bsg-lib.c b/block/bsg-lib.c
index c4513b23f57a..215893dbd038 100644
--- a/block/bsg-lib.c
+++ b/block/bsg-lib.c
@@ -100,7 +100,7 @@ EXPORT_SYMBOL_GPL(bsg_job_done);
*/
static void bsg_softirq_done(struct request *rq)
{
- struct bsg_job *job = rq->special;
+ struct bsg_job *job = blk_mq_rq_to_pdu(rq);
bsg_job_put(job);
}
@@ -129,26 +129,9 @@ static int bsg_map_buffer(struct bsg_buffer *buf, struct
request *req)
static int bsg_create_job(struct device *dev, struct request *req)
{
struct request *rsp = req->next_rq;
- struct request_queue *q = req->q;
- struct scsi_request *rq = scsi_req(req);
- struct bsg_job *job;
+ struct bsg_job *job = blk_mq_rq_to_pdu(req);
int ret;
- BUG_ON(req->special);
-
- job = kzalloc(sizeof(struct bsg_job) + q->bsg_job_size, GFP_KERNEL);
- if (!job)
- return -ENOMEM;
-
- req->special = job;
- job->req = req;
- if (q->bsg_job_size)
- job->dd_data = (void *)&job[1];
- job->request = rq->cmd;
- job->request_len = rq->cmd_len;
- job->reply = rq->sense;
- job->reply_len = SCSI_SENSE_BUFFERSIZE; /* Size of sense buffer
-* allocated */
if (req->bio) {
ret = bsg_map_buffer(&job->request_payload, req);
if (ret)
@@ -187,7 +170,6 @@ static void bsg_request_fn(struct request_queue *q)
{
struct device *dev = q->queuedata;
struct request *req;
- struct bsg_job *job;
int ret;
if (!get_device(dev))
@@ -207,8 +189,7 @@ static void bsg_request_fn(struct request_queue *q)
continue;
}
- job = req->special;
- ret = q->bsg_job_fn(job);
+ ret = q->bsg_job_fn(blk_mq_rq_to_pdu(req));
spin_lock_irq(q->queue_lock);
if (ret)
break;
@@ -219,6 +200,29 @@ static void bsg_request_fn(struct request_queue *q)
spin_lock_irq(q->queue_lock);
}
+static int bsg_init_rq(struct request_queue *q, struct request *req, gfp_t gfp)
+{
+ struct bsg_job *job = blk_mq_rq_to_pdu(req);
+
+ memset(job, 0, sizeof(*job));
+ job->req = req;
+ job->dd_data = job + 1;
+ job->request = job->sreq.cmd;
+ job->request_len = job->sreq.cmd_len;
+ job->reply_len = SCSI_SENSE_BUFFERSIZE;
+ job->reply = job->sreq.sense = kzalloc(job->reply_len, gfp);
+ if (!job->reply)
+ return -ENOM
Re: [RFC PATCH 1/6] bsg: fix kernel panic resulting from missing allocation of a reply-buffer
On Fri, Aug 11, 2017 at 11:14:15AM +0200, Christoph Hellwig wrote: > But patch 1 still creates an additional copy of the sense data for > all bsg users. > Huh? What additional copy? There is one reply-buffer and that is copied into the user-buffer should it contain valid data. Just like in your patch, neither you, nor me touches any of the copy-code. There is also no changes to how the driver get their data into that buffer, it will still be copied in both cases. > > Can you test the patch below which implements my suggestion? Your > other patches should still apply fine on top modulo minor context > changes. Only your patch on top of 4.13-rc4. din_xferp (D) is also empty, which is not taken from the sense-buffer. = BUG kmalloc-1024 (Not tainted): Invalid object pointer 0x4ad9e0f0 - Disabling lock debugging due to kernel taint INFO: Slab 0x03d1012b6600 objects=24 used=11 fp=0x4ad9da58 flags=0x3fffc008101 CPU: 2 PID: 20 Comm: ksoftirqd/2 Tainted: GB 4.13.0-rc4-bsg-regression+ #2 Hardware name: IBM 2964 N96 702 (z/VM 6.4.0) Call Trace: ([<00117532>] show_stack+0x8a/0xe0) [<00bcbaee>] dump_stack+0x96/0xd8 [<003cd5fc>] slab_err+0xac/0xc0 [<003d68e4>] free_debug_processing+0x554/0x570 [<003d69ae>] __slab_free+0xae/0x618 [<003d7dce>] kfree+0x44e/0x4a0 [<00859b4e>] blk_done_softirq+0x146/0x160 [<00bf4ec0>] __do_softirq+0x3d0/0x840 [<001662a6>] run_ksoftirqd+0x3e/0xb8 [<001957fc>] smpboot_thread_fn+0x2f4/0x318 [<0018f6f6>] kthread+0x166/0x178 [<00bf3cf2>] kernel_thread_starter+0x6/0xc [<00bf3cec>] kernel_thread_starter+0x0/0xc INFO: lockdep is turned off. FIX kmalloc-1024: Object at 0x4ad9e0f0 not freed = BUG kmalloc-1024 (Tainted: GB ): Invalid object pointer 0x4ad9f630 - INFO: Slab 0x03d1012b6600 objects=24 used=11 fp=0x4ad98558 flags=0x3fffc008101 CPU: 2 PID: 20 Comm: ksoftirqd/2 Tainted: GB 4.13.0-rc4-bsg-regression+ #2 Hardware name: IBM 2964 N96 702 (z/VM 6.4.0) Call Trace: ([<00117532>] show_stack+0x8a/0xe0) [<00bcbaee>] dump_stack+0x96/0xd8 [<003cd5fc>] slab_err+0xac/0xc0 [<003d68e4>] free_debug_processing+0x554/0x570 [<003d69ae>] __slab_free+0xae/0x618 [<003d7dce>] kfree+0x44e/0x4a0 [<00859b4e>] blk_done_softirq+0x146/0x160 [<00bf4ec0>] __do_softirq+0x3d0/0x840 [<001662a6>] run_ksoftirqd+0x3e/0xb8 [<001957fc>] smpboot_thread_fn+0x2f4/0x318 [<0018f6f6>] kthread+0x166/0x178 [<00bf3cf2>] kernel_thread_starter+0x6/0xc [<00bf3cec>] kernel_thread_starter+0x0/0xc INFO: lockdep is turned off. FIX kmalloc-1024: Object at 0x4ad9f630 not freed = BUG kmalloc-1024 (Tainted: GB ): Invalid object pointer 0x4ad986a0 - INFO: Slab 0x03d1012b6600 objects=24 used=13 fp=0x4ad9d508 flags=0x3fffc008101 CPU: 2 PID: 20 Comm: ksoftirqd/2 Tainted: GB 4.13.0-rc4-bsg-regression+ #2 Hardware name: IBM 2964 N96 702 (z/VM 6.4.0) Call Trace: ([<00117532>] show_stack+0x8a/0xe0) [<00bcbaee>] dump_stack+0x96/0xd8 [<003cd5fc>] slab_err+0xac/0xc0 [<003d68e4>] free_debug_processing+0x554/0x570 [<003d69ae>] __slab_free+0xae/0x618 [<003d7dce>] kfree+0x44e/0x4a0 [<00859b4e>] blk_done_softirq+0x146/0x160 [<00bf4ec0>] __do_softirq+0x3d0/0x840 [<001662a6>] run_ksoftirqd+0x3e/0xb8 [<001957fc>] smpboot_thread_fn+0x2f4/0x318 [<0018f6f6>] kthread+0x166/0x178 [<00bf3cf2>] kernel_thread_starter+0x6/0xc [<00bf3cec>] kernel_thread_starter+0x0/0xc INFO: lockdep is turned off. FIX kmalloc-1024: Object at 0x4ad986a0 not freed = BUG kmalloc-1024 (Tainted: GB ): Invalid object pointer 0x4ad9d650 - INFO: Slab 0x03d1012b6600 objects=24 used=15 fp=0x4ad9ea48 flags=0x3fffc008101 CPU: 2 PID: 20 Comm: ksoftirqd/2 Tainted: GB 4.13.0-rc4-bsg-regression+ #2 Hardware name: IBM 2964 N96 702 (z/VM 6.4.0) Call Trace: ([<00117532>] show_stack+0x8a/0xe0) [<00bcbaee>] dump_stack+0x96/0xd8 [<003cd5fc>] slab_err+0xac/0xc0 [<003d68e4>] free_debug_pro
Re: [RFC PATCH 1/6] bsg: fix kernel panic resulting from missing allocation of a reply-buffer
But patch 1 still creates an additional copy of the sense data for
all bsg users.
Can you test the patch below which implements my suggestion? Your
other patches should still apply fine on top modulo minor context
changes.
---
>From 4cd32ee48e334b62b55bff0d380833b978454040 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig
Date: Fri, 11 Aug 2017 11:03:29 +0200
Subject: bsg-lib: allocate sense data for each request
Since we split the scsi_request out of the request the driver is supposed
to provide storage for the sense buffer. The bsg-lib code failed to do so,
though and will crash anytime it is used.
This patch moves bsg-lib to allocate and setup the bsg_job ahead of time,
and allocate the sense data, which is used as reply buffer in bsg.
Reported-by: Steffen Maier
Signed-off-by: Benjamin Block
Fixes: 82ed4db499b8 ("block: split scsi_request out of struct request")
Cc: #4.11+
---
block/bsg-lib.c | 53 +++--
include/linux/blkdev.h | 1 -
include/linux/bsg-lib.h | 2 ++
3 files changed, 31 insertions(+), 25 deletions(-)
diff --git a/block/bsg-lib.c b/block/bsg-lib.c
index c4513b23f57a..215893dbd038 100644
--- a/block/bsg-lib.c
+++ b/block/bsg-lib.c
@@ -100,7 +100,7 @@ EXPORT_SYMBOL_GPL(bsg_job_done);
*/
static void bsg_softirq_done(struct request *rq)
{
- struct bsg_job *job = rq->special;
+ struct bsg_job *job = blk_mq_rq_to_pdu(rq);
bsg_job_put(job);
}
@@ -129,26 +129,9 @@ static int bsg_map_buffer(struct bsg_buffer *buf, struct
request *req)
static int bsg_create_job(struct device *dev, struct request *req)
{
struct request *rsp = req->next_rq;
- struct request_queue *q = req->q;
- struct scsi_request *rq = scsi_req(req);
- struct bsg_job *job;
+ struct bsg_job *job = blk_mq_rq_to_pdu(req);
int ret;
- BUG_ON(req->special);
-
- job = kzalloc(sizeof(struct bsg_job) + q->bsg_job_size, GFP_KERNEL);
- if (!job)
- return -ENOMEM;
-
- req->special = job;
- job->req = req;
- if (q->bsg_job_size)
- job->dd_data = (void *)&job[1];
- job->request = rq->cmd;
- job->request_len = rq->cmd_len;
- job->reply = rq->sense;
- job->reply_len = SCSI_SENSE_BUFFERSIZE; /* Size of sense buffer
-* allocated */
if (req->bio) {
ret = bsg_map_buffer(&job->request_payload, req);
if (ret)
@@ -187,7 +170,6 @@ static void bsg_request_fn(struct request_queue *q)
{
struct device *dev = q->queuedata;
struct request *req;
- struct bsg_job *job;
int ret;
if (!get_device(dev))
@@ -207,8 +189,7 @@ static void bsg_request_fn(struct request_queue *q)
continue;
}
- job = req->special;
- ret = q->bsg_job_fn(job);
+ ret = q->bsg_job_fn(blk_mq_rq_to_pdu(req));
spin_lock_irq(q->queue_lock);
if (ret)
break;
@@ -219,6 +200,29 @@ static void bsg_request_fn(struct request_queue *q)
spin_lock_irq(q->queue_lock);
}
+static int bsg_init_rq(struct request_queue *q, struct request *req, gfp_t gfp)
+{
+ struct bsg_job *job = blk_mq_rq_to_pdu(req);
+
+ memset(job, 0, sizeof(*job));
+ job->req = req;
+ job->dd_data = job + 1;
+ job->request = job->sreq.cmd;
+ job->request_len = job->sreq.cmd_len;
+ job->reply_len = SCSI_SENSE_BUFFERSIZE;
+ job->reply = job->sreq.sense = kzalloc(job->reply_len, gfp);
+ if (!job->reply)
+ return -ENOMEM;
+ return 0;
+}
+
+static void bsg_exit_rq(struct request_queue *q, struct request *req)
+{
+ struct bsg_job *job = blk_mq_rq_to_pdu(req);
+
+ kfree(job->reply);
+}
+
/**
* bsg_setup_queue - Create and add the bsg hooks so we can receive requests
* @dev: device to attach bsg device to
@@ -235,7 +239,9 @@ struct request_queue *bsg_setup_queue(struct device *dev,
char *name,
q = blk_alloc_queue(GFP_KERNEL);
if (!q)
return ERR_PTR(-ENOMEM);
- q->cmd_size = sizeof(struct scsi_request);
+ q->cmd_size = sizeof(struct bsg_job) + dd_job_size;
+ q->init_rq_fn = bsg_init_rq;
+ q->exit_rq_fn = bsg_exit_rq;
q->request_fn = bsg_request_fn;
ret = blk_init_allocated_queue(q);
@@ -243,7 +249,6 @@ struct request_queue *bsg_setup_queue(struct device *dev,
char *name,
goto out_cleanup_queue;
q->queuedata = dev;
- q->bsg_job_size = dd_job_size;
q->bsg_job_fn = job_fn;
queue_flag_set_unlocked(QUEUE_FLAG_BIDI, q);
queue_flag_set_unlocked(QUEUE_FLAG_SCSI_PASSTHROUGH, q);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index f45f157b2910..6ae9aa6f93f0 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -568,7 +
Re: [RFC PATCH 1/6] bsg: fix kernel panic resulting from missing allocation of a reply-buffer
My point was that we now gurantee that that the sense data is not a stack pointer an a driver can DMA to it. Now for BSG the sense data is "just" abused as reply, but the point still stands - we don't want to pass a possible stack pointer to drivers in a data buffer because we want to allow DMA to it. That being said with your patch 4 that becomes a moot point as we'll now always dynamically allocate it. So maybe just reorder that to go first and we should be fine. -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To post to this group, send email to open-iscsi@googlegroups.com. Visit this group at https://groups.google.com/group/open-iscsi. For more options, visit https://groups.google.com/d/optout.
