From: "Michael S. Tsirkin" <mst@redhat.com>
To: Babis Chalios <bchalios@amazon.es>
Cc: virtio-comment@lists.oasis-open.org,
virtio-dev@lists.oasis-open.org, "Cali,
Marco" <xmarcalx@amazon.co.uk>, "Graf (AWS),
Alexander" <graf@amazon.de>,
"Jason A. Donenfeld" <Jason@zx2c4.com>,
aams@amazon.de
Subject: [virtio-comment] Re: [virtio-dev] [PATCH RFC 3/3] rng: leak detection support
Date: Mon, 18 Sep 2023 09:58:12 -0400 [thread overview]
Message-ID: <20230918091506-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <e75d9667-2ea0-44d0-a26f-fa3d5fe05651@amazon.es>
On Mon, Sep 18, 2023 at 03:00:43PM +0200, Babis Chalios wrote:
>
> > > Right, so I think that there is a race condition between the time the driver
> > > sees the used buffers of the first
> > > batch and until it adds the second batch on the next leak queue.
> > >
> > > 1. driver adds batch 1
> > > 2. leak event
> > > 3. device uses batch 1
> > > 4. driver sees the used buffers and
> > > a. switches leak queues
> > > b. adds batch 2.
> > > 5. devices finds initial leak queue empty and sees buffers in second leak
> > > queue.
> > >
> > > If a second leak event happens after step 3 above and before all of steps 4
> > > complete then batch 2 will not
> > > be processed as part of the second leak event.
> > driver can just pre-add buffers in the second queue.
> >
> > 1. available buffers to queue 1-X
> > 2. available buffers to queue X
> >
> >
> > 3. poll queue X
> > 4. used buffers in queue X
> > 5. avail buffers in queue X
> > 6. poll queue 1-X
> > 7. used buffers in queue X
> > 8. avail buffers in queue X
> > 9. goto 3
> >
>
> Yes, that's what the driver does now in the RFC patch. However, this just
> decreases
> the race window, it doesn't eliminate it. If a third leak event happens it
> might not
> find any buffers to use:
>
> 1. available buffers to queue 1-X
> 2. available buffers to queue X
>
>
> 3. poll queue X
> 4. used buffers in queue X <- leak event 1 will use buffers in X
> 5. avail buffers in queue X
> 6. poll queue 1-X <- leak event 2 will use buffers in 1-X
> 7. used buffers in queue 1-X
> 8. avail buffers in queue 1-X
> <- leak event 3 (it needs buffers in X, race with step 5)
> 9. goto 3
I don't get it. we added buffers in step 5.
>
>
> If, instead, we define a single leak queue and require that VMM should refuse to take a snapshot
> if that queue is empty, we avoid the race condition in all cases and IMHO the protocol becomes
> much simpler.
>
>
> Cheers,
> Babis
>
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Babis Chalios <bchalios@amazon.es>
Cc: virtio-comment@lists.oasis-open.org,
virtio-dev@lists.oasis-open.org, "Cali,
Marco" <xmarcalx@amazon.co.uk>, "Graf (AWS),
Alexander" <graf@amazon.de>,
"Jason A. Donenfeld" <Jason@zx2c4.com>,
aams@amazon.de
Subject: Re: [virtio-dev] [PATCH RFC 3/3] rng: leak detection support
Date: Mon, 18 Sep 2023 09:58:12 -0400 [thread overview]
Message-ID: <20230918091506-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <e75d9667-2ea0-44d0-a26f-fa3d5fe05651@amazon.es>
On Mon, Sep 18, 2023 at 03:00:43PM +0200, Babis Chalios wrote:
>
> > > Right, so I think that there is a race condition between the time the driver
> > > sees the used buffers of the first
> > > batch and until it adds the second batch on the next leak queue.
> > >
> > > 1. driver adds batch 1
> > > 2. leak event
> > > 3. device uses batch 1
> > > 4. driver sees the used buffers and
> > > a. switches leak queues
> > > b. adds batch 2.
> > > 5. devices finds initial leak queue empty and sees buffers in second leak
> > > queue.
> > >
> > > If a second leak event happens after step 3 above and before all of steps 4
> > > complete then batch 2 will not
> > > be processed as part of the second leak event.
> > driver can just pre-add buffers in the second queue.
> >
> > 1. available buffers to queue 1-X
> > 2. available buffers to queue X
> >
> >
> > 3. poll queue X
> > 4. used buffers in queue X
> > 5. avail buffers in queue X
> > 6. poll queue 1-X
> > 7. used buffers in queue X
> > 8. avail buffers in queue X
> > 9. goto 3
> >
>
> Yes, that's what the driver does now in the RFC patch. However, this just
> decreases
> the race window, it doesn't eliminate it. If a third leak event happens it
> might not
> find any buffers to use:
>
> 1. available buffers to queue 1-X
> 2. available buffers to queue X
>
>
> 3. poll queue X
> 4. used buffers in queue X <- leak event 1 will use buffers in X
> 5. avail buffers in queue X
> 6. poll queue 1-X <- leak event 2 will use buffers in 1-X
> 7. used buffers in queue 1-X
> 8. avail buffers in queue 1-X
> <- leak event 3 (it needs buffers in X, race with step 5)
> 9. goto 3
I don't get it. we added buffers in step 5.
>
>
> If, instead, we define a single leak queue and require that VMM should refuse to take a snapshot
> if that queue is empty, we avoid the race condition in all cases and IMHO the protocol becomes
> much simpler.
>
>
> Cheers,
> Babis
>
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
next prev parent reply other threads:[~2023-09-18 13:58 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-21 16:30 [virtio-comment] [PATCH RFC 0/3] virtio-rng based entropy leak reporting Michael S. Tsirkin
2022-11-21 16:30 ` [virtio-comment] [PATCH RFC 1/3] rng: move to a file of its own Michael S. Tsirkin
2022-11-21 16:30 ` [virtio-comment] [PATCH RFC 2/3] rng: be specific about the virtqueue Michael S. Tsirkin
2022-11-21 16:30 ` [virtio-dev] [PATCH RFC 3/3] rng: leak detection support Michael S. Tsirkin
2022-11-25 12:41 ` [virtio-dev] " Babis Chalios
2022-12-12 10:10 ` Babis Chalios
2023-01-11 13:57 ` Babis Chalios
2023-08-31 10:16 ` [virtio-dev] " Babis Chalios
2023-09-12 21:05 ` [virtio-comment] " Michael S. Tsirkin
2023-09-12 21:05 ` Michael S. Tsirkin
2023-09-13 9:32 ` Babis Chalios
2023-09-13 9:37 ` [virtio-comment] " Michael S. Tsirkin
2023-09-13 9:37 ` Michael S. Tsirkin
2023-09-13 11:19 ` Babis Chalios
2023-09-18 11:14 ` Babis Chalios
2023-09-18 12:41 ` [virtio-comment] " Michael S. Tsirkin
2023-09-18 12:41 ` Michael S. Tsirkin
2023-09-18 13:00 ` Babis Chalios
2023-09-18 13:58 ` Michael S. Tsirkin [this message]
2023-09-18 13:58 ` Michael S. Tsirkin
2023-09-18 14:02 ` Babis Chalios
2023-09-18 14:05 ` [virtio-comment] " Michael S. Tsirkin
2023-09-18 14:05 ` Michael S. Tsirkin
2023-09-18 16:30 ` Babis Chalios
2023-09-19 7:32 ` Babis Chalios
2023-09-19 10:01 ` [virtio-comment] " Michael S. Tsirkin
2023-09-19 10:01 ` Michael S. Tsirkin
2023-09-19 10:11 ` Babis Chalios
2023-09-22 12:30 ` Babis Chalios
2023-09-22 15:06 ` [virtio-comment] " Michael S. Tsirkin
2023-09-22 15:06 ` Michael S. Tsirkin
2023-09-22 15:40 ` Babis Chalios
2023-09-22 16:01 ` [virtio-comment] " Michael S. Tsirkin
2023-09-22 16:01 ` Michael S. Tsirkin
2023-09-27 10:43 ` Babis Chalios
2023-09-27 21:47 ` [virtio-comment] " Michael S. Tsirkin
2023-09-27 21:47 ` Michael S. Tsirkin
2023-09-28 18:16 ` Babis Chalios
2023-10-13 7:49 ` Babis Chalios
2023-10-13 13:38 ` [virtio-comment] " Michael S. Tsirkin
2023-10-13 13:38 ` Michael S. Tsirkin
2023-11-02 11:20 ` [virtio-comment] " Michael S. Tsirkin
2023-11-02 11:20 ` Michael S. Tsirkin
2023-11-02 11:38 ` Babis Chalios
2023-11-02 11:51 ` [virtio-comment] " Michael S. Tsirkin
2023-11-02 11:51 ` Michael S. Tsirkin
2023-11-02 13:42 ` Babis Chalios
2023-11-02 11:25 ` [virtio-comment] " Michael S. Tsirkin
2023-11-02 11:25 ` Michael S. Tsirkin
2023-11-02 11:51 ` Babis Chalios
2023-01-12 7:02 ` [virtio-dev] Re: [PATCH RFC 0/3] virtio-rng based entropy leak reporting Michael S. Tsirkin
2023-01-16 11:39 ` Babis Chalios
[not found] ` <CAHmME9ry2fss2gsbPs2zVJkY=8Cdeae0XFD9FzCVnW67Xy3thA@mail.gmail.com>
2023-01-16 18:11 ` [virtio-comment] " Michael S. Tsirkin
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=20230918091506-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=Jason@zx2c4.com \
--cc=aams@amazon.de \
--cc=bchalios@amazon.es \
--cc=graf@amazon.de \
--cc=virtio-comment@lists.oasis-open.org \
--cc=virtio-dev@lists.oasis-open.org \
--cc=xmarcalx@amazon.co.uk \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.