[PATCH] Fix smbclient "notify" command

Jeremy Allison jra at samba.org
Wed Apr 4 23:09:17 UTC 2018


On Wed, Apr 04, 2018 at 01:12:45PM +0200, Volker Lendecke via samba-technical wrote:
> Hi!
> 
> smbclient's "notify" command is pretty broken right now, in particular
> it fails badly for smb2. The attached patch fixes it, with
> confirmation of a user that I did it for.
> 
> Review appreciated!

Nice work ! I'm thinking about how to add a test for this,
probably using the smbclient_s3 torture tests so we don't
break this again going forward, but that's a patch for another
day :-).

RB+ and pushed !

Jeremy.

> -- 
> SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
> phone: +49-551-370000-0, fax: +49-551-370000-9
> AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
> http://www.sernet.de, mailto:kontakt at sernet.de

> From 4342d64ab66e7ef4667b10a2b9dcc02c9cd32c72 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Mon, 30 Oct 2017 14:34:12 +0100
> Subject: [PATCH 1/3] libsmb: Handle long-running smb2cli_notify
> 
> This likely runs into a timeout. Properly cancel the smb2 request,
> allowing the higher-level caller to re-issue this request on an existing
> handle.
> 
> I did not see a proper way to achieve this with tevent_req_set_endtime or
> something like that.
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  libcli/smb/smb2cli_notify.c | 54 +++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 45 insertions(+), 9 deletions(-)
> 
> diff --git a/libcli/smb/smb2cli_notify.c b/libcli/smb/smb2cli_notify.c
> index 0a23cf9ad03..34329ba16cc 100644
> --- a/libcli/smb/smb2cli_notify.c
> +++ b/libcli/smb/smb2cli_notify.c
> @@ -30,9 +30,12 @@ struct smb2cli_notify_state {
>  	struct iovec *recv_iov;
>  	uint8_t *data;
>  	uint32_t data_length;
> +
> +	struct tevent_req *subreq;
>  };
>  
>  static void smb2cli_notify_done(struct tevent_req *subreq);
> +static void smb2cli_notify_timedout(struct tevent_req *subreq);
>  
>  struct tevent_req *smb2cli_notify_send(TALLOC_CTX *mem_ctx,
>  				       struct tevent_context *ev,
> @@ -64,21 +67,50 @@ struct tevent_req *smb2cli_notify_send(TALLOC_CTX *mem_ctx,
>  	SIVAL(fixed, 24, completion_filter);
>  	SIVAL(fixed, 28, 0); 	/* reserved */
>  
> -	subreq = smb2cli_req_send(state, ev, conn, SMB2_OP_NOTIFY,
> -				  0, 0, /* flags */
> -				  timeout_msec,
> -				  tcon,
> -				  session,
> -				  state->fixed, sizeof(state->fixed),
> -				  NULL, 0, /* dyn* */
> -				  0); /* max_dyn_len */
> +	state->subreq = smb2cli_req_send(state, ev, conn, SMB2_OP_NOTIFY,
> +					 0, 0, /* flags */
> +					 0,	/* timeout_msec */
> +					 tcon,
> +					 session,
> +					 state->fixed, sizeof(state->fixed),
> +					 NULL, 0, /* dyn* */
> +					 0); /* max_dyn_len */
> +	if (tevent_req_nomem(state->subreq, req)) {
> +		return tevent_req_post(req, ev);
> +	}
> +	tevent_req_set_callback(state->subreq, smb2cli_notify_done, req);
> +
> +	subreq = tevent_wakeup_send(state, ev,
> +				    timeval_current_ofs_msec(timeout_msec));
>  	if (tevent_req_nomem(subreq, req)) {
>  		return tevent_req_post(req, ev);
>  	}
> -	tevent_req_set_callback(subreq, smb2cli_notify_done, req);
> +	tevent_req_set_callback(subreq, smb2cli_notify_timedout, req);
> +
>  	return req;
>  }
>  
> +static void smb2cli_notify_timedout(struct tevent_req *subreq)
> +{
> +	struct tevent_req *req = tevent_req_callback_data(
> +		subreq, struct tevent_req);
> +	struct smb2cli_notify_state *state = tevent_req_data(
> +		req, struct smb2cli_notify_state);
> +	bool ok;
> +
> +	ok = tevent_wakeup_recv(subreq);
> +	if (!ok) {
> +		tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
> +		return;
> +	}
> +
> +	ok = tevent_req_cancel(state->subreq);
> +	if (!ok) {
> +		tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
> +		return;
> +	}
> +}
> +
>  static void smb2cli_notify_done(struct tevent_req *subreq)
>  {
>  	struct tevent_req *req = tevent_req_callback_data(
> @@ -98,6 +130,10 @@ static void smb2cli_notify_done(struct tevent_req *subreq)
>  	status = smb2cli_req_recv(subreq, state, &iov,
>  				  expected, ARRAY_SIZE(expected));
>  	TALLOC_FREE(subreq);
> +
> +	if (NT_STATUS_EQUAL(status, NT_STATUS_CANCELLED)) {
> +		status = NT_STATUS_IO_TIMEOUT;
> +	}
>  	if (tevent_req_nterror(req, status)) {
>  		return;
>  	}
> -- 
> 2.11.0
> 
> 
> From 557d063fd8ed66c20b5efd21437bfc04ce926198 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Mon, 30 Oct 2017 14:36:46 +0100
> Subject: [PATCH 2/3] libsmb: Handle IO_TIMEOUT in cli_smb2_notify properly
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  source3/libsmb/cli_smb2_fnum.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/source3/libsmb/cli_smb2_fnum.c b/source3/libsmb/cli_smb2_fnum.c
> index 2d87b58d730..c397b29b381 100644
> --- a/source3/libsmb/cli_smb2_fnum.c
> +++ b/source3/libsmb/cli_smb2_fnum.c
> @@ -4192,6 +4192,15 @@ NTSTATUS cli_smb2_notify(struct cli_state *cli, uint16_t fnum,
>  				completion_filter, recursive,
>  				frame, &base, &len);
>  
> +	if (NT_STATUS_EQUAL(status, NT_STATUS_IO_TIMEOUT)) {
> +		len = 0;
> +		status = NT_STATUS_OK;
> +	}
> +
> +	if (!NT_STATUS_IS_OK(status)) {
> +		goto fail;
> +	}
> +
>  	ofs = 0;
>  
>  	while (len - ofs >= 12) {
> -- 
> 2.11.0
> 
> 
> From 4e194811e1b0dcec49a2a837bbb17a501f6dbd14 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Mon, 30 Oct 2017 16:15:03 +0100
> Subject: [PATCH 3/3] smbclient: Handle ENUM_DIR in "notify" command
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  source3/client/client.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/source3/client/client.c b/source3/client/client.c
> index 23ed02d9cc0..1429b44e9cf 100644
> --- a/source3/client/client.c
> +++ b/source3/client/client.c
> @@ -4568,12 +4568,17 @@ static int cmd_notify(void)
>  	}
>  
>  	while (1) {
> -		uint32_t i, num_changes;
> -		struct notify_change *changes;
> +		uint32_t i;
> +		uint32_t num_changes = 0;
> +		struct notify_change *changes = NULL;
>  
>  		status = cli_notify(cli, fnum, 1000, FILE_NOTIFY_CHANGE_ALL,
>  				    true,
>  				    talloc_tos(), &num_changes, &changes);
> +		if (NT_STATUS_EQUAL(status, STATUS_NOTIFY_ENUM_DIR)) {
> +			printf("NOTIFY_ENUM_DIR\n");
> +			status = NT_STATUS_OK;
> +		}
>  		if (!NT_STATUS_IS_OK(status)) {
>  			d_printf("notify returned %s\n",
>  				 nt_errstr(status));
> -- 
> 2.11.0
> 




More information about the samba-technical mailing list