memcached_clone of SASL connection closes random file descriptor

Bug #1630615 reported by Alexander Petrossian (PAF)
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
libmemcached
New
Undecided
Unassigned

Bug Description

memcached_clone(0, connection_that_already_configured_to_use_SASL_but_not_connected)
internally calls
memcached_create
that does not initialise fd field.

later SASL data fields are cloned.
that sets BINARY behaviour.
setting that behaviour tries to close existing connection (that was NOT yet made in this use-case):
memcached_behavior_set:
  case MEMCACHED_BEHAVIOR_BINARY_PROTOCOL:
    send_quit(ptr); // We need t shutdown all of the connections to make sure we do the correct protocol

since fd is not initialised, send_quit goes along some random fd number that happen to be in memory that was malloced during memcached_create.

trivial solution:
add
self->fd= INVALID_SOCKET;
_memcached_init in libmemcached/memcached.cc

Revision history for this message
Alexander Petrossian (PAF) (thispaf) wrote :

hmm. too fast.
uninitialized = true.
solution = wrong.
thinking....

Revision history for this message
Alexander Petrossian (PAF) (thispaf) wrote :

unitialized = false ;)

we have a preconfigured source, with one server.
during memcached_clone happens memcached_push which opens new connection.
but later on in memcached_clone happens memcached_clone_sasl which CLOSES freshly opened connection just to... open it again:

gdb bt:
Breakpoint 1, 0x0073aaa0 in shutdown () from /lib/libc.so.6
(gdb) bt
#0 0x0073aaa0 in shutdown () from /lib/libc.so.6
#1 0x080b81e2 in memcached_io_close (ptr=0xf0f09d28) at libmemcached/io.cc:672
#2 0x080baf8c in memcached_quit_server (ptr=0xf0f09d28, io_death=false) at libmemcached/quit.cc:111
#3 0x080bb1ff in send_quit (ptr=0xf0f10468) at libmemcached/quit.cc:140
#4 0x080b1997 in memcached_behavior_set (ptr=0xf0f10468, flag=MEMCACHED_BEHAVIOR_SND_TIMEOUT, data=1) at libmemcached/behavior.cc:99
#5 0x080c0560 in memcached_set_sasl_auth_data (ptr=0xf0f10468, username=0x8893678 "GYH", password=0x8891b4c "teligent") at libmemcached/sasl.cc:342
#6 0x080c0910 in memcached_clone_sasl (clone=0xf0f10468, source=0x8892dc0) at libmemcached/sasl.cc:443
#7 0x080ba0af in memcached_clone (clone=0x0, source=0x8892dc0) at libmemcached/memcached.cc:412
#8 0x080a0dc5 in Box::Backend::memcached_traits::memc_pool_traits::create (this=0x8899704) at backend/memcached_api.cpp:893
#9 0x080a1e0a in obtain (ptr=0x8887bf0, block=true, err=0xf1a4b8b0) at /Users/paf/Documents/BOX/builds/workspace/RHEL5/include/libsinny/pool.h:194

Revision history for this message
Alexander Petrossian (PAF) (thispaf) wrote :

possible solution:
behaviour.cc:

   case MEMCACHED_BEHAVIOR_BINARY_PROTOCOL:
+ if(ptr->flags.binary_protocol == bool(data)) break;

our connection is already binary, and flags were already cloned, and there is no reason on Earth to disconnect it.

Revision history for this message
Alexander Petrossian (PAF) (thispaf) wrote :

no, that alone brought trouble.
current trunk version also has that.
memcached_clone does connect before setting sasl options.

final approach we took and it tested OK:
behavior.cc (memcached_behavior_set):
  case MEMCACHED_BEHAVIOR_BINARY_PROTOCOL:
    if(ptr->flags.binary_protocol != bool(data)) {
      send_quit(ptr); // We need t shutdown all of the connections to make sure we do the correct protocol
    }

memcached.cc (memcached_clone):
  if (LIBMEMCACHED_WITH_SASL_SUPPORT and source->sasl.callbacks)
  {
    if (memcached_failed(memcached_clone_sasl(new_clone, source)))
    {
      memcached_free(new_clone);
      return NULL;
    }
  }

  if (memcached_server_count(source))
  {
    if (memcached_failed(memcached_push(new_clone, source)))
    {
      return NULL;
    }
  }

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.