This bug is similar to bug 8541. If i create a symlink on a samba share using a linux client (unix extensions enabled, wide links disabled) which points to a file that exists on the server it can be created and read but not deleted. Steps to reproduce: In a directory mounted via cifs from a samba-server: ln -s . link1 ln -s / link2 ln -s /a link3 rm link1 (no error, is inside the share) rm link3 (no error, does not exist on the server) rm link2 (rm: cannot remove `link2': Permission denied)
Created attachment 7194 [details] git-am fix for 3.5.next This fixes it for me. I'll create fixes for master and 3.6.next. Jeremy.
Comment on attachment 7194 [details] git-am fix for 3.5.next Gonna make one more check before I'm convinced this is correct. Bear with me.. :-).
Created attachment 7197 [details] git-am fix for 3.5.next Ok, I'm happy with this one ! Jeremy.
Sorry, but probably I just don't understand Posix permissions. symlinks do not have any permissions on their own at all? Do you have a reference for that? Thanks, Volker Lendecke
No, it isn't that they don't have permissions, it's just that we shouldn't check them as we'll never store Windows permissions separately on them. So we should just do the delete and allow the OS to decide. Jeremy.
Created attachment 7199 [details] git-am fix for 3.5.next One last (hopefully :-) change. Ensure VALID_STAT before checking the ISLNK in patch #3.
Created attachment 7200 [details] git-am fix for 3.6.x Same fix for 3.6.next.
Ok, letting that grow for a week or two. That seems to be still under active development. Volker
Think I'm done now :-). So long as it gets in for 3.5.next and 3.6.next I'm happy to wait over Christmas. Jeremy.
Is there a chance to get the reviews done until Thursday? The patch could be included in 3.6.2 then. Thanks, Karolin
*** Bug 8696 has been marked as a duplicate of this bug. ***
Comment on attachment 7200 [details] git-am fix for 3.6.x Adding David - need to get this reviewed for 3.6.next!
(In reply to comment #12) ... > Adding David - need to get this reviewed for 3.6.next! Patch looks good at first glance Jeremy. I'd like to do some testing tomorrow before giving it the final +.
(In reply to comment #13) > (In reply to comment #12) > ... > > Adding David - need to get this reviewed for 3.6.next! > > Patch looks good at first glance Jeremy. I'd like to do some testing tomorrow > before giving it the final +. Looking at the patch again, the check_name() removals from the open path have me concerned. IIUC it makes veto_files and widelink enforcement the responsibility of the IO path and as I see it these checks are currently missing.
check_name() is called inside filename_convert(), just after unix_convert(). This is the path that also handles the DFS redirect calls. All paths that get to open_file() *must* have gone through filename_convert() (or in the case of rename or unlink calls where the name is pulled from a directory listing they call check_name() directly. There is no codepath (*) where open can get called without going through check_name() which is why these changes are safe. Jeremy. (*) No Windows codepath - the UNIX codepath case for POSIX calls from a Linux client are safe as they always return the value of the LSTAT call, not a STAT call - so a link is never followed (that's what part of these changes are about).
Comment on attachment 7200 [details] git-am fix for 3.6.x Thanks for the explanation Jeremy.
Re-assigning to Karolin for inclusion in 3.5.next and 3.6.2. Jeremy.
Pushed to v3-5-test and v3-6-test. Closing out bug report. Thanks!