All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-devel@nongnu.org, qemu-stable@nongnu.org,
	Fabiano Rosas <farosas@suse.de>
Subject: Re: [PATCH] tests/qtest/migration/tls-tests.c: Don't use tls_psk end hook for no_tls tests
Date: Thu, 12 Feb 2026 12:05:30 -0500	[thread overview]
Message-ID: <aY4IWncKW_itrxDg@x1.local> (raw)
In-Reply-To: <20260212114747.1103466-1-peter.maydell@linaro.org>

On Thu, Feb 12, 2026 at 11:47:47AM +0000, Peter Maydell wrote:
> If you run the TLS tests under a clang undefined-behaviour sanitizer build
> it will fall over like this:
> 
> ../../tests/unit/crypto-tls-psk-helpers.c:53:12: runtime error: null pointer passed as argument 1, which is declared to never be null
> /usr/include/unistd.h:858:48: note: nonnull attribute specified here
>     #0 0x62bd810762ee in test_tls_psk_cleanup /home/pm215/qemu/build/clang/../../tests/unit/crypto-tls-psk-helpers.c:53:5
>     #1 0x62bd81073f89 in migrate_hook_end_tls_psk /home/pm215/qemu/build/clang/../../tests/qtest/migration/tls-tests.c:101:5
>     #2 0x62bd81062ef0 in test_precopy_common /home/pm215/qemu/build/clang/../../tests/qtest/migration/framework.c:947:9
> 
> This happens because test_precopy_tcp_no_tls() uses a custom start_hook
> that only sets a couple of parameters, but reuses the tsk_psk end_hook.
> However, the end_hook runs cleanup that assumes that the data was set
> up by migrate_hook_start_tls_psk_common(). In particular, it will
> unconditionally call test_tls_psk_cleanup(data->pskfile), and
> test_tls_psk_cleanup() will unconditionally unlink() the filename it
> is passed, which is undefined behaviour if you pass it a NULL pointer.
> 
> Instead of creating a TestMigrateTLSPSKData struct which we never set
> any fields in and requiring the migrate_hook_end_tls_psk() hook to
> cope with that, don't allocate the struct in the start_hook. Then
> there is nothing we need to clean up, and we can set the end_hook
> to NULL (which the test framework will interpret as "don't call
> any end_hook").
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu



      parent reply	other threads:[~2026-02-12 17:06 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-12 11:47 [PATCH] tests/qtest/migration/tls-tests.c: Don't use tls_psk end hook for no_tls tests Peter Maydell
2026-02-12 12:46 ` Fabiano Rosas
2026-02-12 17:05 ` Peter Xu [this message]

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=aY4IWncKW_itrxDg@x1.local \
    --to=peterx@redhat.com \
    --cc=farosas@suse.de \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@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.