What steps will reproduce the problem? 1. do "cat /dev/zero | nc -q1 127.0.0.1 11211" 2. Wait a short while 3. Watch the server crash

What is the expected output? What do you see instead?

The server should probably disconnect the client on such bad input

What version of the product are you using? On what operating system?

memcached 1.2.8

ubuntu jaunty

Please provide any additional information below.

The problem here is that the client will try to spool the data it receives from the client until it receives '\n' (indicating that it got the complete command). In this case we never receive '\n' so we end up reallocating the input buffer until realloc fails, causing the client connection to be shut down. My server didn't crash, but that could be caused other mallocs succeeding (and failing in your case). There might therefore be a path through the code that doesn't handle a malloc failure.

To protect us against someone eating up all of the memory in one buffer, I suggest that we just shut down the connection if we receive more than KEY_MAX_LENGTH + let's say 100 bytes (command, flags, length, cas etc) without seeing '\n'.

Another part of the problem is try_read_network. in this function we try to read out all of the data from the socket reallocating the input buffer to a bigger buffer if there is more data available. In the test-case above we might never break out of this loop, because one core can fill the buffer while the other core on the system is busy reading and reallocating. As a workaround for this we should jump back out after a low number of reallocs.

Proposed fix at: http://github.com/trondn/memcached/tree/issue_102

This doesn't seem right. Doesn't that make multiget not work, or am I not understanding?

I think the first, say, 16 bytes needs to look like a valid command at the very least. Your code makes sense for anything except get, so perhaps it'd be OK and get can be special cased?

It'd also be good to do away with the "allow any arbitrary whitespace before commands" stuff in the protocol. That'd simplify it down to just checking the first byte for most things.

Latest changes look good. Thanks! :)