The Samba-Bugzilla – Attachment 6737 Details for
Bug 8319
Intermittent "Invalid packet of length 0" errors in recoverd
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch to make queue_io_read() safe for reentry
0001-io-Make-queue_io_read-safe-for-reentry.patch (text/plain), 5.27 KB, created by
David Disseldorp
on 2011-07-31 02:08:36 UTC
(
hide
)
Description:
Patch to make queue_io_read() safe for reentry
Filename:
MIME Type:
Creator:
David Disseldorp
Created:
2011-07-31 02:08:36 UTC
Size:
5.27 KB
patch
obsolete
>From 7e1b948a8a8f6155b04426322245b72a2f7ea661 Mon Sep 17 00:00:00 2001 >From: David Disseldorp <ddiss@suse.de> >Date: Sun, 31 Jul 2011 03:14:54 +0200 >Subject: [PATCH] io: Make queue_io_read() safe for reentry > >queue_io_read() may be reentered via the queue callback, recoverd is >particularly guilty of this. > >queue_io_read() is not safe for reentry if more than one packet is >received and partial chunks follow - data read off the pipe on re-entry >is assumed to be the start-of-packet four byte length. This leads to a >wrongly aligned stream and the notorious "Invalid packet of length 0" >errors. > >This change fixes queue_io_read() to be safe under reentry, only a >single packet is processed per call. > >https://bugzilla.samba.org/show_bug.cgi?id=8319 >--- > common/ctdb_io.c | 121 +++++++++++++++++++++++------------------------------ > 1 files changed, 53 insertions(+), 68 deletions(-) > >diff --git a/common/ctdb_io.c b/common/ctdb_io.c >index b7feed9..a7a9768 100644 >--- a/common/ctdb_io.c >+++ b/common/ctdb_io.c >@@ -64,103 +64,88 @@ int ctdb_queue_length(struct ctdb_queue *queue) > > /* > called when an incoming connection is readable >+ This function MUST be safe for reentry via the queue callback! > */ > static void queue_io_read(struct ctdb_queue *queue) > { > int num_ready = 0; >+ uint32_t sz_bytes_req; >+ uint32_t pkt_size; >+ uint32_t pkt_bytes_remaining; >+ uint32_t to_read; > ssize_t nread; >- uint8_t *data, *data_base; >+ uint8_t *data; > > if (ioctl(queue->fd, FIONREAD, &num_ready) != 0) { > return; > } >+ > if (num_ready == 0) { > /* the descriptor has been closed */ > goto failed; > } > >- >- queue->partial.data = talloc_realloc_size(queue, queue->partial.data, >- num_ready + queue->partial.length); >- > if (queue->partial.data == NULL) { >- DEBUG(DEBUG_ERR,("read error alloc failed for %u\n", >- num_ready + queue->partial.length)); >- goto failed; >- } >- >- nread = read(queue->fd, queue->partial.data + queue->partial.length, num_ready); >- if (nread <= 0) { >- DEBUG(DEBUG_ERR,("read error nread=%d\n", (int)nread)); >- goto failed; >+ /* starting fresh, allocate buf for size bytes */ >+ sz_bytes_req = sizeof(pkt_size); >+ queue->partial.data = talloc_size(queue, sizeof(pkt_size)); >+ } else if (queue->partial.length < sizeof(pkt_size)) { >+ /* yet to find out the packet length */ >+ sz_bytes_req = sizeof(pkt_size) - queue->partial.length; >+ } else { >+ /* partial packet, length known, full buf allocated */ >+ sz_bytes_req = 0; > } >- >- > data = queue->partial.data; >- nread += queue->partial.length; >- >- queue->partial.data = NULL; >- queue->partial.length = 0; > >- if (nread >= 4 && *(uint32_t *)data == nread) { >- /* it is the responsibility of the incoming packet >- function to free 'data' */ >- queue->callback(data, nread, queue->private_data); >- return; >- } >- >- data_base = data; >- >- while (nread >= 4 && *(uint32_t *)data <= nread) { >- /* 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 */ >- DEBUG(DEBUG_CRIT,("Invalid packet of length 0\n")); >- goto failed; >- } >- d2 = talloc_memdup(queue, data, len); >- if (d2 == NULL) { >- DEBUG(DEBUG_ERR,("read error memdup failed for %u\n", len)); >- /* sigh */ >+ if (sz_bytes_req > 0) { >+ to_read = MIN(sz_bytes_req, num_ready); >+ nread = read(queue->fd, data + queue->partial.length, >+ to_read); >+ if (nread <= 0) { >+ DEBUG(DEBUG_ERR,("read error nread=%d\n", (int)nread)); > goto failed; > } >+ queue->partial.length += nread; > >- queue->destroyed = &destroyed; >- queue->callback(d2, len, queue->private_data); >- /* If callback freed us, don't do anything else. */ >- if (destroyed) { >+ if (nread < sz_bytes_req) { >+ /* not enough to know the length */ >+ DEBUG(DEBUG_DEBUG,("Partial packet length read\n")); > return; > } >- queue->destroyed = NULL; >+ /* size now known, allocate buffer for the full packet */ >+ queue->partial.data = talloc_realloc_size(queue, data, >+ *(uint32_t *)data); >+ data = queue->partial.data; >+ num_ready -= nread; >+ } > >- data += len; >- nread -= len; >+ pkt_size = *(uint32_t *)data; >+ if (pkt_size == 0) { >+ DEBUG(DEBUG_CRIT,("Invalid packet of length 0\n")); >+ goto failed; > } > >- if (nread > 0) { >- /* we have only part of a packet */ >- if (data_base == data) { >- queue->partial.data = data; >- queue->partial.length = nread; >- } else { >- queue->partial.data = talloc_memdup(queue, data, nread); >- if (queue->partial.data == NULL) { >- DEBUG(DEBUG_ERR,("read error memdup partial failed for %u\n", >- (unsigned)nread)); >- goto failed; >- } >- queue->partial.length = nread; >- talloc_free(data_base); >- } >+ pkt_bytes_remaining = pkt_size - queue->partial.length; >+ to_read = MIN(pkt_bytes_remaining, num_ready); >+ nread = read(queue->fd, data + queue->partial.length, >+ to_read); >+ if (nread <= 0) { >+ DEBUG(DEBUG_ERR,("read error nread=%d\n", >+ (int)nread)); >+ goto failed; >+ } >+ queue->partial.length += nread; >+ >+ if (queue->partial.length < pkt_size) { >+ DEBUG(DEBUG_DEBUG,("Partial packet data read\n")); > return; > } > >- talloc_free(data_base); >+ queue->partial.data = NULL; >+ queue->partial.length = 0; >+ /* it is the responsibility of the callback to free 'data' */ >+ queue->callback(data, pkt_size, queue->private_data); > return; > > failed: >-- >1.7.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Actions:
View
Attachments on
bug 8319
:
6715
|
6737
|
6738
|
6742
|
6743