Bug 14878 - Recursive directory delete with veto files is broken in 4.15.0.
Summary: Recursive directory delete with veto files is broken in 4.15.0.
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: 4.15.0
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Jule Anger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks: 14879 14892
  Show dependency treegraph
 
Reported: 2021-10-21 18:39 UTC by Jeremy Allison
Modified: 2021-11-24 19:49 UTC (History)
4 users (show)

See Also:


Attachments
git-am fix for 4.15.next. (9.65 KB, patch)
2021-10-29 18:13 UTC, Jeremy Allison
slow: review+
Details
patch for v4-14 (9.41 KB, patch)
2021-11-24 11:52 UTC, Pavel Fiipenský
jra: review-
Details
squashed patch for v4-14 (7.96 KB, patch)
2021-11-24 19:43 UTC, Pavel Fiipenský
pfilipen: review? (jra)
pfilipen: review? (slow)
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Allison 2021-10-21 18:39:06 UTC
Due to second call to SMB_VFS_UNLINKAT() inside rmdir_internals() using the wrong parameters.

Have fix, need bugnumber and regression test.
Comment 1 Jeremy Allison 2021-10-29 18:13:53 UTC
Created attachment 16899 [details]
git-am fix for 4.15.next.

Cherry-picked from master. Note this must be applied *before* the fix for https://bugzilla.samba.org/show_bug.cgi?id=14879
Comment 2 Samba QA Contact 2021-10-29 18:21:37 UTC
This bug was referenced in samba master:

ad0082d79a681b981154747dcde5713e1933b88f
73de1194c3c429ab93d722a852aa4f54213b112a
Comment 3 Ralph Böhme 2021-11-04 07:55:48 UTC
Reassigning to Jule for inclusion in 4.15.
Comment 4 Jeremy Allison 2021-11-04 21:17:57 UTC
Note to Jule:

Please note the patches for these bugs *MUST* be applied in the following
order:

https://bugzilla.samba.org/show_bug.cgi?id=14878
https://bugzilla.samba.org/show_bug.cgi?id=14879
https://bugzilla.samba.org/show_bug.cgi?id=14892

in order to apply correctly. Thanks !
Comment 5 Jule Anger 2021-11-10 14:25:43 UTC
Pushed to autobuild-v4-15-test.
Comment 6 Samba QA Contact 2021-11-10 17:09:05 UTC
This bug was referenced in samba v4-15-test:

dab3fa1d8c27e696afa15e071331f646e06d9706
9f76641627ffb0b7fe07e2a071b958a96ec87226
Comment 7 Jule Anger 2021-11-10 18:04:37 UTC
Closing out bug report.

Thanks!
Comment 8 Pavel Fiipenský 2021-11-24 11:52:49 UTC
Created attachment 17023 [details]
patch for v4-14

73de1194c3c429ab93d722a852aa4f54213b112a s3: smbd: Fix recursive directory delete of a directory containing veto file and msdfs links.

- commit above was modified,changes in source3/smbd/close.c are not applicable
Comment 9 Jeremy Allison 2021-11-24 18:34:23 UTC
Hi Pavel,

I'm probably not going to be able to get to this until Monday due to the USA Thanksgiving Holiday, sorry.

Jeremy.
Comment 10 Jeremy Allison 2021-11-24 18:45:27 UTC
Comment on attachment 17023 [details]
patch for v4-14

Pavel, I don't see any code changes here. Just adding the tests and then removing the knownfail. Is this patch what you intended ?
Comment 11 Pavel Fiipenský 2021-11-24 19:09:57 UTC
Yes, that is intended.

If you check the patch for 4.15 there is only one code change:
-			       dirfsp,
+			       parent_fname->fsp,

For 4.14 this is not applicable, so I have just added the tests, so we can test the functionality. 
Do you think that such patch should be skipped completely?
Comment 12 Jeremy Allison 2021-11-24 19:22:39 UTC
Oh I'm sorry, I misunderstood. Then the patch is on the whole OK, but just squash into one patch. There's no point in adding and then removing the knownfail. Just add the test without a knownfail, as 4.14.x doesn't fail here.
Comment 13 Pavel Fiipenský 2021-11-24 19:43:57 UTC
Created attachment 17025 [details]
squashed patch for v4-14

Adding a new patch squashed into a single commit
Comment 14 Pavel Fiipenský 2021-11-24 19:49:15 UTC
I see that the commit message for the squashed patch still says:

Add knownfail.

Let me know if this should be removed, or can be accepted