r12225: r11729@cabra: derrell | 2005-12-13 22:59:45 -0500

 1. Fix a crash bug which should have reared its ugly head ages ago, but for
    some reason, remained dormant until recently.  The bug pertained to
    libsmbclient doing a structure assignment of a cli after having opened a
    pipe.  The pipe open code makes a copy of the cli pointer that was passed
    to it.  If the cli is later copied (and that cli pointer that was saved
    is no longer valid), the pipe code will cause a crash during shutdown or
    when the copied cli is closed.

 2. The 'type' field in enumerated shares was not being set correctly with
    the new RPC-based mechanism for enumerating shares.

diff --git a/examples/libsmbclient/testbrowse2.c b/examples/libsmbclient/testbrowse2.c
index a3d2cf3b8e202d09b2671405494f00004b501d75..76d98b9602a7552949ef5a96945d30dc70653f99 100644 (file)
--- a/examples/libsmbclient/testbrowse2.c
+++ b/examples/libsmbclient/testbrowse2.c
@@ -85,7 +85,7 @@ void smbc_auth_fn(
 
     strncpy(wrkgrp, workgroup, wrkgrplen - 1); wrkgrp[wrkgrplen - 1] = 0;
     strncpy(user, username, userlen - 1); user[userlen - 1] = 0;
-    strncpy(passwd, password, passwdlen - 1); passwd[passwdlen - 1] = 0;       
+    strncpy(passwd, password, passwdlen - 1); passwd[passwdlen - 1] = 0;
 }
 
 SMBCCTX* create_smbctx(){

diff --git a/source/libsmb/clientgen.c b/source/libsmb/clientgen.c
index 6dc7386c03b9c24c9b22fc14094500ab1ff09e8c..2f980adcf8733f4a99005047f5aa0b1a7ec10626 100644 (file)
--- a/source/libsmb/clientgen.c
+++ b/source/libsmb/clientgen.c
@@ -358,11 +358,13 @@ struct cli_state *cli_initialise(struct cli_state *cli)
 void cli_rpc_pipe_close(struct rpc_pipe_client *cli)
 {
        if (!cli_close(cli->cli, cli->fnum)) {
-               DEBUG(0,("cli_rpc_pipe_close: cli_close failed on pipe %s "
-                       "to machine %s.  Error was %s\n",
-                       cli->pipe_name,
-                       cli->cli->desthost,
-                       cli_errstr(cli->cli)));
+               DEBUG(0,("cli_rpc_pipe_close: cli_close failed on pipe %s, "
+                         "fnum 0x%x "
+                         "to machine %s.  Error was %s\n",
+                         cli->pipe_name,
+                         (int) cli->fnum,
+                         cli->cli->desthost,
+                         cli_errstr(cli->cli)));
        }
 
        if (cli->auth.cli_auth_data_free_func) {

diff --git a/source/libsmb/libsmbclient.c b/source/libsmb/libsmbclient.c
index 32282cb6a647ac016fbf2f3682590ec08d4edac3..ba57c03abadd0cf170e59b2b03c367740947f49f 100644 (file)
--- a/source/libsmb/libsmbclient.c
+++ b/source/libsmb/libsmbclient.c
@@ -82,7 +82,6 @@ static int DLIST_CONTAINS(SMBCFILE * list, SMBCFILE *p) {
 /*
  * Find an lsa pipe handle associated with a cli struct.
  */
-
 static struct rpc_pipe_client *find_lsa_pipe_hnd(struct cli_state *ipc_cli)
 {
        struct rpc_pipe_client *pipe_hnd;
@@ -855,14 +854,27 @@ SMBCSRV *smbc_attr_server(SMBCCTX *context,
                         return NULL;
                 }
 
+                ipc_srv = SMB_MALLOC_P(SMBCSRV);
+                if (!ipc_srv) {
+                        errno = ENOMEM;
+                        cli_shutdown(ipc_cli);
+                        return NULL;
+                }
+
+                ZERO_STRUCTP(ipc_srv);
+                ipc_srv->cli = *ipc_cli;
+
+                free(ipc_cli);
+
                 if (pol) {
-                        pipe_hnd = cli_rpc_pipe_open_noauth(ipc_cli,
+                        pipe_hnd = cli_rpc_pipe_open_noauth(&ipc_srv->cli,
                                                             PI_LSARPC,
                                                             &nt_status);
                         if (!pipe_hnd) {
                                 DEBUG(1, ("cli_nt_session_open fail!\n"));
                                 errno = ENOTSUP;
-                                cli_shutdown(ipc_cli);
+                                cli_shutdown(&ipc_srv->cli);
+                                free(ipc_srv);
                                 return NULL;
                         }
 
@@ -874,30 +886,18 @@ SMBCSRV *smbc_attr_server(SMBCCTX *context,
         
                         nt_status = rpccli_lsa_open_policy(
                                 pipe_hnd,
-                                ipc_cli->mem_ctx,
+                                ipc_srv->cli.mem_ctx,
                                 True, 
                                 GENERIC_EXECUTE_ACCESS,
                                 pol);
         
                         if (!NT_STATUS_IS_OK(nt_status)) {
-                                errno = smbc_errno(context, ipc_cli);
-                                cli_shutdown(ipc_cli);
+                                errno = smbc_errno(context, &ipc_srv->cli);
+                                cli_shutdown(&ipc_srv->cli);
                                 return NULL;
                         }
                 }
 
-                ipc_srv = SMB_MALLOC_P(SMBCSRV);
-                if (!ipc_srv) {
-                        errno = ENOMEM;
-                        cli_shutdown(ipc_cli);
-                        return NULL;
-                }
-
-                ZERO_STRUCTP(ipc_srv);
-                ipc_srv->cli = *ipc_cli;
-
-                free(ipc_cli);
-
                 /* now add it to the cache (internal or external) */
 
                 errno = 0;      /* let cache function set errno if it likes */
@@ -2191,12 +2191,23 @@ list_fn(const char *name, uint32 type, const char *comment, void *state)
        SMBCFILE *dir = (SMBCFILE *)state;
        int dirent_type;
 
-       /* We need to process the type a little ... */
-
+       /*
+         * We need to process the type a little ...
+         *
+         * Disk share     = 0x00000000
+         * Print share    = 0x00000001
+         * Comms share    = 0x00000002 (obsolete?)
+         * IPC$ share     = 0x00000003 
+         *
+         * administrative shares:
+         * ADMIN$, IPC$, C$, D$, E$ ...  are type |= 0x80000000
+         */
+        
        if (dir->dir_type == SMBC_FILE_SHARE) {
                
                switch (type) {
-               case 0: /* Directory tree */
+                case 0 | 0x80000000:
+               case 0:
                        dirent_type = SMBC_FILE_SHARE;
                        break;
 
@@ -2208,6 +2219,7 @@ list_fn(const char *name, uint32 type, const char *comment, void *state)
                        dirent_type = SMBC_COMMS_SHARE;
                        break;
 
+                case 3 | 0x80000000:
                case 3:
                        dirent_type = SMBC_IPC_SHARE;
                        break;
@@ -2217,7 +2229,9 @@ list_fn(const char *name, uint32 type, const char *comment, void *state)
                        break;
                }
        }
-       else dirent_type = dir->dir_type;
+       else {
+                dirent_type = dir->dir_type;
+        }
 
        if (add_dirent(dir, name, comment, dirent_type) < 0) {
 
@@ -2252,15 +2266,16 @@ net_share_enum_rpc(struct cli_state *cli,
 {
         int i;
        WERROR result;
-        NTSTATUS nt_status;
        ENUM_HND enum_hnd;
         uint32 info_level = 1;
        uint32 preferred_len = 0xffffffff;
+        uint32 type;
        SRV_SHARE_INFO_CTR ctr;
        fstring name = "";
         fstring comment = "";
         void *mem_ctx;
        struct rpc_pipe_client *pipe_hnd;
+        NTSTATUS nt_status;
 
         /* Open the server service pipe */
         pipe_hnd = cli_rpc_pipe_open_noauth(cli, PI_SRVSVC, &nt_status);
@@ -2273,6 +2288,7 @@ net_share_enum_rpc(struct cli_state *cli,
         mem_ctx = talloc_init("libsmbclient: net_share_enum_rpc");
         if (mem_ctx == NULL) {
                 DEBUG(0, ("out of memory for net_share_enum_rpc!\n"));
+                cli_rpc_pipe_close(pipe_hnd);
                 return -1; 
         }
 
@@ -2302,14 +2318,17 @@ net_share_enum_rpc(struct cli_state *cli,
                 rpcstr_pull_unistr2_fstring(
                         comment, &ctr.share.info1[i].info_1_str.uni_remark);
 
+                /* Get the type value */
+                type = ctr.share.info1[i].info_1.type;
+
                 /* Add this share to the list */
-                (*fn)(name, SMBC_FILE_SHARE, comment, state);
+                (*fn)(name, type, comment, state);
         }
 
-        /* We're done with the pipe */
-        cli_rpc_pipe_close(pipe_hnd);
-        
 done:
+        /* Close the server service pipe */
+        cli_rpc_pipe_close(pipe_hnd);
+
         /* Free all memory which was allocated for this request */
         talloc_free(mem_ctx);
 

diff --git a/source/rpc_client/cli_pipe.c b/source/rpc_client/cli_pipe.c
index 7965aee8074fad093918687a133dfa2ba4e0d17d..41266838a87a87ef896a22e142e4b978951d3396 100644 (file)
--- a/source/rpc_client/cli_pipe.c
+++ b/source/rpc_client/cli_pipe.c
@@ -2150,6 +2150,15 @@ static NTSTATUS rpc_pipe_bind(struct rpc_pipe_client *cli,
 
 /****************************************************************************
  Open a named pipe over SMB to a remote server.
+ *
+ * CAVEAT CALLER OF THIS FUNCTION:
+ *    The returned rpc_pipe_client saves a copy of the cli_state cli pointer,
+ *    so be sure that this function is called AFTER any structure (vs pointer)
+ *    assignment of the cli.  In particular, libsmbclient does structure
+ *    assignments of cli, which invalidates the data in the returned
+ *    rpc_pipe_client if this function is called before the structure assignment
+ *    of cli.
+ * 
  ****************************************************************************/
 
 static struct rpc_pipe_client *cli_rpc_pipe_open(struct cli_state *cli, int pipe_idx, NTSTATUS *perr)