All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Schoenebeck <qemu_oss@crudebyte.com>
To: qemu-devel@nongnu.org
Cc: Gerd Hoffmann <kraxel@redhat.com>,
	Geoffrey McRae <geoff@hostfission.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [PATCH v5 1/1] audio/jack: fix use after free segfault
Date: Thu, 20 Aug 2020 12:06:39 +0200	[thread overview]
Message-ID: <3140676.b1PlGooJ8z@silver> (raw)
In-Reply-To: <20200820053728.kv7pngxqzp32uky3@sirius.home.kraxel.org>

On Donnerstag, 20. August 2020 07:37:28 CEST Gerd Hoffmann wrote:
>   Hi,
> 
> > > +    qemu_bh_cancel(c->shutdown_bh);
> > 
> > Looks like a potential race. Quote from the API doc of qemu_bh_cancel():
> > 	"While cancellation itself is also wait-free and thread-safe, it can of
> > 	course race with the loop that executes bottom halves unless you are
> > 	holding the iothread mutex.  This makes it mostly useless if you are not
> > 	holding the mutex."
> 
> Should not be a problem, all auto backend code should only be called
> while qemu holds the iothread mutex.  With the exception of the shutdown
> handler which jack might call from signal context (which is why we need
> the BH in the first place).

Hmmm, as Geoffrey already added a lock today, I noticed that QEMU's main IO 
thread mutex is not initialized as 'recursive' lock type. Does that make 
sense? I.e. shouldn't there be a

	qemu_rec_mutex_init(&qemu_global_mutex);

in softmmu/cpus.c for safety reasons to prevent nested locks from same thread 
causing misbehaviour?

CCing Paolo to clarify.

Best regards,
Christian Schoenebeck




  reply	other threads:[~2020-08-20 10:07 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-19  6:29 [PATCH v5 0/1] audio/jack: fix use after free segfault Geoffrey McRae
2020-08-19  6:29 ` [PATCH v5 1/1] " Geoffrey McRae
2020-08-19 15:21   ` Christian Schoenebeck
2020-08-19 15:27     ` Geoffrey McRae
2020-08-20  5:37     ` Gerd Hoffmann
2020-08-20 10:06       ` Christian Schoenebeck [this message]
2020-08-20 10:54         ` Paolo Bonzini
2020-08-20 12:00           ` Christian Schoenebeck
2020-08-21 13:13             ` Paolo Bonzini
2020-08-26 13:48               ` PTHREAD_MUTEX_ERRORCHECK and fork() Christian Schoenebeck
2020-08-21 11:12           ` recursive locks (in general) Christian Schoenebeck
2020-08-21 13:08             ` Paolo Bonzini
2020-08-21 15:25               ` Christian Schoenebeck
2020-08-21 11:28           ` [PATCH v5 1/1] audio/jack: fix use after free segfault Geoffrey McRae
2020-08-21 13:13             ` Paolo Bonzini

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=3140676.b1PlGooJ8z@silver \
    --to=qemu_oss@crudebyte.com \
    --cc=geoff@hostfission.com \
    --cc=kraxel@redhat.com \
    --cc=pbonzini@redhat.com \
    --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 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.