lkml.org 
[lkml]   [2021]   [Mar]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH] nvme: disallow passthru cmd from targeting a nsid != nsid of the block dev
    Date
    On Thu, Mar 25, 2021 at 09:48:37AM +0000, Niklas Cassel wrote:
    > From: Niklas Cassel <niklas.cassel@wdc.com>
    >
    > When a passthru command targets a specific namespace, the ns parameter to
    > nvme_user_cmd()/nvme_user_cmd64() is set. However, there is currently no
    > validation that the nsid specified in the passthru command targets the
    > namespace/nsid represented by the block device that the ioctl was
    > performed on.
    >
    > Add a check that validates that the nsid in the passthru command matches
    > that of the supplied namespace.
    >
    > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
    > ---
    > Currently, if doing NVME_IOCTL_IO_CMD on the controller char device,
    > if and only if there is a single namespace in the ctrl->namespaces list,
    > nvme_dev_user_cmd() will call nvme_user_cmd() with ns parameter set.
    > While it might be good that we validate the nsid even in this case,
    > perhaps we want to allow a nsid value in the passthru command to be
    > 0x0 and/or the NSID broadcast value? (Only when NVME_IOCTL_IO_CMD was
    > done on the controller char device though.)

    TL;DR:
    Since nvme_dev_user_cmd() only calls nvme_user_cmd() if and only if
    there is a single namespace in the ctrl->namespaces list, I think that
    this patch is good as is.


    Long story:
    If we merge Javier's patch series, then we will have all namespaces
    (rejected or not) in ctrl->namespaces.
    We could then decide to remove the weird restriction that you can
    only use the contoller char device to do NVME_IOCTL_IO_CMD
    when there is one and only one entry in the ctrl->namespaces list.
    i.e. the user could specify any NSID, we just iterate the list and
    get the struct nvme_ns that matches the cmd.nsid.

    We could then also allow the broadcast value being specified as
    an NSID, and in that case, use a disk less request_queue (like
    Keith suggested in an earlier thread).

    Being allowed to specify any NSID when using the controller char
    device could be ok, as you probably wouldn't allow everyone access
    to it.

    So Javier's patch series still provides value, even if we allow any
    NSID to be specified in the controller char device, since the per
    namespace char devices can have more fine grained access control.


    Kind regards,
    Niklas
    \
     
     \ /
      Last update: 2021-03-25 13:07    [W:2.810 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site