From d14d2a69af2d64e8d15b3d0e2ef13aeacea58068 Mon Sep 17 00:00:00 2001 From: Andre Noll Date: Thu, 8 Apr 2010 03:07:50 +0200 Subject: [PATCH] para_client: Fix unreliable uses of FD_ISSET(). The client_recv_buffer() function of client_common.c did not check for EAGAIN. Use the new nonblock API if possible that gets things just right and add a check for EAGAIN for RC4 encrypted connections. --- client_common.c | 104 +++++++++++++++++++++++++----------------------- 1 file changed, 55 insertions(+), 49 deletions(-) diff --git a/client_common.c b/client_common.c index f34f81bf..593cb2c0 100644 --- a/client_common.c +++ b/client_common.c @@ -107,17 +107,29 @@ static void client_pre_select(struct sched *s, struct task *t) } } -static ssize_t client_recv_buffer(struct client_task *ct, char *buf, size_t len) +static int client_recv_buffer(struct client_task *ct, fd_set *rfds, + char *buf, size_t sz, size_t *n) { - ssize_t ret; + int ret; if (ct->status < CL_SENT_CH_RESPONSE) - ret = recv_buffer(ct->rc4c.fd, buf, len); - else - ret = rc4_recv_buffer(&ct->rc4c, buf, len); + return read_nonblock(ct->rc4c.fd, buf, sz, rfds, n); + + *n = 0; + ret = rc4_recv_buffer(&ct->rc4c, buf, sz); + /* + * rc4_recv_buffer is used with blocking fds elsewhere, so it + * does not use the nonblock-API. Therefore we need to + * check for EOF and EAGAIN. + */ if (ret == 0) return -E_SERVER_EOF; - return ret; + if (ret == -ERRNO_TO_PARA_ERROR(EAGAIN)) + return 0; + if (ret < 0) + return ret; + *n = ret; + return 0; } /** @@ -138,6 +150,7 @@ static void client_post_select(struct sched *s, struct task *t) struct client_task *ct = container_of(t, struct client_task, task); struct btr_node *btrn = ct->btrn; int ret = 0; + size_t n; char buf[CLIENT_BUFSIZE]; t->error = 0; @@ -145,11 +158,9 @@ static void client_post_select(struct sched *s, struct task *t) return; switch (ct->status) { case CL_CONNECTED: /* receive welcome message */ - if (!FD_ISSET(ct->rc4c.fd, &s->rfds)) - return; - ret = client_recv_buffer(ct, buf, sizeof(buf)); - if (ret < 0) - goto err; + ret = client_recv_buffer(ct, &s->rfds, buf, sizeof(buf), &n); + if (ret < 0 || n == 0) + goto out; ct->status = CL_RECEIVED_WELCOME; return; case CL_RECEIVED_WELCOME: /* send auth command */ @@ -173,14 +184,12 @@ static void client_post_select(struct sched *s, struct task *t) /* the SHA1 of the decrypted challenge */ unsigned char challenge_sha1[HASH_SIZE]; - if (!FD_ISSET(ct->rc4c.fd, &s->rfds)) - return; - ret = client_recv_buffer(ct, buf, sizeof(buf)); - if (ret < 0) - goto err; - PARA_INFO_LOG("<-- [challenge] (%d bytes)\n", ret); + ret = client_recv_buffer(ct, &s->rfds, buf, sizeof(buf), &n); + if (ret < 0 || n == 0) + goto out; + PARA_INFO_LOG("<-- [challenge] (%zu bytes)\n", n); ret = para_decrypt_buffer(ct->key_file, crypt_buf, - (unsigned char *)buf, ret); + (unsigned char *)buf, n); if (ret < 0) goto err; sha1_hash((char *)crypt_buf, CHALLENGE_SIZE, challenge_sha1); @@ -199,19 +208,15 @@ static void client_post_select(struct sched *s, struct task *t) } case CL_SENT_CH_RESPONSE: /* read server response */ { - size_t bytes_received; - if (!FD_ISSET(ct->rc4c.fd, &s->rfds)) - return; - ret = client_recv_buffer(ct, buf, sizeof(buf)); - if (ret < 0) - goto err; - bytes_received = ret; + ret = client_recv_buffer(ct, &s->rfds, buf, sizeof(buf), &n); + if (ret < 0 || n == 0) + goto out; /* check if server has sent "Proceed" message */ ret = -E_CLIENT_AUTH; - if (bytes_received < PROCEED_MSG_LEN) - goto err; + if (n < PROCEED_MSG_LEN) + goto out; if (!strstr(buf, PROCEED_MSG)) - goto err; + goto out; ct->status = CL_RECEIVED_PROCEED; return; } @@ -239,23 +244,20 @@ static void client_post_select(struct sched *s, struct task *t) case CL_SENT_COMMAND: { char *buf2; - if (!FD_ISSET(ct->rc4c.fd, &s->rfds)) - return; /* can not use "buf" here because we need a malloced buffer */ buf2 = para_malloc(CLIENT_BUFSIZE); - ret = client_recv_buffer(ct, buf2, CLIENT_BUFSIZE); - if (ret < 0) { + ret = client_recv_buffer(ct, &s->rfds, buf2, CLIENT_BUFSIZE, &n); + if (n > 0) { + if (strstr(buf2, AWAITING_DATA_MSG)) { + free(buf2); + ct->status = CL_SENDING; + return; + } + ct->status = CL_RECEIVING; + btr_add_output(buf2, n, btrn); + } else free(buf2); - goto err; - } - if (strstr(buf2, AWAITING_DATA_MSG)) { - free(buf2); - ct->status = CL_SENDING; - return; - } - ct->status = CL_RECEIVING; - btr_add_output(buf2, ret, btrn); - return; + goto out; } case CL_SENDING: { @@ -283,20 +285,24 @@ static void client_post_select(struct sched *s, struct task *t) goto err; if (ret == 0) return; + /* + * The FD_ISSET() is not strictly necessary, but is allows us + * to skip the malloc below if there is nothing to read anyway. + */ if (!FD_ISSET(ct->rc4c.fd, &s->rfds)) return; buf2 = para_malloc(CLIENT_BUFSIZE); - ret = client_recv_buffer(ct, buf2, CLIENT_BUFSIZE); - if (ret < 0) { + ret = client_recv_buffer(ct, &s->rfds, buf2, CLIENT_BUFSIZE, &n); + if (n > 0) { + buf2 = para_realloc(buf2, n); + btr_add_output(buf2, n, btrn); + } else free(buf2); - goto err; - } - buf2 = para_realloc(buf2, ret); - btr_add_output(buf2, ret, btrn); - return; + goto out; } } err: +out: t->error = ret; if (ret < 0) { if (ret != -E_SERVER_EOF && ret != -E_BTR_EOF) -- 2.39.5