[PATCH] Fix bug #Bug 12831 - smbcacls got error NT_STATUS_NETWORK_NAME_DELETED

[Richard Sharpe]

On Wed, Jun 14, 2017 at 8:50 AM, Jeremy Allison via samba-technical
<samba-technical at lists.samba.org> wrote:
> It turns out that smbclient, smbcacls and smbtorture3 depend
> on the ability to temporarily replace the client tcon connection
> struct internally to the client with a new connection to IPC$,
> do some calls on that new connection (usually lookupnames/lookupsids),
> then replace the old values of the tcon and continue.
>
> This has some problems. (a). It only works for SMB1 (for SMB2 we
> overwrote the tcon pointer) and (b). It didn't really work for SMB1
> either - we ended up with a bastardized cli->smb1.tcon pointer that
> contains type and string from the IPC$ connection whilst being
> connected to the share connection. We only got away with this
> due to the fact we don't use the type and string for anything
> on the wire.
>
> This patch unifies the cli->smb[1|2].tcon pointer handling and
> allows temporary replacement of the pointer, and also SMB2 access
> to the get/set the 32-bit tid value.
>
> Most of this patch is fixing up the smbclient/smbcacls/smbtorture3
> code so everything keeps working with make test, so I know this
> is being tested (it took me a day or so to keep all our tests
> passing :-).
>
> Please review and push if you're happy !

Minor nit. Here:
----------------------
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index 58f0ce0a31f..378cb8ba35f 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -1302,6 +1302,7 @@ static bool run_tcon_test(int dummy)
        const char *fname = "\\tcontest.tmp";
        uint16_t fnum1;
        uint32_t cnum1, cnum2, cnum3;
+       struct smbXcli_tcon *orig_tcon = NULL;
        uint16_t vuid1, vuid2;
        char buf[4];
        bool ret = True;
@@ -1333,6 +1334,11 @@ static bool run_tcon_test(int dummy)
                return False;
        }

+       orig_tcon = cli_state_save_tcon(cli);
+       if (orig_tcon == NULL) {
+               return false;
+       }
+
        status = cli_tree_connect_creds(cli, share, "?????", torture_creds);
        if (!NT_STATUS_IS_OK(status)) {
                printf("%s refused 2nd tree connect (%s)\n", host,
@@ -1400,6 +1406,8 @@ static bool run_tcon_test(int dummy)
                return False;
        }

+       cli_state_restore_tcon(cli, orig_tcon);
+
        cli_state_set_tid(cli, cnum1);

        if (!torture_close_connection(cli)) {
-------------------------

it looks like maybe you can remove this line around 1327

       cnum1 = cli_state_get_tid(cli);

and this line around 1403 just after you restore the tcon:

       cli_state_set_tid(cli, cnum1);

You seem to have done that in at least
source3/utils/net_rpc.c:show_userlist but not in
source3/torture/torture.c:run_tcon_test. However, you might have a
good reason for that.

Apart from that: Reviewed-by: Richard Sharpe <realrichardsharpe at gmail.com>

-- 
Regards,
Richard Sharpe
(何以解憂？唯有杜康。--曹操)