From: Cornelia Huck <cohuck@redhat.com>
To: linux-s390@vger.kernel.org, kvm@vger.kernel.org
Subject: Re: [PATCH v1 2/2] vfio: ccw: Register mediated device once all structures are initialized
Date: Tue, 23 Oct 2018 12:17:13 +0000 [thread overview]
Message-ID: <20181023141713.2ebcf24d.cohuck@redhat.com> (raw)
In-Reply-To: <2ca9df71-848e-1ec1-9773-100f05d01b8a@linux.ibm.com>
On Mon, 22 Oct 2018 19:52:25 +0200
Halil Pasic <pasic@linux.ibm.com> wrote:
> On 10/20/2018 07:10 PM, Cornelia Huck wrote:
> > On Thu, 18 Oct 2018 15:58:48 +0200
> > Halil Pasic <pasic@linux.ibm.com> wrote:
> >
> >> On 10/18/2018 09:51 AM, Pierre Morel wrote:
> >>> There is a risk that the mediated device is used before all the
> >>> data are initialized if it is register too early.
> >>
> >> How big is the risk? Is this a bug fix (if it is do we need cc stable)
> >> or readability improvement?
> >
> > This looks like a bug fix to me, but not sure if it is stable material.
> > I wanted to apply as-is.
> >
>
> Isn't atomic_set(&private->avail, 1); sufficient to ensure no mdev
> is created on top of the parent dev unless private->io_work is already
> initialized? (->avail an ->state are zero initialized, which for the
> latter means VFIO_CCW_STATE_NOT_OPER.)
>
> Pierre states in the cover letter that this one is a bugfix. But I don't
> quite get it.
>
> If it is an exploitable bug, I think it should be stable material, or?
>
> Regardless, I would prefer a better commit message and title. For
> instance 'if it is register too early' does not seem grammatical to me,
> and the 'register' is also misleading: I guess, it is not about
> vfio_ccw_mdev_reg() but the register triggered by mdev create.
>
> But no strong feelings here: the patch, if nothing else, contributes to
> readability, thus it is useful.
>
> BTW I do remember not being confident about the synchronization in vfio-ccw,
> but was hoping all this gets cleared up when the new functions (clear, halt,
> etc) get implement.
OK, I'm not in the position at the moment to do an in-depth discussion
of this (conference)... Pierre, can you post an updated description,
and Halil, can you let me know if that satisfies your requirements? If
I'm going to apply this, I'd like to know whether people are happy with
that soon...
>
> Regards,
> Halil
>
> >>
> >> I would prefer a commit message that clearly answers these questions.
> >>
> >> Otherwise I don't have a strong opinion.
> >>
> >> Regards,
> >> Halil
> >>
> >>>
> >>> Let's register the mediated device when all the data structures
> >>> which could be used are initialized.
> >>>
> >>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> >>
> >
>
parent reply other threads:[~2018-10-23 12:17 UTC|newest]
Thread overview: expand[flat|nested] mbox.gz Atom feed
[parent not found: <2ca9df71-848e-1ec1-9773-100f05d01b8a@linux.ibm.com>]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181023141713.2ebcf24d.cohuck@redhat.com \
--to=cohuck@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox