diff options
author | dormando <dormando@rydia.net> | 2023-04-12 21:06:58 -0700 |
---|---|---|
committer | dormando <dormando@rydia.net> | 2023-04-12 21:06:58 -0700 |
commit | 4e310166bc1f633430f18f05ff4ac62f40124f31 (patch) | |
tree | 6d14ca981fc94a775e858250a8cdeb7e40754f46 /proxy_lua.c | |
parent | 6a3c554972723e27905924c5db9605e3374a28d4 (diff) | |
download | memcached-4e310166bc1f633430f18f05ff4ac62f40124f31.tar.gz |
proxy: fix data corruption bug
Bug introduced in 6c80728: use after free for response buffer while
under concurrency.
The await code has a different method of wrapping up a lua coroutine
than a standard response, so it was not managing the lifecycle of the
response object properly, causing data buffers to be reused before being
written back to the client.
This fix separates the accounting of memory from the freeing of the
buffer, so there is no more race.
Further restructuring is needed to both make this less bug prone and
make memory accounting be lock step with the memory freeing.
Diffstat (limited to 'proxy_lua.c')
-rw-r--r-- | proxy_lua.c | 9 |
1 files changed, 6 insertions, 3 deletions
diff --git a/proxy_lua.c b/proxy_lua.c index 7f969c5..c3a2af8 100644 --- a/proxy_lua.c +++ b/proxy_lua.c @@ -85,12 +85,15 @@ static int mcplib_response_gc(lua_State *L) { LIBEVENT_THREAD *t = PROXY_GET_THR(L); mcp_resp_t *r = luaL_checkudata(L, -1, "mcp.response"); + // FIXME: we handle the accounting here, but the actual response buffer is + // freed elsewhere, after the network write. + pthread_mutex_lock(&t->proxy_limit_lock); + t->proxy_buffer_memory_used -= r->blen; + pthread_mutex_unlock(&t->proxy_limit_lock); + // On error/similar we might be holding the read buffer. // If the buf is handed off to mc_resp for return, this pointer is NULL if (r->buf != NULL) { - pthread_mutex_lock(&t->proxy_limit_lock); - t->proxy_buffer_memory_used -= r->blen; - pthread_mutex_unlock(&t->proxy_limit_lock); free(r->buf); } |