qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-devel@nongnu.org, Laurent Vivier <laurent@vivier.eu>,
	Jon Alduan <jon.alduan@gmail.com>
Subject: Re: [PATCH] linux-user: Don't assume 0 is not a valid host timer_t value
Date: Mon, 25 Jul 2022 12:13:24 +0100	[thread overview]
Message-ID: <Yt561CDN+UjmaDK3@redhat.com> (raw)
In-Reply-To: <20220725110035.1273441-1-peter.maydell@linaro.org>

On Mon, Jul 25, 2022 at 12:00:35PM +0100, Peter Maydell wrote:
> For handling guest POSIX timers, we currently use an array
> g_posix_timers[], whose entries are a host timer_t value, or 0 for
> "this slot is unused".  When the guest calls the timer_create syscall
> we look through the array for a slot containing 0, and use that for
> the new timer.
> 
> This scheme assumes that host timer_t values can never be zero.  This
> is unfortunately not a valid assumption -- for some host libc
> versions, timer_t values are simply indexes starting at 0.  When
> using this kind of host libc, the effect is that the first and second
> timers end up sharing a slot, and so when the guest tries to operate
> on the first timer it changes the second timer instead.

For sake of historical record, could you mention here which specific
libc impl / version highlights the problem.

> 
> Rework the timer allocation code, so that:
>  * the 'slot in use' indication uses a separate array from the
>    host timer_t array
>  * we grab the free slot atomically, to avoid races when multiple
>    threads call timer_create simultaneously
>  * releasing an allocated slot is abstracted out into a new
>    free_host_timer_slot() function called in the correct places
> 
> This fixes:
>  * problems on hosts where timer_t 0 is valid
>  * the FIXME in next_free_host_timer() about locking
>  * bugs in the error paths in timer_create where we forgot to release
>    the slot we grabbed, or forgot to free the host timer
> 
> Reported-by: Jon Alduan <jon.alduan@gmail.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  linux-user/syscall.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



  reply	other threads:[~2022-07-25 11:14 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-25 11:00 [PATCH] linux-user: Don't assume 0 is not a valid host timer_t value Peter Maydell
2022-07-25 11:13 ` Daniel P. Berrangé [this message]
2022-07-25 12:45   ` Peter Maydell
2022-07-26 22:13     ` Jon Alduan
2022-07-29 11:06       ` Peter Maydell
2022-08-01 11:43   ` Peter Maydell
2022-08-09  9:51     ` Peter Maydell
2022-08-10  5:59       ` Laurent Vivier
2022-08-10 12:24         ` Peter Maydell
2022-09-27  9:18 ` Laurent Vivier

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=Yt561CDN+UjmaDK3@redhat.com \
    --to=berrange@redhat.com \
    --cc=jon.alduan@gmail.com \
    --cc=laurent@vivier.eu \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.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;
as well as URLs for NNTP newsgroup(s).