bubblewrap: Fix leak of xasprintf-ed source path for --(ro)-bind-fd flag

[bbhtt]

When --bind-fd or --ro-bind-fd is used, op->source is set to a heap-allocated string from xasprintf() which was not being free-d

The trace is from 0.11.0

Direct leak of 34 byte(s) in 2 object(s) allocated from:
    #0 0x7fb8df6fd9c7  (/lib/x86_64-linux-gnu/libasan.so.8+0xfd9c7) (BuildId: 0241d5a774aeb1d6babd9f68d743bdcf31b4a97d)
    #1 0x7fb8dea8f947  (/lib/x86_64-linux-gnu/libc.so.6+0x8f947) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
    #2 0x7fb8df6cf32e  (/lib/x86_64-linux-gnu/libasan.so.8+0xcf32e) (BuildId: 0241d5a774aeb1d6babd9f68d743bdcf31b4a97d)
    #3 0x55f2646fdddd in xasprintf ../subprojects/bubblewrap/utils.c:364
    #4 0x55f2646e73e7 in parse_args_recurse ../subprojects/bubblewrap/bubblewrap.c:2018
    #5 0x55f2646e5867 in parse_args_recurse ../subprojects/bubblewrap/bubblewrap.c:1855
    #6 0x55f2646efc76 in parse_args ../subprojects/bubblewrap/bubblewrap.c:2782
    #7 0x55f2646f10e8 in main ../subprojects/bubblewrap/bubblewrap.c:2927
    #8 0x7fb8dea2a1c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
    #9 0x7fb8dea2a28a  (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
    #10 0x55f2646d7344 in _start (/home/runner/work/flatpak/flatpak/builddir/subprojects/bubblewrap/flatpak-bwrap+0x3b344) (BuildId: c4d1276e28ed30dbeb13c58c7bd96078e062ef26)
SUMMARY: AddressSanitizer: 34 byte(s) leaked in 2 allocation(s).

[swick]

A bit unrelated but I'm absolutely confused by what's going on here...

[smcv]

For --bind (etc.) it's a fatal error if the source is a dangling symlink, but for --bind-try we save the original source and try it again later for some reason, instead of skipping the setup op like I'd have expected. (I'm not entirely sure why.)

[swick]

Change looks fine

[smcv]

Leaks in bubblewrap are not actually a big deal, because it never allocates very much memory in total, and its memory use is bounded (it isn't a daemon that runs for hours and leaks one string per request, it'll only leak O(total size of argv) bytes per invocation), so fixing this doesn't have to be a high priority.

[smcv]

Freeing a const char * is/should be a red flag.

I think it would be easier to follow the structure of this code if instead of a boolean flag, it was more like:

struct _SetupOp {
  const char *source;
  char *source_owned;
...
};

and anything that mallocs a new source worked more like:

free (steal_pointer (&op->source_owned));
op->source = op->source_owned = xstrdup (...);

with explicit ownership transfer (the C equivalent of std::move) where appropriate, like:

cleanup_free char *new_source = NULL;
...
new_source = realpath (old_source, NULL);
if (new_source != NULL)
  {
    free (steal_pointer (&op->source_owned));
    op->source = op->source_owned = steal_pointer (&new_source);
  }
else
  {
    ... recover ...
  }

(If we had an equivalent of g_clear_pointer() we could use that instead of free(steal(...)), but unfortunately cleanup_freep is not that yet because it doesn't clear the dangling pointer, as discussed in #667.)

[smcv]

For --bind (etc.) it's a fatal error if the source is a dangling symlink, but for --bind-try we save the original source and try it again later for some reason, instead of skipping the setup op like I'd have expected. (I'm not entirely sure why.)