lkml.org 
[lkml]   [2018]   [May]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v4 0/2] vfio/mdev: Device namespace protection
    On Tue, 22 May 2018 19:17:07 +0200
    Halil Pasic <pasic@linux.ibm.com> wrote:

    > From vfio-ccw perspective I join Connie's assessment: vfio-ccw should
    > be fine with these changes. I'm however not too deeply involved with
    > the mdev framework, thus I don't feel comfortable r-b-ing. That results
    > in
    > Acked-by: Halil Pasic <pasic@linux.ibm.com>
    > for both patches.
    >
    > While at it I have would like to ask about the semantics and intended
    > use of the mdev interfaces.
    >
    > static int vfio_ccw_sch_probe(struct subchannel *sch)
    > {
    >
    > /* HALIL: 8< Not so interesting stuff happens here. >8 */

    This was interesting:

    private->state = VFIO_CCW_STATE_NOT_OPER;

    > ret = vfio_ccw_mdev_reg(sch);
    > if (ret)
    > goto out_disable;
    > /*
    > * HALIL:
    > * This might be racy. Somewhere in vfio_ccw_mdev_reg() the create attribute
    > * is made available (it calls mdev_register_device()). For instance create will
    > * attempt to decrement private->avail which is initialized below. I fail to
    > * understand how is this well synchronized.
    > */
    > INIT_WORK(&private->io_work, vfio_ccw_sch_io_todo);
    > atomic_set(&private->avail, 1);
    > private->state = VFIO_CCW_STATE_STANDBY;
    >
    > return 0;
    >
    > out_disable:
    > cio_disable_subchannel(sch);
    > out_free:
    > dev_set_drvdata(&sch->dev, NULL);
    > kfree(private);
    > return ret;
    > }
    >
    > Should not initialization of go before mdev_register_device(), and then rolled
    > back if necessary if mdev_register_device() fails?
    >
    > In practice it does not seem very likely that userspace can trigger
    > mdev_device_create() before vfio_ccw_sch_probe() finishes so it should
    > not be a practical problem. But I would like to understand how synchronization
    > is supposed to work.
    >
    > [Added Dong Jia, maybe he is also able to answer my question.]

    vfio_ccw_mdev_create() requires that private->state is not
    VFIO_CCW_STATE_NOT_OPER but vfio_ccw_sch_probe() explicitly sets state
    to this value before calling vfio_ccw_mdev_reg(), so a create should
    return -ENODEV if racing with parent registration. Is there something
    else that I'm missing? Thanks,

    Alex

    \
     
     \ /
      Last update: 2018-05-22 20:38    [W:4.103 / U:0.068 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site