Skip to content

Commit 50b4d48

Browse files
Benjamin-Blockaxboe
Benjamin-Block
authored and
axboe
committed
bsg-lib: fix kernel panic resulting from missing allocation of reply-buffer
Since we split the scsi_request out of struct request bsg fails to provide a reply-buffer for the drivers. This was done via the pointer for sense-data, that is not preallocated anymore. Failing to allocate/assign it results in illegal dereferences because LLDs use this pointer unquestioned. An example panic on s390x, using the zFCP driver, looks like this (I had debugging on, otherwise NULL-pointer dereferences wouldn't even panic on s390x): Unable to handle kernel pointer dereference in virtual kernel address space Failing address: 6b6b6b6b6b6b6000 TEID: 6b6b6b6b6b6b6403 Fault in home space mode while using kernel ASCE. AS:0000000001590007 R3:0000000000000024 Oops: 0038 ilc:2 [#1] PREEMPT SMP DEBUG_PAGEALLOC Modules linked in: <Long List> CPU: 2 PID: 0 Comm: swapper/2 Not tainted 4.12.0-bsg-regression+ #3 Hardware name: IBM 2964 N96 702 (z/VM 6.4.0) task: 0000000065cb0100 task.stack: 0000000065cb4000 Krnl PSW : 0704e00180000000 000003ff801e4156 (zfcp_fc_ct_els_job_handler+0x16/0x58 [zfcp]) R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0 EA:3 Krnl GPRS: 0000000000000001 000000005fa9d0d0 000000005fa9d078 0000000000e16866 000003ff00000290 6b6b6b6b6b6b6b6b 0000000059f78f00 000000000000000f 00000000593a0958 00000000593a0958 0000000060d88800 000000005ddd4c38 0000000058b50100 07000000659cba08 000003ff801e8556 00000000659cb9a8 Krnl Code: 000003ff801e4146: e31020500004 lg %r1,80(%r2) 000003ff801e414c: 58402040 l %r4,64(%r2) #000003ff801e4150: e35020200004 lg %r5,32(%r2) >000003ff801e4156: 50405004 st %r4,4(%r5) 000003ff801e415a: e54c50080000 mvhi 8(%r5),0 000003ff801e4160: e33010280012 lt %r3,40(%r1) 000003ff801e4166: a718fffb lhi %r1,-5 000003ff801e416a: 1803 lr %r0,%r3 Call Trace: ([<000003ff801e8556>] zfcp_fsf_req_complete+0x726/0x768 [zfcp]) [<000003ff801ea82a>] zfcp_fsf_reqid_check+0x102/0x180 [zfcp] [<000003ff801eb980>] zfcp_qdio_int_resp+0x230/0x278 [zfcp] [<00000000009b91b6>] qdio_kick_handler+0x2ae/0x2c8 [<00000000009b9e3e>] __tiqdio_inbound_processing+0x406/0xc10 [<00000000001684c2>] tasklet_action+0x15a/0x1d8 [<0000000000bd28ec>] __do_softirq+0x3ec/0x848 [<00000000001675a4>] irq_exit+0x74/0xf8 [<000000000010dd6a>] do_IRQ+0xba/0xf0 [<0000000000bd19e8>] io_int_handler+0x104/0x2d4 [<00000000001033b6>] enabled_wait+0xb6/0x188 ([<000000000010339e>] enabled_wait+0x9e/0x188) [<000000000010396a>] arch_cpu_idle+0x32/0x50 [<0000000000bd0112>] default_idle_call+0x52/0x68 [<00000000001cd0fa>] do_idle+0x102/0x188 [<00000000001cd41e>] cpu_startup_entry+0x3e/0x48 [<0000000000118c64>] smp_start_secondary+0x11c/0x130 [<0000000000bd2016>] restart_int_handler+0x62/0x78 [<0000000000000000>] (null) INFO: lockdep is turned off. Last Breaking-Event-Address: [<000003ff801e41d6>] zfcp_fc_ct_job_handler+0x3e/0x48 [zfcp] Kernel panic - not syncing: Fatal exception in interrupt This patch moves bsg-lib to allocate and setup struct bsg_job ahead of time, including the allocation of a buffer for the reply-data. This means, struct bsg_job is not allocated separately anymore, but as part of struct request allocation - similar to struct scsi_cmd. Reflect this in the function names that used to handle creation/destruction of struct bsg_job. Reported-by: Steffen Maier <maier@linux.vnet.ibm.com> Suggested-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Benjamin Block <bblock@linux.vnet.ibm.com> Fixes: 82ed4db ("block: split scsi_request out of struct request") Cc: <stable@vger.kernel.org> #4.11+ Signed-off-by: Jens Axboe <axboe@kernel.dk>
1 parent 1e6ec9e commit 50b4d48

File tree

3 files changed

+46
-31
lines changed

3 files changed

+46
-31
lines changed

‎block/bsg-lib.c

Lines changed: 44 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -29,26 +29,25 @@
2929
#include <scsi/scsi_cmnd.h>
3030

3131
/**
32-
* bsg_destroy_job - routine to teardown/delete a bsg job
32+
* bsg_teardown_job - routine to teardown a bsg job
3333
* @job: bsg_job that is to be torn down
3434
*/
35-
static void bsg_destroy_job(struct kref *kref)
35+
static void bsg_teardown_job(struct kref *kref)
3636
{
3737
struct bsg_job *job = container_of(kref, struct bsg_job, kref);
3838
struct request *rq = job->req;
3939

40-
blk_end_request_all(rq, BLK_STS_OK);
41-
4240
put_device(job->dev); /* release reference for the request */
4341

4442
kfree(job->request_payload.sg_list);
4543
kfree(job->reply_payload.sg_list);
46-
kfree(job);
44+
45+
blk_end_request_all(rq, BLK_STS_OK);
4746
}
4847

4948
void bsg_job_put(struct bsg_job *job)
5049
{
51-
kref_put(&job->kref, bsg_destroy_job);
50+
kref_put(&job->kref, bsg_teardown_job);
5251
}
5352
EXPORT_SYMBOL_GPL(bsg_job_put);
5453

@@ -100,7 +99,7 @@ EXPORT_SYMBOL_GPL(bsg_job_done);
10099
*/
101100
static void bsg_softirq_done(struct request *rq)
102101
{
103-
struct bsg_job *job = rq->special;
102+
struct bsg_job *job = blk_mq_rq_to_pdu(rq);
104103

105104
bsg_job_put(job);
106105
}
@@ -122,33 +121,20 @@ static int bsg_map_buffer(struct bsg_buffer *buf, struct request *req)
122121
}
123122

124123
/**
125-
* bsg_create_job - create the bsg_job structure for the bsg request
124+
* bsg_prepare_job - create the bsg_job structure for the bsg request
126125
* @dev: device that is being sent the bsg request
127126
* @req: BSG request that needs a job structure
128127
*/
129-
static int bsg_create_job(struct device *dev, struct request *req)
128+
static int bsg_prepare_job(struct device *dev, struct request *req)
130129
{
131130
struct request *rsp = req->next_rq;
132-
struct request_queue *q = req->q;
133131
struct scsi_request *rq = scsi_req(req);
134-
struct bsg_job *job;
132+
struct bsg_job *job = blk_mq_rq_to_pdu(req);
135133
int ret;
136134

137-
BUG_ON(req->special);
138-
139-
job = kzalloc(sizeof(struct bsg_job) + q->bsg_job_size, GFP_KERNEL);
140-
if (!job)
141-
return -ENOMEM;
142-
143-
req->special = job;
144-
job->req = req;
145-
if (q->bsg_job_size)
146-
job->dd_data = (void *)&job[1];
147135
job->request = rq->cmd;
148136
job->request_len = rq->cmd_len;
149-
job->reply = rq->sense;
150-
job->reply_len = SCSI_SENSE_BUFFERSIZE; /* Size of sense buffer
151-
* allocated */
137+
152138
if (req->bio) {
153139
ret = bsg_map_buffer(&job->request_payload, req);
154140
if (ret)
@@ -187,7 +173,6 @@ static void bsg_request_fn(struct request_queue *q)
187173
{
188174
struct device *dev = q->queuedata;
189175
struct request *req;
190-
struct bsg_job *job;
191176
int ret;
192177

193178
if (!get_device(dev))
@@ -199,16 +184,15 @@ static void bsg_request_fn(struct request_queue *q)
199184
break;
200185
spin_unlock_irq(q->queue_lock);
201186

202-
ret = bsg_create_job(dev, req);
187+
ret = bsg_prepare_job(dev, req);
203188
if (ret) {
204189
scsi_req(req)->result = ret;
205190
blk_end_request_all(req, BLK_STS_OK);
206191
spin_lock_irq(q->queue_lock);
207192
continue;
208193
}
209194

210-
job = req->special;
211-
ret = q->bsg_job_fn(job);
195+
ret = q->bsg_job_fn(blk_mq_rq_to_pdu(req));
212196
spin_lock_irq(q->queue_lock);
213197
if (ret)
214198
break;
@@ -219,6 +203,35 @@ static void bsg_request_fn(struct request_queue *q)
219203
spin_lock_irq(q->queue_lock);
220204
}
221205

206+
static int bsg_init_rq(struct request_queue *q, struct request *req, gfp_t gfp)
207+
{
208+
struct bsg_job *job = blk_mq_rq_to_pdu(req);
209+
struct scsi_request *sreq = &job->sreq;
210+
211+
memset(job, 0, sizeof(*job));
212+
213+
scsi_req_init(sreq);
214+
sreq->sense_len = SCSI_SENSE_BUFFERSIZE;
215+
sreq->sense = kzalloc(sreq->sense_len, gfp);
216+
if (!sreq->sense)
217+
return -ENOMEM;
218+
219+
job->req = req;
220+
job->reply = sreq->sense;
221+
job->reply_len = sreq->sense_len;
222+
job->dd_data = job + 1;
223+
224+
return 0;
225+
}
226+
227+
static void bsg_exit_rq(struct request_queue *q, struct request *req)
228+
{
229+
struct bsg_job *job = blk_mq_rq_to_pdu(req);
230+
struct scsi_request *sreq = &job->sreq;
231+
232+
kfree(sreq->sense);
233+
}
234+
222235
/**
223236
* bsg_setup_queue - Create and add the bsg hooks so we can receive requests
224237
* @dev: device to attach bsg device to
@@ -235,15 +248,16 @@ struct request_queue *bsg_setup_queue(struct device *dev, char *name,
235248
q = blk_alloc_queue(GFP_KERNEL);
236249
if (!q)
237250
return ERR_PTR(-ENOMEM);
238-
q->cmd_size = sizeof(struct scsi_request);
251+
q->cmd_size = sizeof(struct bsg_job) + dd_job_size;
252+
q->init_rq_fn = bsg_init_rq;
253+
q->exit_rq_fn = bsg_exit_rq;
239254
q->request_fn = bsg_request_fn;
240255

241256
ret = blk_init_allocated_queue(q);
242257
if (ret)
243258
goto out_cleanup_queue;
244259

245260
q->queuedata = dev;
246-
q->bsg_job_size = dd_job_size;
247261
q->bsg_job_fn = job_fn;
248262
queue_flag_set_unlocked(QUEUE_FLAG_BIDI, q);
249263
queue_flag_set_unlocked(QUEUE_FLAG_SCSI_PASSTHROUGH, q);

‎include/linux/blkdev.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -568,7 +568,6 @@ struct request_queue {
568568

569569
#if defined(CONFIG_BLK_DEV_BSG)
570570
bsg_job_fn *bsg_job_fn;
571-
int bsg_job_size;
572571
struct bsg_class_device bsg_dev;
573572
#endif
574573

‎include/linux/bsg-lib.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#define _BLK_BSG_
2525

2626
#include <linux/blkdev.h>
27+
#include <scsi/scsi_request.h>
2728

2829
struct request;
2930
struct device;
@@ -37,6 +38,7 @@ struct bsg_buffer {
3738
};
3839

3940
struct bsg_job {
41+
struct scsi_request sreq;
4042
struct device *dev;
4143
struct request *req;
4244

0 commit comments

Comments
 (0)