sin/samba-mirror
1
0
Fork 0
mirror of https://github.com/samba-team/samba.git synced 2024-12-22 13:34:15 +03:00
Code

ctdb_io: fix use-after-free on invalid packets

Browse Source 
Wolfgang saw a talloc complaint about using freed memory in ctdb_tcp_read_cb.
His fix was to remove the talloc_free() in that function, which causes
loops when a socket is closed (as it does not get removed from the event
system), eg:
	netcat 192.168.1.2 4379 < /dev/null

The real bug is that when we have more than one pending packet in the
queue, we loop calling the callback without any safeguards should that
callback free the queue (as it tends to do on invalid packets).  This
can be reproduced by sending more than one bogus packet at once:
	# Length word at start: 4 == empty packet (assumed little endian)
	/usr/bin/printf \\4\\0\\0\\0\\4\\0\\0\\0 > /tmp/pkt
	netcat 192.168.1.2 4379 < /tmp/pkt

Using a destructor we can check if the callback frees us, and exit
immediately.  Elsewhere, we return after the callback anyway.

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

(This used to be ctdb commit 4d0523dd94fb07e860b3e8118691f93d1ef8d0fa)
This commit is contained in:
Rusty Russell 2009-12-02 08:57:42 +10:30 committed by Ronnie Sahlberg
parent 6f045cad29
commit 9e84872ecd
1 changed files with 19 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
ctdb/common/ctdb_io.c
View File

@ -52,6 +52,7 @@ struct ctdb_queue {
size_t alignment;
void *private_data;
ctdb_queue_cb_fn_t callback;
bool *destroyed;
};
@ -114,6 +115,8 @@ static void queue_io_read(struct ctdb_queue *queue)
/* we have at least one packet */
uint8_t *d2;
uint32_t len;
bool destroyed = false;
len = *(uint32_t *)data;
if (len == 0) {
/* bad packet! treat as EOF */
@ -126,7 +129,15 @@ static void queue_io_read(struct ctdb_queue *queue)
/* sigh */
goto failed;
}
queue->destroyed = &destroyed;
queue->callback(d2, len, queue->private_data);
/* If callback freed us, don't do anything else. */
if (destroyed) {
return;
}
queue->destroyed = NULL;
data += len;
nread -= len;
}
@ -337,7 +348,13 @@ int ctdb_queue_set_fd(struct ctdb_queue *queue, int fd)
return 0;
}
/* If someone sets up this pointer, they want to know if the queue is freed */
static int queue_destructor(struct ctdb_queue *queue)
{
if (queue->destroyed != NULL)
*queue->destroyed = true;
return 0;
}
/*
setup a packet queue on a socket
@ -364,6 +381,7 @@ struct ctdb_queue *ctdb_queue_setup(struct ctdb_context *ctdb,
return NULL;
}
}
talloc_set_destructor(queue, queue_destructor);
return queue;
}