vss: Avoid use after free in vss_send().

In vss_send() we increment the current slice number for the fec
client after fc->send_fec() has sent a slice. This results in a use
after free in case of a write error because ->send_fec() frees the
fec client structure on write errors. Valgrind complains about this
with the splat below.

To avoid this, the fc pointer must not be dereferenced after
->send_fec() has been called. This patch increases the current slice
number *before* the call to ->send_fec(). This works because the fec
clients do not care about this number.

The bug was introduced eight years ago in commit 625c5cd (Add forward
error correction code to the udp sender/receiver).

	==8615== Invalid read of size 1
	==8615==    at 0x805022B: vss_send (vss.c:1051)
	==8615==    by 0x805022B: vss_post_select (vss.c:1168)
	==8615==    by 0x8061DC7: call_post_select (sched.c:84)
	==8615==    by 0x8061DC7: sched_post_select (sched.c:110)
	==8615==    by 0x8061DC7: schedule (sched.c:163)
	==8615==    by 0x804CBFD: main (server.c:607)
	==8615==  Address 0x4670168 is 80 bytes inside a block of size 116 free'd
	==8615==    at 0x402D221: free (vg_replace_malloc.c:530)
	==8615==    by 0x8062D7C: udp_delete_target (udp_send.c:80)
	==8615==    by 0x80630DC: udp_send_fec (udp_send.c:305)
	==8615==    by 0x805022A: vss_send (vss.c:1049)
	==8615==    by 0x805022A: vss_post_select (vss.c:1168)
	==8615==    by 0x8061DC7: call_post_select (sched.c:84)
	==8615==    by 0x8061DC7: sched_post_select (sched.c:110)
	==8615==    by 0x8061DC7: schedule (sched.c:163)
	==8615==    by 0x804CBFD: main (server.c:607)
	==8615==  Block was alloc'd at
	==8615==    at 0x402C1F0: malloc (vg_replace_malloc.c:299)
	==8615==    by 0x8052D7E: para_malloc (string.c:67)
	==8615==    by 0x8052FBD: para_calloc (string.c:90)
	==8615==    by 0x804F48F: vss_add_fec_client (vss.c:686)
	==8615==    by 0x8063433: udp_com_add (udp_send.c:342)
	==8615==    by 0x8063688: udp_init_target_list (udp_send.c:395)
	==8615==    by 0x806371D: udp_send_init (udp_send.c:442)
	==8615==    by 0x805062A: init_vss_task (vss.c:1195)
	==8615==    by 0x804CA57: server_init (server.c:537)
	==8615==    by 0x804CA57: main (server.c:605)

diff --git a/vss.c b/vss.c
index 5484db9d9479401fe90e481f842c37a9afed8e4f..4d73a95cd142908f26ec9c2108139f1561a4c995 100644 (file)
--- a/vss.c
+++ b/vss.c
@@ -1029,9 +1029,9 @@ static void vss_send(struct vss_task *vsst)
                        continue;
                PARA_DEBUG_LOG("sending %u:%u (%u bytes)\n", fc->group.num,
                        fc->current_slice_num, fc->group.slice_bytes);
+               fc->current_slice_num++;
                fc->fcp->send_fec(fc->sc, (char *)fc->enc_buf,
                        fc->group.slice_bytes + FEC_HEADER_SIZE);
-               fc->current_slice_num++;
                fec_active = 1;
        }
        if (mmd->current_chunk >= mmd->afd.afhi.chunks_total) { /* eof */