Bug 10634 - Notify response is failing with NOTIFY_ENUM_DIR.
Summary: Notify response is failing with NOTIFY_ENUM_DIR.
Status: RESOLVED FIXED
Alias: None
Product: Samba 3.6
Classification: Unclassified
Component: SMB2 (show other bugs)
Version: 3.6.12
Hardware: All All
: P5 major
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-27 11:32 UTC by Hemanth
Modified: 2015-10-26 10:38 UTC (History)
2 users (show)

See Also:


Attachments
proposed patch to resolve this problem. (1.03 KB, text/plain)
2014-05-27 11:43 UTC, Hemanth
no flags Details
Make test fails, sorry for the noise (969 bytes, patch)
2015-10-16 14:43 UTC, Volker Lendecke
no flags Details
git-am fix for master. (2.04 KB, patch)
2015-10-16 22:29 UTC, Jeremy Allison
no flags Details
Updated git-am fix for master. (2.02 KB, patch)
2015-10-17 04:39 UTC, Jeremy Allison
no flags Details
git-am cherry-pick from master - for 4.3.next, 4.2.next. (2.59 KB, patch)
2015-10-18 00:44 UTC, Jeremy Allison
vl: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Hemanth 2014-05-27 11:32:44 UTC
Samba notify.c notify_marshall_changes() calls ndr_push_struct_blob() to package file information into change notify buffer. ndr_push_struct_blob allocates the temporary data blob with extra buffer. The temporary data blob length is used to compare against client output buffer length, and causes the file information to be dropped. Usually it's not an issue, since client normally accepts large enough output buffer. 

In case of Windows Explorer, it gives only 32 bytes and cause the file entry to be dropped and returning NOTIFY_ENUM_DIR error. But client is able to get the changes as it is re-enumerating(using find) data even when it receives NOTIFY_ENUM_DIR error. But I think we should discard the extra padding while comparing against max offset.

		ndr_err = ndr_push_struct_blob(&blob, talloc_tos(), &m,
			(ndr_push_flags_fn_t)ndr_push_FILE_NOTIFY_INFORMATION);
		if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
			return false;
		}

		if (DEBUGLEVEL >= 10) {
			NDR_PRINT_DEBUG(FILE_NOTIFY_INFORMATION, &m);
		}

		if (!data_blob_append(talloc_tos(), final_blob,
				      blob.data, blob.length)) {
			data_blob_free(&blob);
			return false;
		}

		data_blob_free(&blob);

		if (final_blob->length > max_offset) {  <--- final_blob contains extra padding
			/* Too much data for client. */
			DEBUG(0, ("Client only wanted %d bytes, trying to "
				   "marshall %d bytes\n", (int)max_offset,
				   (int)final_blob->length));
			return False;
		}


Ideally we should compare the unaligned notify information size with max offset instead of aligned data which has the extra padding.
Comment 1 Hemanth 2014-05-27 11:43:10 UTC
Created attachment 9980 [details]
proposed patch to resolve this problem.

Here I am attaching the proposed patch which can resolve the issue. Basically calculated unaligned size and compared against max offset to check if the notify information size is exceeding to the size client expect.
Comment 2 Volker Lendecke 2015-10-16 14:43:24 UTC
Created attachment 11500 [details]
Make test fails, sorry for the noise

Would the attached patch also solve your issue? It's against master, but the same file exists in 3.6/source3/librpc/idl, same patch.
Comment 3 Jeremy Allison 2015-10-16 22:29:03 UTC
Created attachment 11503 [details]
git-am fix for master.

Passes make test.
Comment 4 Jeremy Allison 2015-10-17 04:39:32 UTC
Created attachment 11504 [details]
Updated git-am fix for master.

Hmmm. After thinking about it some the :

                [flag(NDR_ALIGN4)]    DATA_BLOB _pad;

shouldn't really have the "[flag(NDR_ALIGN4)]"
attribute. Doesn't do any good where I put it.
Comment 5 Jeremy Allison 2015-10-18 00:44:51 UTC
Created attachment 11505 [details]
git-am cherry-pick from master - for 4.3.next, 4.2.next.
Comment 6 Karolin Seeger 2015-10-19 08:53:46 UTC
Pushed to autobuild-v4-[2|3]-test.
Comment 7 Karolin Seeger 2015-10-19 09:46:53 UTC
(In reply to Karolin Seeger from comment #6)
Pushed to both branches.
Closing out bug report.

Thanks!