On Mon, Oct 17, 2016 at 09:41:10AM -0700, Jeremy Allison via samba wrote:> On Mon, Oct 17, 2016 at 05:13:08PM +0100, Rebecca Gellman via samba wrote: > > > > > > Hi, > > > > So I did some digging into the source code, and I think I've found the > > issue. Around line 120 of source3/libads/cldap.c: > > > > for (i=0; i<num_servers; i++) { > > NTSTATUS status; > > > > status = cldap_socket_init(state->cldap, > > NULL, /* local_addr */ > > state->servers[i], > > &state->cldap[i]); > > > > if (tevent_req_nterror(req, status)) { > > return tevent_req_post(req, ev); > > } > > > > /* Code omitted for brevity */ > > > > } > > > > This is in cldap_multi_netlogon_send(), a function that sends CLDAP > > requests to multiple DCs in one go. The loop here sets up a socket for > > each DC. cldap_socket_init() in turn (possibly several calls deeper) > > sets up the UDP socket, and calls connect() on it, which fails with > > "Network unreachable". This bubbles up the chain and comes back to > > cldap_multi_netlogon_send() as NT_STATUS_NETWORK_UNREACHABLE. > > > > Note however the return from the function: it returns an error if *any* > > of the servers queried returned an error, even if any of them succeeded. > > Great analysis - thanks ! I'll look into a patch for this. > > We'll need a new bug report for this one.OK, here's the new bug: https://bugzilla.samba.org/show_bug.cgi?id=12381 and here is (I think) the patch. Can you test this and let me know if it fixes your test case ? CC:ing to samba-technical for followups. Cheers, Jeremy.
On 2016-10-17 18:13, Jeremy Allison wrote:> On Mon, Oct 17, 2016 at 09:41:10AM -0700, Jeremy Allison via samba wrote: On Mon, Oct 17, 2016 at 05:13:08PM +0100, Rebecca Gellman via samba wrote: > > Hi, > > So I did some digging into the source code, and I think I've found the > issue. Around line 120 of source3/libads/cldap.c: > > for (i=0; i<num_servers; i++) { > NTSTATUS status; > > status = cldap_socket_init(state->cldap, > NULL, /* local_addr */ > state->servers[i], > &state->cldap[i]); > > if (tevent_req_nterror(req, status)) { > return tevent_req_post(req, ev); > } > > /* Code omitted for brevity */ > > } > > This is in cldap_multi_netlogon_send(), a function that sends CLDAP > requests to multiple DCs in one go. The loop here sets up a socket for > each DC. cldap_socket_init() in turn (possibly several calls deeper) > sets up the UDP socket, and calls connect() on it, which fails with > "Network unreachable". This bubbles up the chain and comes back to > cldap_multi_netlogon_send() as NT_STATUS_NETWORK_UNREACHABLE. > > Note however the return from the function: it returns an error if *any* > of the servers queried returned an error, even if any of them succeeded. > Great analysis - thanks ! I'll look into a patch for this. > > We'll need a new bug report for this one.OK, here's the new bug: https://bugzilla.samba.org/show_bug.cgi?id=12381 and here is (I think) the patch. Can you test this and let me know if it fixes your test case ? CC:ing to samba-technical for followups. Cheers, Jeremy. Results not good m'fraid. It seems cldap_socket_init() should be setting up the cldap_socket (param 3 is struct cldap_socket **), but obviously doesn't get this far. When this is passed to cldap_search_send() it segfaults on line 603: if (!cldap->connected) { This is going to be icky, isn't it? Regards Rebecca Gellman
On Mon, Oct 17, 2016 at 07:13:46PM +0100, Rebecca Gellman via samba wrote:> > > On 2016-10-17 18:13, Jeremy Allison wrote: > > > On Mon, Oct 17, 2016 at 09:41:10AM -0700, Jeremy Allison via samba wrote: On Mon, Oct 17, 2016 at 05:13:08PM +0100, Rebecca Gellman via samba wrote: > > > > Hi, > > > > So I did some digging into the source code, and I think I've found the > > issue. Around line 120 of source3/libads/cldap.c: > > > > for (i=0; i<num_servers; i++) { > > NTSTATUS status; > > > > status = cldap_socket_init(state->cldap, > > NULL, /* local_addr */ > > state->servers[i], > > &state->cldap[i]); > > > > if (tevent_req_nterror(req, status)) { > > return tevent_req_post(req, ev); > > } > > > > /* Code omitted for brevity */ > > > > } > > > > This is in cldap_multi_netlogon_send(), a function that sends CLDAP > > requests to multiple DCs in one go. The loop here sets up a socket for > > each DC. cldap_socket_init() in turn (possibly several calls deeper) > > sets up the UDP socket, and calls connect() on it, which fails with > > "Network unreachable". This bubbles up the chain and comes back to > > cldap_multi_netlogon_send() as NT_STATUS_NETWORK_UNREACHABLE. > > > > Note however the return from the function: it returns an error if *any* > > of the servers queried returned an error, even if any of them succeeded. > > Great analysis - thanks ! I'll look into a patch for this. > > > > We'll need a new bug report for this one. > > OK, here's the new bug: > > https://bugzilla.samba.org/show_bug.cgi?id=12381 > > and here is (I think) the patch. Can you test this > and let me know if it fixes your test case ? > > CC:ing to samba-technical for followups. > > Cheers, > > Jeremy. > > Results not good m'fraid. > > It seems cldap_socket_init() should be setting up the cldap_socket > (param 3 is struct cldap_socket **), but obviously doesn't get this far. > When this is passed to cldap_search_send() it segfaults on line 603: > > if (!cldap->connected) { > > This is going to be icky, isn't it?No, it's not so bad. libcli/cldap/cldap.c is clearly supposed to cope with caller.cldap == NULL - you can see this in the function cldap_search_state_destructor() which checks: if (s->caller.cldap) before doing anything. Can you try this (amended) patch ? Thanks so much for your timely testing ! Cheers, Jeremy.