mirrors/core-lightning
1
0
Fork 0
mirror of https://github.com/ElementsProject/lightning.git synced 2025-01-18 05:12:45 +01:00
Code Issues Projects Releases Wiki Activity

io_write_wire: always make a copy (or steal if take).

Browse Source 
I caught the gossip daemon freeing a message, while it was queued to be
written.  Using tal_dup_arr() is the Right Thing, as it handles taken()
properly automatically.

------------------------------- Valgrind errors --------------------------------
Valgrind error file: /tmp/lightning-rvc7d5oi/test_forward/lightning-3/valgrind-errors
==11057== Invalid read of size 8
==11057==    at 0x1328F2: to_tal_hdr (tal.c:174)
==11057==    by 0x133894: tal_len (tal.c:659)
==11057==    by 0x11BBE7: do_write_wire (wire_io.c:103)
==11057==    by 0x127B95: do_plan (io.c:369)
==11057==    by 0x127C31: io_ready (io.c:390)
==11057==    by 0x129461: io_loop (poll.c:295)
==11057==    by 0x10CBB4: main (gossip.c:722)
==11057==  Address 0x55a99d8 is 24 bytes inside a block of size 200 free'd
==11057==    at 0x4C2ED5B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11057==    by 0x133000: del_tree (tal.c:416)
==11057==    by 0x132F77: del_tree (tal.c:405)
==11057==    by 0x13333E: tal_free (tal.c:504)
==11057==    by 0x1123F1: queue_broadcast (broadcast.c:38)
==11057==    by 0x111EB0: handle_node_announcement (routing.c:918)
==11057==    by 0x10B166: handle_gossip_msg (gossip.c:170)
==11057==    by 0x10B76B: owner_msg_in (gossip.c:335)
==11057==    by 0x12712E: next_plan (io.c:59)
==11057==    by 0x127BD0: do_plan (io.c:376)
==11057==    by 0x127C09: io_ready (io.c:386)
==11057==    by 0x129461: io_loop (poll.c:295)
==11057==  Block was alloc'd at
==11057==    at 0x4C2DB2F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11057==    by 0x132AE7: allocate (tal.c:245)
==11057==    by 0x1330A3: tal_alloc_ (tal.c:443)
==11057==    by 0x1332A6: tal_alloc_arr_ (tal.c:491)
==11057==    by 0x133FEC: tal_dup_ (tal.c:846)
==11057==    by 0x112347: new_queued_message (broadcast.c:20)
==11057==    by 0x11240B: queue_broadcast (broadcast.c:43)
==11057==    by 0x111EB0: handle_node_announcement (routing.c:918)
==11057==    by 0x10B166: handle_gossip_msg (gossip.c:170)
==11057==    by 0x10B76B: owner_msg_in (gossip.c:335)
==11057==    by 0x12712E: next_plan (io.c:59)
==11057==    by 0x127BD0: do_plan (io.c:376)

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

wire_io: make a copy in io_write_wire (unless taken()).

I hit a corner case where gossipd freed a duplicate while it was being
sent out; this kind of thing doesn't happen if io_write_wire() makes
a copy by default.

We also do a memcheck() here; this gives us a caller in the backtrace
if there are uninitialized bytes, rather than waiting until the write
which happens later.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell 2017-05-19 20:33:28 +09:30 committed by Christian Decker
parent 3d2f166364
commit 55510ea27a
1 changed files with 4 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
wire/wire_io.c
View File

@ -1,6 +1,7 @@
#include <ccan/endian/endian.h> #include <ccan/endian/endian.h>
/* FIXME: io_plan needs size_t */ /* FIXME: io_plan needs size_t */
#include <unistd.h> #include <unistd.h>
#include <ccan/mem/mem.h>
#include <ccan/io/io_plan.h> #include <ccan/io/io_plan.h>
#include <ccan/short_types/short_types.h> #include <ccan/short_types/short_types.h>
#include <ccan/take/take.h> #include <ccan/take/take.h>
@ -115,8 +116,7 @@ static int do_write_wire(int fd, struct io_plan_arg *arg)
if (arg->u2.s != totlen) if (arg->u2.s != totlen)
return 0; return 0;
if (taken(arg->u1.cp)) tal_free(arg->u1.cp);
tal_free(arg->u1.cp);
return 1; return 1;
} }
@ -128,7 +128,8 @@ struct io_plan *io_write_wire_(struct io_conn *conn,
{ {
struct io_plan_arg *arg = io_plan_arg(conn, IO_OUT); struct io_plan_arg *arg = io_plan_arg(conn, IO_OUT);
arg->u1.const_vp = data; arg->u1.const_vp = tal_dup_arr(conn, u8, memcheck(data, tal_len(data)),
tal_len(data), 0);
/* We use u2 to store the length we've written. */ /* We use u2 to store the length we've written. */
arg->u2.s = INSIDE_HEADER_BIT; arg->u2.s = INSIDE_HEADER_BIT;