samba-bugs@samba.org
2007-Nov-12 06:06 UTC
DO NOT REPLY [Bug 5075] New: Syncing with --iconv may yield protocol error
https://bugzilla.samba.org/show_bug.cgi?id=5075
Summary: Syncing with --iconv may yield protocol error
Product: rsync
Version: 3.0.0
Platform: All
OS/Version: All
Status: NEW
Severity: major
Priority: P3
Component: core
AssignedTo: wayned@samba.org
ReportedBy: lennart.samba@lovstrand.com
QAContact: rsync-qa@samba.org
If you use --iconv to sync between two machines and a file with a name that
needs conversion is to be deleted, the rsync protocol may get out of sync and
yield a protocol error if the converted filename differs in length form the
unconverted name. This is because mplex_write in io.c will record and send the
message length *before* the message has been converted using iconvbuf. If the
converted message differs in length, the reading rsync process will get
confused and interpret part of the message data as tag & length.
Here is an example:
[21:45:27]lenux:~/Mirror$ rsync -a --progress --del -n --iconv=utf-8,utf-8-mac
Lennart/Auto/ sva:Mirror/Lennart/Auto/
sending incremental file list
Audi Cabrio 92 (CUB931)/D\#303\#244ck/
deleting Audi Cabrio 92 (CUB931)/D\#303\#244ck/www.stro.nu.url.URL\#001
unexpected tag -7 [sender]
rsync error: error in rsync protocol data stream (code 12) at io.c(1135)
[sender=3.0.0pre5]
(Note the garbage #\001 at the end of the filename.)
To fix this, mplex_write needs to convert the message data before sending its
length. Here is a possible rewrite:
/* Write an message to a multiplexed stream. If this fails, rsync exits. */
static void mplex_write(int fd, enum msgcode code, const char *buf, size_t len,
int convert)
{
char buffer[BIGPATHBUFLEN]; /* Oversized for use by iconv code. */
size_t n = len;
#ifdef ICONV_OPTION
if (convert && ic_send == (iconv_t)-1)
#endif
convert = 0;
#ifdef ICONV_OPTION
/* We need to convert buf before doing anything else so that we
* can include the (converted) byte length in the message header.
*/
if (convert) {
xbuf outbuf, inbuf;
INIT_XBUF(outbuf, buffer + 4, 0, sizeof(buffer) - 4);
INIT_XBUF(inbuf, (char*)buf, len, -1);
iconvbufs(ic_send, &inbuf, &outbuf,
ICB_INCLUDE_BAD | ICB_INCLUDE_INCOMPLETE);
if (inbuf.len > 0) {
rprintf(FERROR, "conversion buffer overflow in
mplex_write; "
"either enlarge 'buffer' or
rewrite
code to allocate "
"buffer dynamically");
}
n = len = outbuf.len;
} else
#endif
if (n > sizeof(buffer) - 4)
n = 0;
else
memcpy(buffer + 4, buf, n);
SIVAL(buffer, 0, ((MPLEX_BASE + (int)code)<<24) + len);
defer_forwarding_keep = 1; /* defer_forwarding_messages++ on return */
writefd_unbuffered(fd, buffer, n+4);
defer_forwarding_keep = 0;
len -= n;
buf += n;
if (len)
writefd_unbuffered(fd, buf, len);
if (!--defer_forwarding_messages && !no_flush)
msg_flush();
}
Context diff based on 3.0.0pre5 follows:
*** io.c~ Sat Nov 3 00:20:05 2007
--- io.c Sun Nov 11 21:59:57 2007
***************
*** 468,485 ****
char buffer[BIGPATHBUFLEN]; /* Oversized for use by iconv code. */
size_t n = len;
- SIVAL(buffer, 0, ((MPLEX_BASE + (int)code)<<24) + len);
-
#ifdef ICONV_OPTION
if (convert && ic_send == (iconv_t)-1)
#endif
convert = 0;
! if (convert || n > 1024 - 4) /* BIGPATHBUFLEN can handle 1024 bytes
*/
n = 0;
else
memcpy(buffer + 4, buf, n);
defer_forwarding_keep = 1; /* defer_forwarding_messages++ on return */
writefd_unbuffered(fd, buffer, n+4);
defer_forwarding_keep = 0;
--- 468,508 ----
char buffer[BIGPATHBUFLEN]; /* Oversized for use by iconv code. */
size_t n = len;
#ifdef ICONV_OPTION
if (convert && ic_send == (iconv_t)-1)
#endif
convert = 0;
! #ifdef ICONV_OPTION
! /* We need to convert buf before doing anything else so that we
! * can include the (converted) byte length in the message header.
! */
! if (convert) {
! xbuf outbuf, inbuf;
!
! INIT_XBUF(outbuf, buffer + 4, 0, sizeof(buffer) - 4);
! INIT_XBUF(inbuf, (char*)buf, len, -1);
!
! iconvbufs(ic_send, &inbuf, &outbuf,
! ICB_INCLUDE_BAD | ICB_INCLUDE_INCOMPLETE);
!
! if (inbuf.len > 0) {
! rprintf(FERROR, "conversion buffer overflow in
mplex_write; "
! "either enlarge 'buffer' or
rewrite
code to allocate "
! "buffer dynamically");
! }
!
! n = len = outbuf.len;
! } else
! #endif
! if (n > sizeof(buffer) - 4)
n = 0;
else
memcpy(buffer + 4, buf, n);
+ SIVAL(buffer, 0, ((MPLEX_BASE + (int)code)<<24) + len);
+
+
defer_forwarding_keep = 1; /* defer_forwarding_messages++ on return */
writefd_unbuffered(fd, buffer, n+4);
defer_forwarding_keep = 0;
***************
*** 487,506 ****
len -= n;
buf += n;
- #ifdef ICONV_OPTION
- if (convert) {
- xbuf outbuf, inbuf;
-
- INIT_CONST_XBUF(outbuf, buffer);
- INIT_XBUF(inbuf, (char*)buf, len, -1);
-
- do {
- iconvbufs(ic_send, &inbuf, &outbuf,
- ICB_INCLUDE_BAD | ICB_INCLUDE_INCOMPLETE);
- writefd_unbuffered(fd, outbuf.buf, outbuf.len);
- } while (inbuf.len);
- } else
- #endif
if (len)
writefd_unbuffered(fd, buf, len);
--
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.
samba-bugs@samba.org
2007-Nov-12 06:14 UTC
DO NOT REPLY [Bug 5075] Syncing with --iconv may yield protocol error
https://bugzilla.samba.org/show_bug.cgi?id=5075 ------- Comment #1 from lennart.samba@lovstrand.com 2007-11-12 00:14 CST ------- Created an attachment (id=2963) --> (https://bugzilla.samba.org/attachment.cgi?id=2963&action=view) Diffs to fix bug -- Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact.
samba-bugs@samba.org
2007-Nov-12 06:17 UTC
DO NOT REPLY [Bug 5075] Syncing with --iconv may yield protocol error
https://bugzilla.samba.org/show_bug.cgi?id=5075
------- Comment #2 from lennart.samba@lovstrand.com 2007-11-12 00:17 CST
-------
Sorry, I meant to add this too:
There is also an uninitialized variable in readfd_buffered that can cause
garbage to be read independently of the bug in mplex_write. Under case
MSG_DELETED, inbuf.pos is never initialized which can -- and will -- cause the
call to iconvbufs further down to process random data.
The fix is to initialize inbuf with a proper call to INIT_XBUF before
proceeding to do the conversion.
Diff below and attached.
--- 510,515 ----
***************
*** 1060,1066 ****
int pos = 0;
INIT_CONST_XBUF(outbuf, line);
! inbuf.buf = ibuf;
while (msg_bytes) {
inbuf.len = msg_bytes > sizeof ibuf
--- 1069,1075 ----
int pos = 0;
INIT_CONST_XBUF(outbuf, line);
! INIT_XBUF(inbuf, ibuf, 0, -1);
while (msg_bytes) {
inbuf.len = msg_bytes > sizeof ibuf
--
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.
samba-bugs@samba.org
2007-Nov-22 16:09 UTC
DO NOT REPLY [Bug 5075] Syncing with --iconv may yield protocol error
https://bugzilla.samba.org/show_bug.cgi?id=5075
wayned@samba.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |FIXED
------- Comment #3 from wayned@samba.org 2007-11-22 10:09 CST -------
Thank you very much for the problem report and the patch. Both problems are
now fixed in the git version.
--
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.