Ian Campbell
2008-Apr-30 16:32 UTC
[Xen-devel] [XENSTORED] Fix xenstored abort when connection dropped.
[XENSTORED] Fix xenstored abort when connection dropped.
If a connection is dropped with pending input and output data then the
connection will be dereferenced by both handle_input and handle_output
resulting in a double free when the main loop dereferences the
connection.
Fix this issue by taking/releasing a reference over the calls to
handle_input and handle_output separately and checking the result of
talloc_free to see if the connection went away.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
The attached t.c will show you the crash in a few seconds at most
# gcc ~/t.c
# while ./a.out ; do : ; done
main: Connection refused
Aborted
Ian.
--=-gCKqkrg026Oni0mXNRuV
Content-Disposition: attachment; filename=xenstore-abort.patch
Content-Type: text/x-patch; name=xenstore-abort.patch; charset=UTF-8
Content-Transfer-Encoding: 7bit
# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1209573032 -3600
# Node ID a7dc490dcec043804f4b615a6ffa64eb6e534e06
# Parent 483d006cc60765357dcdb66ab0fc43c955ecd8fe
[XENSTORED] Fix xenstored abort when connection dropped.
If a connection is dropped with pending input and output data then the
connection will be dereferenced by both handle_input and handle_output
resulting in a double free when the main loop dereferences the
connection.
Fix this issue by taking/releasing a reference over the calls to
handle_input and handle_output separately and checking the result of
talloc_free to see if the connection went away.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
diff -r 483d006cc607 -r a7dc490dcec0 tools/xenstore/xenstored_core.c
--- a/tools/xenstore/xenstored_core.c Fri Apr 25 13:46:27 2008 +0100
+++ b/tools/xenstore/xenstored_core.c Wed Apr 30 17:30:32 2008 +0100
@@ -1929,7 +1929,7 @@ int main(int argc, char *argv[])
/* Main loop. */
for (;;) {
- struct connection *conn, *old_conn;
+ struct connection *conn, *next;
if (select(max+1, &inset, &outset, NULL, timeout) < 0) {
if (errno == EINTR)
@@ -1953,27 +1953,39 @@ int main(int argc, char *argv[])
if (evtchn_fd != -1 && FD_ISSET(evtchn_fd, &inset))
handle_event();
- conn = list_entry(connections.next, typeof(*conn), list);
- while (&conn->list != &connections) {
- talloc_increase_ref_count(conn);
+ next = list_entry(connections.next, typeof(*conn), list);
+ while (&next->list != &connections) {
+ conn = next;
+
+ next = list_entry(conn->list.next,
+ typeof(*conn), list);
if (conn->domain) {
+ talloc_increase_ref_count(conn);
if (domain_can_read(conn))
handle_input(conn);
+ if (talloc_free(conn) == 0)
+ continue;
+
+ talloc_increase_ref_count(conn);
if (domain_can_write(conn) &&
!list_empty(&conn->out_list))
handle_output(conn);
+ if (talloc_free(conn) == 0)
+ continue;
} else {
+ talloc_increase_ref_count(conn);
if (FD_ISSET(conn->fd, &inset))
handle_input(conn);
+ if (talloc_free(conn) == 0)
+ continue;
+
+ talloc_increase_ref_count(conn);
if (FD_ISSET(conn->fd, &outset))
handle_output(conn);
+ if (talloc_free(conn) == 0)
+ continue;
}
-
- old_conn = conn;
- conn = list_entry(old_conn->list.next,
- typeof(*conn), list);
- talloc_free(old_conn);
}
max = initialize_set(&inset, &outset, *sock, *ro_sock,
--=-gCKqkrg026Oni0mXNRuV
Content-Disposition: attachment; filename=t.c
Content-Type: text/x-csrc; name=t.c; charset=UTF-8
Content-Transfer-Encoding: 7bit
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <fcntl.h>
#include <string.h>
#include <stdint.h>
#include <sys/socket.h>
#include <sys/un.h>
#define SOCKET "/var/run/xenstored/socket"
#define READ_PATH "/"
enum xsd_sockmsg_type
{
XS_DEBUG,
XS_DIRECTORY,
XS_READ,
XS_GET_PERMS,
XS_WATCH,
XS_UNWATCH,
XS_TRANSACTION_START,
XS_TRANSACTION_END,
XS_INTRODUCE,
XS_RELEASE,
XS_GET_DOMAIN_PATH,
XS_WRITE,
XS_MKDIR,
XS_RM,
XS_SET_PERMS,
XS_WATCH_EVENT,
XS_ERROR,
XS_IS_DOMAIN_INTRODUCED,
XS_RESUME,
XS_RESTRICT
};
struct xsd_sockmsg
{
uint32_t type; /* XS_??? */
uint32_t req_id;/* Request identifier, echoed in daemon''s response.
*/
uint32_t tx_id; /* Transaction id (0 if not related to a transaction). */
uint32_t len; /* Length of data following this. */
/* Generally followed by nul-terminated string(s). */
};
int main(void)
{
struct sockaddr_un addr;
int sock, saved_errno, flags;
struct xsd_sockmsg hdr;
sock = socket(PF_UNIX, SOCK_STREAM, 0);
if (sock < 0)
return -1;
if ((flags = fcntl(sock, F_GETFD)) < 0)
goto error;
flags |= FD_CLOEXEC;
if (fcntl(sock, F_SETFD, flags) < 0)
goto error;
addr.sun_family = AF_UNIX;
strcpy(addr.sun_path, SOCKET);
if (connect(sock, (struct sockaddr *)&addr, sizeof(addr)) != 0)
goto error;
hdr.type = XS_READ;
hdr.req_id = 1;
hdr.tx_id = 0;
hdr.len = strlen(READ_PATH);
write(sock, &hdr, sizeof(hdr));
write(sock, READ_PATH, strlen(READ_PATH));
return 0;
error:
perror("main");
abort();
}
--=-gCKqkrg026Oni0mXNRuV
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
--=-gCKqkrg026Oni0mXNRuV--