OstreeRepoAutoTransaction used with uninitialized refcount in ostree_repo_prepare_transaction()

[smcv]

While trying to port Flatpak to FUSE 3 (flatpak/flatpak#4326), I needed to upgrade the version of libostree used to build it in CI from 2020.8 to a version that uses FUSE 3, and tried upgrading to 2022.2. The build that is using FUSE 3 happens to be the one that also runs the tests under valgrind.

Having done so, several tests fail like this:

+ env G_SLICE=always-malloc valgrind -q --leak-check=no --error-exitcode=1 --gen-suppressions=all --num-callers=30 --suppressions=/home/runner/work/flatpak/flatpak/_build/../tests/flatpak.supp --suppressions=/home/runner/work/flatpak/flatpak/_build/../tests/glib.supp flatpak build-update-repo --gpg-homedir=/tmp/test-flatpak-5K7eYm/gpghome --gpg-sign=7B0961FD repos/test
--68438-- WARNING: unhandled amd64-linux syscall: 315
--68438-- You may be able to write your own handler.
--68438-- Read the file README_MISSING_SYSCALL_OR_IOCTL.
--68438-- Nevertheless we consider this a bug.  Please report
--68438-- it at http://valgrind.org/support/bug_reports.html.
Updating appstream branch
==68438== Conditional jump or move depends on uninitialised value(s)
==68438==    at 0x49F19E5: _ostree_repo_auto_transaction_unref (ostree-repo.c:832)
==68438==    by 0x4A068B7: glib_autoptr_clear_OstreeRepoAutoTransaction (ostree-repo-private.h:555)
==68438==    by 0x4A068B7: glib_autoptr_cleanup_OstreeRepoAutoTransaction (ostree-repo-private.h:555)
==68438==    by 0x4A068B7: ostree_repo_prepare_transaction (ostree-repo-commit.c:1692)
==68438==    by 0x1FBE61: flatpak_repo_generate_appstream (in /home/runner/work/flatpak/flatpak/_build/flatpak)
==68438==    by 0x159FDC: flatpak_builtin_build_update_repo (in /home/runner/work/flatpak/flatpak/_build/flatpak)
==68438==    by 0x137CC4: main (in /home/runner/work/flatpak/flatpak/_build/flatpak)
==68438== 
{
   <insert_a_suppression_name_here>
   Memcheck:Cond
   fun:_ostree_repo_auto_transaction_unref
   fun:glib_autoptr_clear_OstreeRepoAutoTransaction
   fun:glib_autoptr_cleanup_OstreeRepoAutoTransaction
   fun:ostree_repo_prepare_transaction
   fun:flatpak_repo_generate_appstream
   fun:flatpak_builtin_build_update_repo
   fun:main
}

This appears to be triggered by the change in c987534 "repo/private: allow committing/aborting through a transaction guard", which allocates a new sub-transaction "the hard way" instead of going through the _ostree_repo_auto_transaction_start() API:

  /* Set up to abort the transaction if we return early from this function.
   * This needs to be manually built here due to a circular dependency. */
  g_autoptr(OstreeRepoAutoTransaction) txn = g_malloc(sizeof(OstreeRepoAutoTransaction));
  txn->repo = self;

c987534 was actually correct (although it was a bit of a layering violation) because at the time of that change, transactions were not refcounted. However, 8a9737a "repo/private: move OstreeRepoAutoTransaction to a boxed type" subsequently made it refcounted, at which point the direct use in ostree_repo_prepare_transaction() is leaving txn->atomic_refcount uninitialized.

I think in practice this means the intended transaction-abort is usually not happening, because the uninitialized refcount is unlikely to come out as exactly 1?

cc @lucab