From aac8905afdd8b4ed6f560b0be4f1b0bbfbbaba78 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 14 Aug 2023 15:33:26 +0930 Subject: [PATCH] plugins/renepay/test: fix access-after-free. We cannot carry pointers into the gossmap across localmod addition or removal. We didn't notice because the map->chan_arr is not normally resized, but if we change gossmap.c line 689 to only allocate 1 to start, we see this: ``` VALGRIND=1 valgrind -q --error-exitcode=7 --track-origins=yes --leak-check=full --show-reachable=yes --errors-for-leak-kinds=all plugins/renepay/test/run-mcf > /dev/null ==2349744== Invalid read of size 4 ==2349744== at 0x1788C2: gossmap_chan_scid (gossmap.c:558) ==2349744== by 0x1872A2: get_chan_extra_half_by_chan (flow.c:346) ==2349744== by 0x187797: remove_completed_flow (flow.c:488) ==2349744== by 0x187927: remove_completed_flow_set (flow.c:518) ==2349744== by 0x18DF4D: main (run-mcf.c:393) ==2349744== Address 0x4b80f38 is 88 bytes inside a block of size 136 free'd ==2349744== at 0x4848C63: realloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==2349744== by 0x173D71: tal_resize_ (tal.c:744) ==2349744== by 0x177E36: next_free_chan (gossmap.c:336) ==2349744== by 0x177ED3: new_channel (gossmap.c:351) ==2349744== by 0x178441: add_channel (gossmap.c:458) ==2349744== by 0x1798D4: gossmap_apply_localmods (gossmap.c:904) ==2349744== by 0x18DEDB: main (run-mcf.c:388) ==2349744== Block was alloc'd at ==2349744== at 0x4848C63: realloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==2349744== by 0x173D71: tal_resize_ (tal.c:744) ==2349744== by 0x177E36: next_free_chan (gossmap.c:336) ==2349744== by 0x177ED3: new_channel (gossmap.c:351) ==2349744== by 0x178441: add_channel (gossmap.c:458) ==2349744== by 0x178B6D: map_catchup (gossmap.c:635) ==2349744== by 0x178F45: load_gossip_store (gossmap.c:697) ==2349744== by 0x179D71: gossmap_load (gossmap.c:978) ==2349744== by 0x18D22F: main (run-mcf.c:295) ``` Signed-off-by: Rusty Russell --- plugins/renepay/test/run-mcf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/renepay/test/run-mcf.c b/plugins/renepay/test/run-mcf.c index 9d862f203..539e97358 100644 --- a/plugins/renepay/test/run-mcf.c +++ b/plugins/renepay/test/run-mcf.c @@ -369,6 +369,9 @@ int main(int argc, char *argv[]) assert(amount_msat_eq(ce->half[1].known_min, AMOUNT_MSAT(0))); assert(amount_msat_eq(ce->half[1].known_max, AMOUNT_MSAT(1000000000))); + /* Clear that */ + remove_completed_flow_set(gossmap, chan_extra_map, flows); + // /* Now try adding a local channel scid */ struct short_channel_id scid13; @@ -389,9 +392,6 @@ int main(int argc, char *argv[]) struct gossmap_chan *local_chan = gossmap_find_chan(gossmap, &scid13); assert(local_chan); - /* Clear that */ - remove_completed_flow_set(gossmap, chan_extra_map, flows); - /* The local chans have no "capacity", so set it manually. */ new_chan_extra(chan_extra_map, scid13, AMOUNT_MSAT(400000000));