Bug#673583: apt-pkg/contrib/strutl.cc: ReadMessages() deadlocks if message is split badly
Package: apt
Version: 0.9.3
Severity: normal
Hi,
I was playing around writing my own little method for apt and apt
always hangs. Comparing my output with the output for other methods I
could see no difference so I started looking at the apt source to see
where it hangs.
apt-pkg/contrib/strutl.cc:
// ReadMessages - Read messages from the FD /*{{{*/
// ---------------------------------------------------------------------
/* This pulls full messages from the input FD into the message buffer.
It assumes that messages will not pause during transit so no
fancy buffering is used.
Line 753 and following:
for (char *I = Buffer; I + 1 < End; I++)
{
if (I[0] != '\n' || I[1] != '\n')
continue;
This waits for "\n\n" to be in the Buffer.
// Pull the message out
string Message(Buffer,I-Buffer);
PartialMessage += Message;
// Fix up the buffer
for (; I < End && *I == '\n'; I++);
End -= I-Buffer;
memmove(Buffer,I,End-Buffer);
I = Buffer;
List.push_back(PartialMessage);
PartialMessage.clear();
}
if (End != Buffer)
{
// If there's text left in the buffer, store it
// in PartialMessage and throw the rest of the buffer
// away. This allows us to handle messages that
// are longer than the static buffer size.
PartialMessage += string(Buffer, End);
End = Buffer;
}
And this puts the partial message recieved so far into PartialMessage
and clears the Buffer.
Now what happens if the partial message is
'100 Capabilities
Version: 1.2
Pipeline: true
Send-Config: true
'
and on the next iteration of the while loop the message ends with:
'
'
Because the double newline is split in two ReadMessage never notices
the end of the message.
[pid 30598] <... read resumed> "Send-Config: true\n", 64000) = 18
[pid 30598] select(6, [5], NULL, NULL, NULL <unfinished ...>
[pid 30598] <... select resumed> ) = 1 (in [5])
[pid 30598] read(5, <unfinished ...>
[pid 30598] <... read resumed> "\n", 64000) = 1
[pid 30598] select(6, [5], NULL, NULL, NULL <unfinished ...>
Apt gets stuck in WaitFd(Fd) waiting for a double newline it has
already recieved.
I think the simplest fix would be in Line 777. Don't store all
of the partial message in PartialMessage. Keep the last character in
the buffer like so:
--- apt-0.9.3/apt-pkg/contrib/strutl.cc.orig 2012-05-19 23:37:04.000000000 +0200
+++ apt-0.9.3/apt-pkg/contrib/strutl.cc 2012-05-20 00:50:30.000000000 +0200
@@ -774,8 +774,9 @@
// in PartialMessage and throw the rest of the buffer
// away. This allows us to handle messages that
// are longer than the static buffer size.
- PartialMessage += string(Buffer, End);
- End = Buffer;
+ PartialMessage += string(Buffer, End - 1);
+ Buffer[0] = End[-1];
+ End = Buffer + 1;
}
else
{
Line 782:
// BUG ALERT: if a message block happens to end at a
// multiple of 64000 characters, this will cause it to
// terminate early, leading to a badly formed block and
// probably crashing the method. However, this is the only
// way we have to find the end of the message block. I have
// an idea of how to fix this, but it will require changes
// to the protocol (essentially to mark the beginning and
// end of the block).
//
// -- dburrows 2008-04-02
return true;
This looks wrong to me. This code path is entered whenever the read()
returns after reading exactly up to the end of a message (double
newline). It has nothing to do with a message ending at a multiple of
64000 characters. At that point all recieved messages are queued up
and I assume after processing them apt will try to read more messages
and will start right at the begining of the next message. Adding a
debug message there shows that this is triggered often without any
adverse effects.
Where exactly is the message block badly formed there?
And one last thing I'm unsure about:
Line 744:
// No data
if (Res < 0 && errno == EAGAIN)
return true;
If this ever occurs then there is something wrong. ReadMessage is
called (at least where I checked) only after WaitFd(InFD), meaning
when there is something to read from and the wile loop also calls
WaitFD(Fd) for repeated reads.
At this place an errno of EAGAIN would indicate that someone called
ReadMessage when there was nothing to read (forgot to WaitFD()). Since
that is a waste of cpu time wouldn't it make sense to give a warning
in the debug output there so the caller can be fixed?
MfG
Goswin
-- Package-specific info:
-- (/etc/apt/preferences present, but not submitted) --
-- (/etc/apt/sources.list present, but not submitted) --
-- System Information:
Debian Release: wheezy/sid
Architecture: amd64 (x86_64)
Kernel: Linux 3.2.0-2-amd64 (SMP w/4 CPU cores)
Locale: LANG=C, LC_CTYPE=de_DE (charmap=ISO-8859-1)
Shell: /bin/sh linked to /bin/dash
Versions of packages apt depends on:
ii debian-archive-keyring 2010.08.28
ii gnupg 1.4.12-4
ii libc6 2.13-27
ii libgcc1 1:4.6.2-5
ii libstdc++6 4.6.2-5
ii zlib1g 1:1.2.3.4.dfsg-3
apt recommends no packages.
Versions of packages apt suggests:
pn apt-doc 0.8.15.10
pn aptitude 0.6.4-1.2
pn bzip2 1.0.6-1
pn dpkg-dev 1.16.1.2
pn lzma 9.22-2
pn python-apt <none>
-- no debconf information
Reply to: