Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

memcached_clone of SASL connection closes random file descriptor #75

Closed
m6w6 opened this issue Jan 20, 2020 · 4 comments
Closed

memcached_clone of SASL connection closes random file descriptor #75

m6w6 opened this issue Jan 20, 2020 · 4 comments

Comments

@m6w6
Copy link
Collaborator

m6w6 commented Jan 20, 2020

Imported from Launchpad using lp2gh.


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

@m6w6 m6w6 added the New label Jan 20, 2020
@m6w6
Copy link
Collaborator Author

m6w6 commented Jan 20, 2020

  • Comment by: lp:~thispaf:
  • Created at: 2016-10-05T14:58:53Z

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

@m6w6
Copy link
Collaborator Author

m6w6 commented Jan 20, 2020

  • Comment by: lp:~thispaf:
  • Created at: 2016-10-05T15:12:33Z

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

@m6w6
Copy link
Collaborator Author

m6w6 commented Jan 20, 2020

  • Comment by: lp:~thispaf:
  • Created at: 2016-10-05T15:30:23Z

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.

@m6w6
Copy link
Collaborator Author

m6w6 commented Jan 20, 2020

  • Comment by: lp:~thispaf:
  • Created at: 2016-10-11T10:46:36Z

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;
    }
  }

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

1 participant