Bug 14025 - libsmbclient notfiy API fails with NT_STATUS_REVISION_MISMATCH
Summary: libsmbclient notfiy API fails with NT_STATUS_REVISION_MISMATCH
Status: ASSIGNED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: libsmbclient (show other bugs)
Version: 4.9.5
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Jeremy Allison
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-07-03 16:36 UTC by Jonathon Reinhart
Modified: 2019-07-04 15:03 UTC (History)
0 users

See Also:


Attachments
Proof of concept C code using libsmbclient (3.11 KB, text/x-csrc)
2019-07-03 16:36 UTC, Jonathon Reinhart
no flags Details
Patch that went into the 4.10.x code base (27.66 KB, patch)
2019-07-03 16:56 UTC, Jeremy Allison
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathon Reinhart 2019-07-03 16:36:41 UTC
Created attachment 15282 [details]
Proof of concept C code using libsmbclient

Samba version: 4.9.5+dfsg-5
libsmbclient version: 4.9.5+dfsg-5


I'm trying to use the "notify" API of libsmbclient, testing against a Samba AD DC. The function is failing with errno=22 (mapped from NT_STATUS_REVISION_MISMATCH), and I'm seeing the following error message:

    smb1cli_req_writev_submit: called for dialect[SMB3_11]
server[dc1.example.com]

It looks like libsmbclient is, for some reason, using SMB1 but needs to be using SMB3_11.

Note that the following command (which, like my code, forces the use of Kerberos) works just fine:

    $ smbclient -kN //dc1.example.com/sysvol -c 'notify /'

See my proof-of-concept source code here (also attached):
https://gitlab.com/JonathonReinhart/libsmbclient-notify-test
Comment 1 Jeremy Allison 2019-07-03 16:55:01 UTC
Ah, checked the versions sources.

This was fixed in 4.10, but not fixed in 4.9.

4.9 source code: source3/libsmb/clifile.c

---------------------------------------------
truct tevent_req *cli_notify_send(TALLOC_CTX *mem_ctx,
                                   struct tevent_context *ev,
                                   struct cli_state *cli, uint16_t fnum,
                                   uint32_t buffer_size,
                                   uint32_t completion_filter, bool recursive)
{
        struct tevent_req *req, *subreq;
        struct cli_notify_state *state;
        unsigned old_timeout;

        req = tevent_req_create(mem_ctx, &state, struct cli_notify_state);
        if (req == NULL) {
                return NULL;
        }

        SIVAL(state->setup, 0, completion_filter);
        SSVAL(state->setup, 4, fnum);
        SSVAL(state->setup, 6, recursive);

        /*
         * Notifies should not time out
         */
        old_timeout = cli_set_timeout(cli, 0);

        subreq = cli_trans_send(
                state,                  /* mem ctx. */
                ev,                     /* event ctx. */
                cli,                    /* cli_state. */
                0,                      /* additional_flags2 */
                SMBnttrans,             /* cmd. */
                NULL,                   /* pipe name. */
                -1,                     /* fid. */
                NT_TRANSACT_NOTIFY_CHANGE, /* function. */
                0,                      /* flags. */
                (uint16_t *)state->setup, /* setup. */
                4,                      /* num setup uint16_t words. */
                0,                      /* max returned setup. */
                NULL,                   /* param. */
                0,                      /* num param. */
                buffer_size,            /* max returned param. */
                NULL,                   /* data. */
                0,                      /* num data. */
                0);                     /* max returned data. */
------------------------------------------------------------

4.10 source code:

struct tevent_req *cli_notify_send(TALLOC_CTX *mem_ctx,
                                   struct tevent_context *ev,
                                   struct cli_state *cli, uint16_t fnum,
                                   uint32_t buffer_size,
                                   uint32_t completion_filter, bool recursive)
{
        struct tevent_req *req;
        struct cli_notify_state *state;
        unsigned old_timeout;

        req = tevent_req_create(mem_ctx, &state, struct cli_notify_state);
        if (req == NULL) {
                return NULL;
        }

        if (smbXcli_conn_protocol(cli->conn) >= PROTOCOL_SMB2_02) {
                /*
                 * Notifies should not time out
                 */
                old_timeout = cli_set_timeout(cli, 0);

                state->subreq = cli_smb2_notify_send(
                        state,
                        ev,
                        cli,
                        fnum,
                        buffer_size,
                        completion_filter,
                        recursive);
------------------------------------------------------------
Comment 2 Jeremy Allison 2019-07-03 16:56:20 UTC
Created attachment 15283 [details]
Patch that went into the 4.10.x code base

This is a git patchstream from 4.10 created by:

git format-patch --stdout 24b1aa9da2e53e0dba89811f6a8d14b810bd48a8..88d82b44c326cfb3ab1cf1f7288eada2566ab4bc

This may or may not apply cleanly to 4.9. Anyway it shows the fix that went into 4.10.
Comment 3 Jonathon Reinhart 2019-07-03 17:12:55 UTC
That's a pretty big changeset. Would something of this size usually be backported to the previous version (4.9.x)?

Given that Debian Buster (10) is shipping with 4.9.x, I'm wondering if this is a worthwhile backport? Apparently not many people are trying to use libsmbclient notify.
Comment 4 Jeremy Allison 2019-07-03 17:22:45 UTC
Unless there's strong demand from the Linux vendors, we'll probably not backport this. If you're OK with that you can close the bug with a "Fixed in 4.10".
Comment 5 Jonathon Reinhart 2019-07-03 17:39:29 UTC
It's unfortunate, but not a showstopper. Just another reason to move to 4.10.x I suppose :-)
Comment 6 Louis 2019-07-04 15:03:05 UTC
(In reply to Jonathon Reinhart from comment #5)

Please report this one to Debian also, maybe, just maybe, we can get an approval to move buster up to 4.10. 

I already suggested that, but it was denied because of buster is in frozen state
 now. 
Just, if debian is going to use 4.9.5 in the end for buster, well, then i do expect a wave of problems on the samba list also. 

Debian is really to far again in its patches (again), and samba is moving fast. 

And, as said, this patch set is pretty big, and this is only one patch.