bsg: split handling of SCSI CDBs vs transport requeues
The current BSG design tries to shoe-horn the transport-specific
passthrough commands into the overall framework for SCSI passthrough
requests. This has a couple problems:
- each passthrough queue has to set the QUEUE_FLAG_SCSI_PASSTHROUGH flag
despite not dealing with SCSI commands at all. Because of that these
queues could also incorrectly accept SCSI commands from in-kernel
users or through the legacy SCSI_IOCTL_SEND_COMMAND ioctl.
- the real SCSI bsg queues also incorrectly accept bsg requests of the
BSG_SUB_PROTOCOL_SCSI_TRANSPORT type
- the bsg transport code is almost unredable because it tries to reuse
different SCSI concepts for its own purpose.
This patch instead adds a new bsg_ops structure to handle the two cases
differently, and thus solves all of the above problems. Another side
effect is that the bsg-lib queues also don't need to embedd a
struct scsi_request anymore.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
diff --git a/block/bsg.c b/block/bsg.c
index 06dc96e..defa06c 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -130,32 +130,110 @@ static inline struct hlist_head *bsg_dev_idx_hash(int index)
return &bsg_device_list[index & (BSG_LIST_ARRAY_SIZE - 1)];
}
-static int blk_fill_sgv4_hdr_rq(struct request_queue *q, struct request *rq,
- struct sg_io_v4 *hdr, struct bsg_device *bd,
- fmode_t mode)
-{
- struct scsi_request *req = scsi_req(rq);
+#define uptr64(val) ((void __user *)(uintptr_t)(val))
- if (hdr->request_len > BLK_MAX_CDB) {
- req->cmd = kzalloc(hdr->request_len, GFP_KERNEL);
- if (!req->cmd)
+static int bsg_scsi_check_proto(struct sg_io_v4 *hdr)
+{
+ if (hdr->protocol != BSG_PROTOCOL_SCSI ||
+ hdr->subprotocol != BSG_SUB_PROTOCOL_SCSI_CMD)
+ return -EINVAL;
+ return 0;
+}
+
+static int bsg_scsi_fill_hdr(struct request *rq, struct sg_io_v4 *hdr,
+ fmode_t mode)
+{
+ struct scsi_request *sreq = scsi_req(rq);
+
+ sreq->cmd_len = hdr->request_len;
+ if (sreq->cmd_len > BLK_MAX_CDB) {
+ sreq->cmd = kzalloc(sreq->cmd_len, GFP_KERNEL);
+ if (!sreq->cmd)
return -ENOMEM;
}
- if (copy_from_user(req->cmd, (void __user *)(unsigned long)hdr->request,
- hdr->request_len))
+ if (copy_from_user(sreq->cmd, uptr64(hdr->request), sreq->cmd_len))
return -EFAULT;
-
- if (hdr->subprotocol == BSG_SUB_PROTOCOL_SCSI_CMD) {
- if (blk_verify_command(req->cmd, mode))
- return -EPERM;
- } else if (!capable(CAP_SYS_RAWIO))
+ if (blk_verify_command(sreq->cmd, mode))
return -EPERM;
+ return 0;
+}
+
+static int bsg_scsi_complete_rq(struct request *rq, struct sg_io_v4 *hdr)
+{
+ struct scsi_request *sreq = scsi_req(rq);
+ int ret = 0;
/*
- * fill in request structure
+ * fill in all the output members
*/
- req->cmd_len = hdr->request_len;
+ hdr->device_status = sreq->result & 0xff;
+ hdr->transport_status = host_byte(sreq->result);
+ hdr->driver_status = driver_byte(sreq->result);
+ hdr->info = 0;
+ if (hdr->device_status || hdr->transport_status || hdr->driver_status)
+ hdr->info |= SG_INFO_CHECK;
+ hdr->response_len = 0;
+
+ if (sreq->sense_len && hdr->response) {
+ int len = min_t(unsigned int, hdr->max_response_len,
+ sreq->sense_len);
+
+ if (copy_to_user(uptr64(hdr->response), sreq->sense, len))
+ ret = -EFAULT;
+ else
+ hdr->response_len = len;
+ }
+
+ if (rq->next_rq) {
+ hdr->dout_resid = sreq->resid_len;
+ hdr->din_resid = scsi_req(rq->next_rq)->resid_len;
+ } else if (rq_data_dir(rq) == READ) {
+ hdr->din_resid = sreq->resid_len;
+ } else {
+ hdr->dout_resid = sreq->resid_len;
+ }
+
+ return ret;
+}
+
+static void bsg_scsi_free_rq(struct request *rq)
+{
+ scsi_req_free_cmd(scsi_req(rq));
+}
+
+static const struct bsg_ops bsg_scsi_ops = {
+ .check_proto = bsg_scsi_check_proto,
+ .fill_hdr = bsg_scsi_fill_hdr,
+ .complete_rq = bsg_scsi_complete_rq,
+ .free_rq = bsg_scsi_free_rq,
+};
+
+static struct request *
+bsg_map_hdr(struct request_queue *q, struct sg_io_v4 *hdr, fmode_t mode)
+{
+ struct request *rq, *next_rq = NULL;
+ int ret;
+
+ if (!q->bsg_dev.class_dev)
+ return ERR_PTR(-ENXIO);
+
+ if (hdr->guard != 'Q')
+ return ERR_PTR(-EINVAL);
+
+ ret = q->bsg_dev.ops->check_proto(hdr);
+ if (ret)
+ return ERR_PTR(ret);
+
+ rq = blk_get_request(q, hdr->dout_xfer_len ?
+ REQ_OP_SCSI_OUT : REQ_OP_SCSI_IN,
+ GFP_KERNEL);
+ if (IS_ERR(rq))
+ return rq;
+
+ ret = q->bsg_dev.ops->fill_hdr(rq, hdr, mode);
+ if (ret)
+ goto out;
rq->timeout = msecs_to_jiffies(hdr->timeout);
if (!rq->timeout)
@@ -165,79 +243,7 @@ static int blk_fill_sgv4_hdr_rq(struct request_queue *q, struct request *rq,
if (rq->timeout < BLK_MIN_SG_TIMEOUT)
rq->timeout = BLK_MIN_SG_TIMEOUT;
- return 0;
-}
-
-/*
- * Check if sg_io_v4 from user is allowed and valid
- */
-static int
-bsg_validate_sgv4_hdr(struct sg_io_v4 *hdr, int *op)
-{
- int ret = 0;
-
- if (hdr->guard != 'Q')
- return -EINVAL;
-
- switch (hdr->protocol) {
- case BSG_PROTOCOL_SCSI:
- switch (hdr->subprotocol) {
- case BSG_SUB_PROTOCOL_SCSI_CMD:
- case BSG_SUB_PROTOCOL_SCSI_TRANSPORT:
- break;
- default:
- ret = -EINVAL;
- }
- break;
- default:
- ret = -EINVAL;
- }
-
- *op = hdr->dout_xfer_len ? REQ_OP_SCSI_OUT : REQ_OP_SCSI_IN;
- return ret;
-}
-
-/*
- * map sg_io_v4 to a request.
- */
-static struct request *
-bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr, fmode_t mode)
-{
- struct request_queue *q = bd->queue;
- struct request *rq, *next_rq = NULL;
- int ret;
- unsigned int op, dxfer_len;
- void __user *dxferp = NULL;
- struct bsg_class_device *bcd = &q->bsg_dev;
-
- /* if the LLD has been removed then the bsg_unregister_queue will
- * eventually be called and the class_dev was freed, so we can no
- * longer use this request_queue. Return no such address.
- */
- if (!bcd->class_dev)
- return ERR_PTR(-ENXIO);
-
- bsg_dbg(bd, "map hdr %llx/%u %llx/%u\n",
- (unsigned long long) hdr->dout_xferp,
- hdr->dout_xfer_len, (unsigned long long) hdr->din_xferp,
- hdr->din_xfer_len);
-
- ret = bsg_validate_sgv4_hdr(hdr, &op);
- if (ret)
- return ERR_PTR(ret);
-
- /*
- * map scatter-gather elements separately and string them to request
- */
- rq = blk_get_request(q, op, GFP_KERNEL);
- if (IS_ERR(rq))
- return rq;
-
- ret = blk_fill_sgv4_hdr_rq(q, rq, hdr, bd, mode);
- if (ret)
- goto out;
-
- if (op == REQ_OP_SCSI_OUT && hdr->din_xfer_len) {
+ if (hdr->dout_xfer_len && hdr->din_xfer_len) {
if (!test_bit(QUEUE_FLAG_BIDI, &q->queue_flags)) {
ret = -EOPNOTSUPP;
goto out;
@@ -246,42 +252,39 @@ bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr, fmode_t mode)
next_rq = blk_get_request(q, REQ_OP_SCSI_IN, GFP_KERNEL);
if (IS_ERR(next_rq)) {
ret = PTR_ERR(next_rq);
- next_rq = NULL;
goto out;
}
- rq->next_rq = next_rq;
- dxferp = (void __user *)(unsigned long)hdr->din_xferp;
- ret = blk_rq_map_user(q, next_rq, NULL, dxferp,
+ rq->next_rq = next_rq;
+ ret = blk_rq_map_user(q, next_rq, NULL, uptr64(hdr->din_xferp),
hdr->din_xfer_len, GFP_KERNEL);
if (ret)
- goto out;
+ goto out_free_nextrq;
}
if (hdr->dout_xfer_len) {
- dxfer_len = hdr->dout_xfer_len;
- dxferp = (void __user *)(unsigned long)hdr->dout_xferp;
+ ret = blk_rq_map_user(q, rq, NULL, uptr64(hdr->dout_xferp),
+ hdr->dout_xfer_len, GFP_KERNEL);
} else if (hdr->din_xfer_len) {
- dxfer_len = hdr->din_xfer_len;
- dxferp = (void __user *)(unsigned long)hdr->din_xferp;
- } else
- dxfer_len = 0;
-
- if (dxfer_len) {
- ret = blk_rq_map_user(q, rq, NULL, dxferp, dxfer_len,
- GFP_KERNEL);
- if (ret)
- goto out;
+ ret = blk_rq_map_user(q, rq, NULL, uptr64(hdr->din_xferp),
+ hdr->din_xfer_len, GFP_KERNEL);
+ } else {
+ ret = blk_rq_map_user(q, rq, NULL, NULL, 0, GFP_KERNEL);
}
+ if (ret)
+ goto out_unmap_nextrq;
return rq;
+
+out_unmap_nextrq:
+ if (rq->next_rq)
+ blk_rq_unmap_user(rq->next_rq->bio);
+out_free_nextrq:
+ if (rq->next_rq)
+ blk_put_request(rq->next_rq);
out:
- scsi_req_free_cmd(scsi_req(rq));
+ q->bsg_dev.ops->free_rq(rq);
blk_put_request(rq);
- if (next_rq) {
- blk_rq_unmap_user(next_rq->bio);
- blk_put_request(next_rq);
- }
return ERR_PTR(ret);
}
@@ -383,56 +386,18 @@ static struct bsg_command *bsg_get_done_cmd(struct bsg_device *bd)
static int blk_complete_sgv4_hdr_rq(struct request *rq, struct sg_io_v4 *hdr,
struct bio *bio, struct bio *bidi_bio)
{
- struct scsi_request *req = scsi_req(rq);
- int ret = 0;
+ int ret;
- pr_debug("rq %p bio %p 0x%x\n", rq, bio, req->result);
- /*
- * fill in all the output members
- */
- hdr->device_status = req->result & 0xff;
- hdr->transport_status = host_byte(req->result);
- hdr->driver_status = driver_byte(req->result);
- hdr->info = 0;
- if (hdr->device_status || hdr->transport_status || hdr->driver_status)
- hdr->info |= SG_INFO_CHECK;
- hdr->response_len = 0;
-
- if (req->sense_len && hdr->response) {
- int len = min_t(unsigned int, hdr->max_response_len,
- req->sense_len);
-
- ret = copy_to_user((void __user *)(unsigned long)hdr->response,
- req->sense, len);
- if (!ret)
- hdr->response_len = len;
- else
- ret = -EFAULT;
- }
+ ret = rq->q->bsg_dev.ops->complete_rq(rq, hdr);
if (rq->next_rq) {
- hdr->dout_resid = req->resid_len;
- hdr->din_resid = scsi_req(rq->next_rq)->resid_len;
blk_rq_unmap_user(bidi_bio);
blk_put_request(rq->next_rq);
- } else if (rq_data_dir(rq) == READ)
- hdr->din_resid = req->resid_len;
- else
- hdr->dout_resid = req->resid_len;
-
- /*
- * If the request generated a negative error number, return it
- * (providing we aren't already returning an error); if it's
- * just a protocol response (i.e. non negative), that gets
- * processed above.
- */
- if (!ret && req->result < 0)
- ret = req->result;
+ }
blk_rq_unmap_user(bio);
- scsi_req_free_cmd(req);
+ rq->q->bsg_dev.ops->free_rq(rq);
blk_put_request(rq);
-
return ret;
}
@@ -614,7 +579,7 @@ static int __bsg_write(struct bsg_device *bd, const char __user *buf,
/*
* get a request, fill in the blanks, and add to request queue
*/
- rq = bsg_map_hdr(bd, &bc->hdr, mode);
+ rq = bsg_map_hdr(bd->queue, &bc->hdr, mode);
if (IS_ERR(rq)) {
ret = PTR_ERR(rq);
rq = NULL;
@@ -742,11 +707,6 @@ static struct bsg_device *bsg_add_device(struct inode *inode,
struct bsg_device *bd;
unsigned char buf[32];
- if (!blk_queue_scsi_passthrough(rq)) {
- WARN_ONCE(true, "Attempt to register a non-SCSI queue\n");
- return ERR_PTR(-EINVAL);
- }
-
if (!blk_get_queue(rq))
return ERR_PTR(-ENXIO);
@@ -907,7 +867,7 @@ static long bsg_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
if (copy_from_user(&hdr, uarg, sizeof(hdr)))
return -EFAULT;
- rq = bsg_map_hdr(bd, &hdr, file->f_mode);
+ rq = bsg_map_hdr(bd->queue, &hdr, file->f_mode);
if (IS_ERR(rq))
return PTR_ERR(rq);
@@ -959,7 +919,8 @@ void bsg_unregister_queue(struct request_queue *q)
EXPORT_SYMBOL_GPL(bsg_unregister_queue);
int bsg_register_queue(struct request_queue *q, struct device *parent,
- const char *name, void (*release)(struct device *))
+ const char *name, const struct bsg_ops *ops,
+ void (*release)(struct device *))
{
struct bsg_class_device *bcd;
dev_t dev;
@@ -996,6 +957,7 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
bcd->queue = q;
bcd->parent = get_device(parent);
bcd->release = release;
+ bcd->ops = ops;
kref_init(&bcd->ref);
dev = MKDEV(bsg_major, bcd->minor);
class_dev = device_create(bsg_class, parent, dev, NULL, "%s", devname);
@@ -1023,7 +985,17 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
mutex_unlock(&bsg_mutex);
return ret;
}
-EXPORT_SYMBOL_GPL(bsg_register_queue);
+
+int bsg_scsi_register_queue(struct request_queue *q, struct device *parent)
+{
+ if (!blk_queue_scsi_passthrough(q)) {
+ WARN_ONCE(true, "Attempt to register a non-SCSI queue\n");
+ return -EINVAL;
+ }
+
+ return bsg_register_queue(q, parent, NULL, &bsg_scsi_ops, NULL);
+}
+EXPORT_SYMBOL_GPL(bsg_scsi_register_queue);
static struct cdev bsg_cdev;
