r4951: some of the code dealing with libcli was getting too complex trying to

handle the inverted memory hierarchy that a normal session
establishment gave. The inverted hierarchy came from that fact that
you first establish a socket, then a transport, then a session and
finally a tree. That leads to the socket being at the top of the
memory hierarchy and the tree at the bottom, which makes no sense from
the users point of view, as they want to be able to free the tree and
have everything disappear.

The core problem was that the libcli interface didn't distinguish
between establishing a primary context and a secondary context. If you
establish a 2nd session on a transport then you want the transport to
be referenced by the session, whereas if you establish a primary
session then you want the transport to be a child of the session.

To fix this I have added "parent_ctx" and "primary" arguments to the
libcli intialisation functions. This makes using the library much
easier, and gives us a memory hierarchy that makes much more sense.

I was prompted to do this by a bug in the cifs backend, which was
caused by the socket not being properly torn down on a disconnect due
to the inverted memory hierarchy.
(This used to be commit 5e8fd5f70178992e249805c2e1ddafaf6840739b)

diff --git a/source4/libcli/cliconnect.c b/source4/libcli/cliconnect.c
index 4d9fb5ba1f0fb95fdda53d0fb9620722e5a10834..5aabb55f5de7930a6b36e3b60dd4a9a66f322c19 100644 (file)
--- a/source4/libcli/cliconnect.c
+++ b/source4/libcli/cliconnect.c
@@ -40,8 +40,7 @@ BOOL smbcli_socket_connect(struct smbcli_state *cli, const char *server)
                return False;
        }
        
-       cli->transport = smbcli_transport_init(sock);
-       talloc_free(sock);
+       cli->transport = smbcli_transport_init(sock, cli, True);
        if (!cli->transport) {
                return False;
        }
@@ -73,9 +72,8 @@ NTSTATUS smbcli_session_setup(struct smbcli_state *cli,
        NTSTATUS status;
        TALLOC_CTX *mem_ctx;
 
-       cli->session = smbcli_session_init(cli->transport);
+       cli->session = smbcli_session_init(cli->transport, cli, True);
        if (!cli->session) return NT_STATUS_UNSUCCESSFUL;
-       talloc_free(cli->transport);
 
        mem_ctx = talloc_init("smbcli_session_setup");
        if (!mem_ctx) return NT_STATUS_NO_MEMORY;
@@ -114,8 +112,7 @@ NTSTATUS smbcli_send_tconX(struct smbcli_state *cli, const char *sharename,
        TALLOC_CTX *mem_ctx;
        NTSTATUS status;
 
-       cli->tree = smbcli_tree_init(cli->session);
-       talloc_free(cli->session);
+       cli->tree = smbcli_tree_init(cli->session, cli, True);
        if (!cli->tree) return NT_STATUS_UNSUCCESSFUL;
 
        mem_ctx = talloc_init("tcon");

diff --git a/source4/libcli/composite/connect.c b/source4/libcli/composite/connect.c
index 69b394310e526a3fa8f2f25268b028ee3df5466d..8dd7fe39ab420a99da84a9a497d9c6b45d23e3e6 100644 (file)
--- a/source4/libcli/composite/connect.c
+++ b/source4/libcli/composite/connect.c
@@ -112,7 +112,7 @@ static NTSTATUS connect_session_setup(struct smbcli_composite *c,
        state->session->vuid = state->io_setup->out.vuid;
        
        /* setup for a tconx */
-       io->out.tree = smbcli_tree_init(state->session);
+       io->out.tree = smbcli_tree_init(state->session, state, True);
        NT_STATUS_HAVE_NO_MEMORY(io->out.tree);
 
        state->io_tcon = talloc(c, union smb_tcon);
@@ -157,12 +157,9 @@ static NTSTATUS connect_negprot(struct smbcli_composite *c,
        NT_STATUS_NOT_OK_RETURN(status);
 
        /* next step is a session setup */
-       state->session = smbcli_session_init(state->transport);
+       state->session = smbcli_session_init(state->transport, state, True);
        NT_STATUS_HAVE_NO_MEMORY(state->session);
 
-       /* get rid of the extra reference to the transport */
-       talloc_free(state->transport);
-
        state->io_setup = talloc(c, struct smb_composite_sesssetup);
        NT_STATUS_HAVE_NO_MEMORY(state->io_setup);
 
@@ -214,7 +211,7 @@ static NTSTATUS connect_socket(struct smbcli_composite *c,
        NT_STATUS_NOT_OK_RETURN(status);
 
        /* the socket is up - we can initialise the smbcli transport layer */
-       state->transport = smbcli_transport_init(state->sock);
+       state->transport = smbcli_transport_init(state->sock, state, True);
        NT_STATUS_HAVE_NO_MEMORY(state->transport);
 
        calling.name = io->in.calling_name;

diff --git a/source4/libcli/raw/clisession.c b/source4/libcli/raw/clisession.c
index ed50601c25433be4909bdc3e433b79fdffa21378..30382e08376bdbe5b12e34cf0525ba330bcbd20b 100644 (file)
--- a/source4/libcli/raw/clisession.c
+++ b/source4/libcli/raw/clisession.c
@@ -33,18 +33,23 @@
 /****************************************************************************
  Initialize the session context
 ****************************************************************************/
-struct smbcli_session *smbcli_session_init(struct smbcli_transport *transport)
+struct smbcli_session *smbcli_session_init(struct smbcli_transport *transport, 
+                                          TALLOC_CTX *parent_ctx, BOOL primary)
 {
        struct smbcli_session *session;
        uint16_t flags2;
        uint32_t capabilities;
 
-       session = talloc_zero(transport, struct smbcli_session);
+       session = talloc_zero(parent_ctx, struct smbcli_session);
        if (!session) {
                return NULL;
        }
 
-       session->transport = talloc_reference(session, transport);
+       if (primary) {
+               session->transport = talloc_steal(session, transport);
+       } else {
+               session->transport = talloc_reference(session, transport);
+       }
        session->pid = (uint16_t)getpid();
        session->vuid = UID_FIELD_INVALID;
        

diff --git a/source4/libcli/raw/clitransport.c b/source4/libcli/raw/clitransport.c
index e3a8281f3fbe17e102639d2401ce3c7126f967d4..c993fd7e60de40f97d7606d535e58371d45ecf3c 100644 (file)
--- a/source4/libcli/raw/clitransport.c
+++ b/source4/libcli/raw/clitransport.c
@@ -61,16 +61,19 @@ static int transport_destructor(void *ptr)
 /*
   create a transport structure based on an established socket
 */
-struct smbcli_transport *smbcli_transport_init(struct smbcli_socket *sock)
+struct smbcli_transport *smbcli_transport_init(struct smbcli_socket *sock,
+                                              TALLOC_CTX *parent_ctx, BOOL primary)
 {
        struct smbcli_transport *transport;
 
-       transport = talloc_p(sock, struct smbcli_transport);
+       transport = talloc_zero(parent_ctx, struct smbcli_transport);
        if (!transport) return NULL;
 
-       ZERO_STRUCTP(transport);
-
-       transport->socket = talloc_reference(transport, sock);
+       if (primary) {
+               transport->socket = talloc_steal(transport, sock);
+       } else {
+               transport->socket = talloc_reference(transport, sock);
+       }
        transport->negotiate.protocol = PROTOCOL_NT1;
        transport->options.use_spnego = lp_use_spnego();
        transport->options.max_xmit = lp_max_xmit();

diff --git a/source4/libcli/raw/clitree.c b/source4/libcli/raw/clitree.c
index 06963d5bc4268b6f35f706b7247ee2a22adb75d3..ce4dafca9f2d9caf63714c43797fa3edda40d47c 100644 (file)
--- a/source4/libcli/raw/clitree.c
+++ b/source4/libcli/raw/clitree.c
@@ -33,16 +33,22 @@
 /****************************************************************************
  Initialize the tree context
 ****************************************************************************/
-struct smbcli_tree *smbcli_tree_init(struct smbcli_session *session)
+struct smbcli_tree *smbcli_tree_init(struct smbcli_session *session,
+                                    TALLOC_CTX *parent_ctx, BOOL primary)
 {
        struct smbcli_tree *tree;
 
-       tree = talloc_zero(session, struct smbcli_tree);
+       tree = talloc_zero(parent_ctx, struct smbcli_tree);
        if (!tree) {
                return NULL;
        }
 
-       tree->session = talloc_reference(tree, session);
+       if (primary) {
+               tree->session = talloc_steal(tree, session);
+       } else {
+               tree->session = talloc_reference(tree, session);
+       }
+
 
        return tree;
 }

diff --git a/source4/libcli/util/clilsa.c b/source4/libcli/util/clilsa.c
index eac698425425bd4a661487023f71bc05b592ba04..40991a8855bf856ca3df7963ac845808a7fd46a3 100644 (file)
--- a/source4/libcli/util/clilsa.c
+++ b/source4/libcli/util/clilsa.c
@@ -58,7 +58,7 @@ static NTSTATUS smblsa_connect(struct smbcli_state *cli)
                return NT_STATUS_NO_MEMORY;
        }
 
-       lsa->ipc_tree = smbcli_tree_init(cli->session);
+       lsa->ipc_tree = smbcli_tree_init(cli->session, lsa, False);
        if (lsa->ipc_tree == NULL) {
                return NT_STATUS_NO_MEMORY;
        }

diff --git a/source4/torture/basic/secleak.c b/source4/torture/basic/secleak.c
index 4ff34e166fd8529845eeaa55bb485b2196e4c2f7..31c23ff5f7d424212ba49e984a92e15d9b70d940 100644 (file)
--- a/source4/torture/basic/secleak.c
+++ b/source4/torture/basic/secleak.c
@@ -31,7 +31,7 @@ static BOOL try_failed_login(struct smbcli_state *cli)
        struct smb_composite_sesssetup setup;
        struct smbcli_session *session;
 
-       session = smbcli_session_init(cli->transport);
+       session = smbcli_session_init(cli->transport, cli, False);
        setup.in.sesskey = cli->transport->negotiate.sesskey;
        setup.in.capabilities = cli->transport->negotiate.capabilities;
        setup.in.password = "INVALID-PASSWORD";

diff --git a/source4/torture/raw/context.c b/source4/torture/raw/context.c
index 4f1c6337eb9fa44ffc1135e50acb5f3e596bf3f5..5b6a2dd2ad2e77b7c4fe001a4e38e41cb63c3903 100644 (file)
--- a/source4/torture/raw/context.c
+++ b/source4/torture/raw/context.c
@@ -81,7 +81,7 @@ static BOOL test_session(struct smbcli_state *cli, TALLOC_CTX *mem_ctx)
        domain = lp_parm_string(-1, "torture", "userdomain");
 
        printf("create a second security context on the same transport\n");
-       session = smbcli_session_init(cli->transport);
+       session = smbcli_session_init(cli->transport, mem_ctx, False);
 
        setup.in.sesskey = cli->transport->negotiate.sesskey;
        setup.in.capabilities = cli->transport->negotiate.capabilities; /* ignored in secondary session setup, except by our libs, which care about the extended security bit */
@@ -95,7 +95,7 @@ static BOOL test_session(struct smbcli_state *cli, TALLOC_CTX *mem_ctx)
        session->vuid = setup.out.vuid;
 
        printf("create a third security context on the same transport, with vuid set\n");
-       session2 = smbcli_session_init(cli->transport);
+       session2 = smbcli_session_init(cli->transport, mem_ctx, False);
 
        session2->vuid = session->vuid;
        setup.in.sesskey = cli->transport->negotiate.sesskey;
@@ -115,7 +115,7 @@ static BOOL test_session(struct smbcli_state *cli, TALLOC_CTX *mem_ctx)
 
        if (cli->transport->negotiate.capabilities & CAP_EXTENDED_SECURITY) {
                printf("create a fourth security context on the same transport, without extended security\n");
-               session3 = smbcli_session_init(cli->transport);
+               session3 = smbcli_session_init(cli->transport, mem_ctx, False);
 
                session3->vuid = session->vuid;
                setup.in.sesskey = cli->transport->negotiate.sesskey;
@@ -131,7 +131,7 @@ static BOOL test_session(struct smbcli_state *cli, TALLOC_CTX *mem_ctx)
        }
                
        printf("use the same tree as the existing connection\n");
-       tree = smbcli_tree_init(session);
+       tree = smbcli_tree_init(session, mem_ctx, False);
        tree->tid = cli->tree->tid;
 
        printf("create a file using the new vuid\n");
@@ -217,7 +217,7 @@ static BOOL test_tree(struct smbcli_state *cli, TALLOC_CTX *mem_ctx)
        share = lp_parm_string(-1, "torture", "share");
        
        printf("create a second tree context on the same session\n");
-       tree = smbcli_tree_init(cli->session);
+       tree = smbcli_tree_init(cli->session, mem_ctx, False);
 
        tcon.generic.level = RAW_TCON_TCONX;
        tcon.tconx.in.flags = 0;

diff --git a/source4/torture/rpc/xplogin.c b/source4/torture/rpc/xplogin.c
index 25bda40da97e8b828493dc6e2787d39f2d7c5c47..3c4a881d96a1b3fe83dd2606300099fdec792ed4 100644 (file)
--- a/source4/torture/rpc/xplogin.c
+++ b/source4/torture/rpc/xplogin.c
@@ -28,13 +28,6 @@
 #include "librpc/gen_ndr/ndr_srvsvc.h"
 #include "libcli/composite/composite.h"
 
-static int destroy_transport(void *ptr)
-{
-       struct smbcli_transport *trans = ptr;
-       talloc_free(trans->socket);
-       return 0;
-}
-
 static NTSTATUS after_negprot(struct smbcli_transport **dst_transport,
                              const char *dest_host, uint16_t port,
                              const char *my_name)
@@ -54,13 +47,10 @@ static NTSTATUS after_negprot(struct smbcli_transport **dst_transport,
                return NT_STATUS_UNSUCCESSFUL;
        }
 
-       transport = smbcli_transport_init(sock);
-       talloc_free(sock);
+       transport = smbcli_transport_init(sock, NULL, True);
        if (transport == NULL)
                return NT_STATUS_NO_MEMORY;
 
-       talloc_set_destructor(transport, destroy_transport);
-
        {
                struct nbt_name calling;
                struct nbt_name called;
@@ -115,7 +105,7 @@ static NTSTATUS anon_ipc(struct smbcli_transport *transport,
        TALLOC_CTX *mem_ctx;
        NTSTATUS status;
 
-       session = smbcli_session_init(transport);
+       session = smbcli_session_init(transport, NULL, True);
        if (session == NULL)
                return NT_STATUS_NO_MEMORY;
 
@@ -144,8 +134,7 @@ static NTSTATUS anon_ipc(struct smbcli_transport *transport,
 
        talloc_set_destructor(session, destroy_session);
 
-       tree = smbcli_tree_init(session);
-       talloc_free(session);
+       tree = smbcli_tree_init(session, NULL, True);
        if (tree == NULL) {
                talloc_free(mem_ctx);
                return NT_STATUS_NO_MEMORY;