Skip to content

Commit

Permalink
Fix for issue 72: 1.4.0 binary multi-get crash issue
Browse files Browse the repository at this point in the history
Repack the input buffer if the current command and key doesn't fit in the
input buffer (may occur if we read multiple commands in a pipeline)

(Dustin's note):

The original test was connecting to "localhost" which was resolving to
::1 on a few machines.  With the ephemeral port binding, IPv4 and IPv6
were getting different ports, and the code was explicitly looking for
the IPv4 port, but implicitly connecting to ::1.  Now it explicitly
connects to 127.0.0.1 over IPv4.
  • Loading branch information
Trond Norbye authored and dustin committed Aug 15, 2009
1 parent b8e106f commit 7bb8219
Show file tree
Hide file tree
Showing 2 changed files with 120 additions and 1 deletion.
41 changes: 40 additions & 1 deletion memcached.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
#include <assert.h>
#include <limits.h>
#include <sysexits.h>
#include <stddef.h>

/* FreeBSD 4.x doesn't have IOV_MAX exposed. */
#ifndef IOV_MAX
Expand Down Expand Up @@ -1416,7 +1417,45 @@ static void bin_read_key(conn *c, enum bin_substates next_substate, int extra) {
assert(c);
c->substate = next_substate;
c->rlbytes = c->keylen + extra;
assert(c->rsize >= c->rlbytes);

/* Ok... do we have room for the extras and the key in the input buffer? */
ptrdiff_t offset = c->rcurr + sizeof(protocol_binary_request_header) - c->rbuf;
if (c->rlbytes > c->rsize - offset) {
size_t nsize = c->rsize;
size_t size = c->rlbytes + sizeof(protocol_binary_request_header);

while (size > nsize) {
nsize *= 2;
}

if (nsize != c->rsize) {
if (settings.verbose) {
fprintf(stderr, "%d: Need to grow buffer from %lu to %lu\n",
c->sfd, (unsigned long)c->rsize, (unsigned long)nsize);
}
char *newm = realloc(c->rbuf, nsize);
if (newm == NULL) {
if (settings.verbose) {
fprintf(stderr, "%d: Failed to grow buffer.. closing connection\n",
c->sfd);
}
conn_set_state(c, conn_closing);
return;
}

/* rcurr should point to the same offset in the packet */
c->rcurr = c->rbuf + offset - sizeof(protocol_binary_request_header);
c->rsize = nsize;
}
if (c->rbuf != c->rcurr) {
memmove(c->rbuf, c->rcurr, c->rbytes);
c->rcurr = c->rbuf;
if (settings.verbose) {
fprintf(stderr, "%d: Repack input buffer\n", c->sfd);
}
}
}

/* preserve the header in the buffer.. */
c->ritem = c->rcurr + sizeof(protocol_binary_request_header);
conn_set_state(c, conn_nread);
Expand Down
80 changes: 80 additions & 0 deletions testapp.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
/* -*- Mode: C; tab-width: 4; c-basic-offset: 4; indent-tabs-mode: nil -*- */
#undef NDEBUG
#include <sys/types.h>
#include <sys/socket.h>
#include <netdb.h>
#include <arpa/inet.h>
#include <netinet/in.h>
#include <netinet/tcp.h>
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
Expand All @@ -11,6 +17,7 @@
#include <unistd.h>
#include <netinet/in.h>

#include "protocol_binary.h"
#include "config.h"
#include "cache.h"
#include "util.h"
Expand Down Expand Up @@ -333,6 +340,78 @@ static enum test_return test_issue_44(void) {
return TEST_PASS;
}

static struct addrinfo *lookuphost(const char *hostname, in_port_t port)
{
struct addrinfo *ai = 0;
struct addrinfo hints = { .ai_family = AF_UNSPEC,
.ai_protocol = IPPROTO_TCP,
.ai_socktype = SOCK_STREAM };
char service[NI_MAXSERV];
int error;

(void)snprintf(service, NI_MAXSERV, "%d", port);
if ((error = getaddrinfo(hostname, service, &hints, &ai)) != 0) {
if (error != EAI_SYSTEM) {
fprintf(stderr, "getaddrinfo(): %s\n", gai_strerror(error));
} else {
perror("getaddrinfo()");
}
}

return ai;
}

static int connect_server(const char *hostname, in_port_t port)
{
struct addrinfo *ai = lookuphost(hostname, port);
int sock = -1;
if (ai != NULL) {
if ((sock = socket(ai->ai_family, ai->ai_socktype,
ai->ai_protocol)) != -1) {
if (connect(sock, ai->ai_addr, ai->ai_addrlen) == -1) {
fprintf(stderr, "Failed to connect socket: %s\n",
strerror(errno));
close(sock);
sock = -1;
}
} else {
fprintf(stderr, "Failed to create socket: %s\n", strerror(errno));
}

freeaddrinfo(ai);
}
return sock;
}


static enum test_return test_issue_72(void) {
in_port_t port;
pid_t pid = start_server(&port, false);
int sock = connect_server("127.0.0.1", port);
assert(sock != -1);

char data[sizeof(protocol_binary_request_set) + 2048] = { 0 };
protocol_binary_request_set *request = (protocol_binary_request_set*)data;
request->message.header.request.magic = PROTOCOL_BINARY_REQ;
request->message.header.request.opcode = PROTOCOL_BINARY_CMD_SET;
uint16_t keylen = 2048;
request->message.header.request.keylen = htons(keylen);
request->message.header.request.extlen = 8;
request->message.header.request.bodylen = htonl(keylen + 8);

assert(write(sock, data, 2000) == 2000);
usleep(250);
assert(write(sock, data, sizeof(data) - 2000) == sizeof(data) - 2000);

protocol_binary_response_set response;
assert(read(sock, &response, sizeof(response)) == sizeof(response));
assert(response.message.header.response.magic == PROTOCOL_BINARY_RES);
assert(response.message.header.response.status == PROTOCOL_BINARY_RESPONSE_SUCCESS);
close(sock);
assert(kill(pid, SIGTERM) == 0);
return TEST_PASS;
}

typedef enum test_return (*TEST_FUNC)(void);
struct testcase {
const char *description;
Expand All @@ -351,6 +430,7 @@ struct testcase testcases[] = {
{ "strtoul", test_safe_strtoul },
{ "strtoull", test_safe_strtoull },
{ "issue_44", test_issue_44 },
{ "issue_72", test_issue_72 },
{ NULL, NULL }
};

Expand Down

0 comments on commit 7bb8219

Please sign in to comment.