lkml.org 
[lkml]   [2020]   [Jun]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH v2 2/3] scsi: storvsc: Use vmbus_requestor to generate transaction IDs for VMBus hardening
    Date
    Currently, pointers to guest memory are passed to Hyper-V as
    transaction IDs in storvsc. In the face of errors or malicious
    behavior in Hyper-V, storvsc should not expose or trust the transaction
    IDs returned by Hyper-V to be valid guest memory addresses. Instead,
    use small integers generated by vmbus_requestor as requests
    (transaction) IDs.

    Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
    Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
    Cc: linux-scsi@vger.kernel.org
    Signed-off-by: Andres Beltran <lkmlabelt@gmail.com>
    ---
    Changes in v2:
    - Add casts to unsigned long to fix warnings on 32bit.

    drivers/scsi/storvsc_drv.c | 85 +++++++++++++++++++++++++++++++++-----
    1 file changed, 74 insertions(+), 11 deletions(-)

    diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
    index 624467e2590a..6d2df1f0fe6d 100644
    --- a/drivers/scsi/storvsc_drv.c
    +++ b/drivers/scsi/storvsc_drv.c
    @@ -399,6 +399,7 @@ static int storvsc_timeout = 180;
    static struct scsi_transport_template *fc_transport_template;
    #endif

    +static struct scsi_host_template scsi_driver;
    static void storvsc_on_channel_callback(void *context);

    #define STORVSC_MAX_LUNS_PER_TARGET 255
    @@ -698,6 +699,12 @@ static void handle_sc_creation(struct vmbus_channel *new_sc)

    memset(&props, 0, sizeof(struct vmstorage_channel_properties));

    + /*
    + * The size of vmbus_requestor is an upper bound on the number of requests
    + * that can be in-progress at any one time across all channels.
    + */
    + new_sc->rqstor_size = scsi_driver.can_queue;
    +
    ret = vmbus_open(new_sc,
    storvsc_ringbuffer_size,
    storvsc_ringbuffer_size,
    @@ -726,6 +733,7 @@ static void handle_multichannel_storage(struct hv_device *device, int max_chns)
    struct storvsc_cmd_request *request;
    struct vstor_packet *vstor_packet;
    int ret, t;
    + u64 rqst_id;

    /*
    * If the number of CPUs is artificially restricted, such as
    @@ -760,14 +768,23 @@ static void handle_multichannel_storage(struct hv_device *device, int max_chns)
    vstor_packet->flags = REQUEST_COMPLETION_FLAG;
    vstor_packet->sub_channel_count = num_sc;

    + rqst_id = vmbus_next_request_id(&device->channel->requestor,
    + (unsigned long)request);
    + if (rqst_id == VMBUS_RQST_ERROR) {
    + dev_err(dev, "No request id available\n");
    + return;
    + }
    +
    ret = vmbus_sendpacket(device->channel, vstor_packet,
    (sizeof(struct vstor_packet) -
    vmscsi_size_delta),
    - (unsigned long)request,
    + rqst_id,
    VM_PKT_DATA_INBAND,
    VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);

    if (ret != 0) {
    + /* Reclaim request ID to avoid leak of IDs */
    + vmbus_request_addr(&device->channel->requestor, rqst_id);
    dev_err(dev, "Failed to create sub-channel: err=%d\n", ret);
    return;
    }
    @@ -818,20 +835,31 @@ static int storvsc_execute_vstor_op(struct hv_device *device,
    {
    struct vstor_packet *vstor_packet;
    int ret, t;
    + u64 rqst_id;

    vstor_packet = &request->vstor_packet;

    init_completion(&request->wait_event);
    vstor_packet->flags = REQUEST_COMPLETION_FLAG;

    + rqst_id = vmbus_next_request_id(&device->channel->requestor,
    + (unsigned long)request);
    + if (rqst_id == VMBUS_RQST_ERROR) {
    + dev_err(&device->device, "No request id available\n");
    + return -EAGAIN;
    + }
    +
    ret = vmbus_sendpacket(device->channel, vstor_packet,
    (sizeof(struct vstor_packet) -
    vmscsi_size_delta),
    - (unsigned long)request,
    + rqst_id,
    VM_PKT_DATA_INBAND,
    VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
    - if (ret != 0)
    + if (ret != 0) {
    + /* Reclaim request ID to avoid leak of IDs */
    + vmbus_request_addr(&device->channel->requestor, rqst_id);
    return ret;
    + }

    t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
    if (t == 0)
    @@ -1233,9 +1261,17 @@ static void storvsc_on_channel_callback(void *context)
    foreach_vmbus_pkt(desc, channel) {
    void *packet = hv_pkt_data(desc);
    struct storvsc_cmd_request *request;
    + u64 cmd_rqst;

    - request = (struct storvsc_cmd_request *)
    - ((unsigned long)desc->trans_id);
    + cmd_rqst = vmbus_request_addr(&channel->requestor,
    + desc->trans_id);
    + if (cmd_rqst == VMBUS_RQST_ERROR) {
    + dev_err(&device->device,
    + "Incorrect transaction id\n");
    + continue;
    + }
    +
    + request = (struct storvsc_cmd_request *)(unsigned long)cmd_rqst;

    if (request == &stor_device->init_request ||
    request == &stor_device->reset_request) {
    @@ -1256,6 +1292,12 @@ static int storvsc_connect_to_vsp(struct hv_device *device, u32 ring_size,

    memset(&props, 0, sizeof(struct vmstorage_channel_properties));

    + /*
    + * The size of vmbus_requestor is an upper bound on the number of requests
    + * that can be in-progress at any one time across all channels.
    + */
    + device->channel->rqstor_size = scsi_driver.can_queue;
    +
    ret = vmbus_open(device->channel,
    ring_size,
    ring_size,
    @@ -1369,6 +1411,7 @@ static int storvsc_do_io(struct hv_device *device,
    int ret = 0;
    const struct cpumask *node_mask;
    int tgt_cpu;
    + u64 rqst_id;

    vstor_packet = &request->vstor_packet;
    stor_device = get_out_stor_device(device);
    @@ -1463,6 +1506,13 @@ static int storvsc_do_io(struct hv_device *device,

    vstor_packet->operation = VSTOR_OPERATION_EXECUTE_SRB;

    + rqst_id = vmbus_next_request_id(&outgoing_channel->requestor,
    + (unsigned long)request);
    + if (rqst_id == VMBUS_RQST_ERROR) {
    + dev_err(&device->device, "No request id available\n");
    + return -EAGAIN;
    + }
    +
    if (request->payload->range.len) {

    ret = vmbus_sendpacket_mpb_desc(outgoing_channel,
    @@ -1470,18 +1520,21 @@ static int storvsc_do_io(struct hv_device *device,
    vstor_packet,
    (sizeof(struct vstor_packet) -
    vmscsi_size_delta),
    - (unsigned long)request);
    + rqst_id);
    } else {
    ret = vmbus_sendpacket(outgoing_channel, vstor_packet,
    (sizeof(struct vstor_packet) -
    vmscsi_size_delta),
    - (unsigned long)request,
    + rqst_id,
    VM_PKT_DATA_INBAND,
    VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
    }

    - if (ret != 0)
    + if (ret != 0) {
    + /* Reclaim request ID to avoid leak of IDs */
    + vmbus_request_addr(&outgoing_channel->requestor, rqst_id);
    return ret;
    + }

    atomic_inc(&stor_device->num_outstanding_req);

    @@ -1562,7 +1615,7 @@ static int storvsc_host_reset_handler(struct scsi_cmnd *scmnd)
    struct storvsc_cmd_request *request;
    struct vstor_packet *vstor_packet;
    int ret, t;
    -
    + u64 rqst_id;

    stor_device = get_out_stor_device(device);
    if (!stor_device)
    @@ -1577,14 +1630,24 @@ static int storvsc_host_reset_handler(struct scsi_cmnd *scmnd)
    vstor_packet->flags = REQUEST_COMPLETION_FLAG;
    vstor_packet->vm_srb.path_id = stor_device->path_id;

    + rqst_id = vmbus_next_request_id(&device->channel->requestor,
    + (unsigned long)&stor_device->reset_request);
    + if (rqst_id == VMBUS_RQST_ERROR) {
    + dev_err(&device->device, "No request id available\n");
    + return FAILED;
    + }
    +
    ret = vmbus_sendpacket(device->channel, vstor_packet,
    (sizeof(struct vstor_packet) -
    vmscsi_size_delta),
    - (unsigned long)&stor_device->reset_request,
    + rqst_id,
    VM_PKT_DATA_INBAND,
    VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
    - if (ret != 0)
    + if (ret != 0) {
    + /* Reclaim request ID to avoid leak of IDs */
    + vmbus_request_addr(&device->channel->requestor, rqst_id);
    return FAILED;
    + }

    t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
    if (t == 0)
    --
    2.25.1
    \
     
     \ /
      Last update: 2020-06-29 22:03    [W:3.644 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site