As noted in the Mercurial BTS entry, I was able to reproduce the bug with
latest stable Samba 3.5.6 (compiled from source on Ubuntu Linux 10.10 Maverick).

Note that the required file open (as done with mercurial.windows.posixfile)
opens the file with FILE_SHARE_DELETE for the win32 API CreateFile
( http://selenic.com/repo/hg/file/bf763946f8b0/mercurial/osutil.c#l476 )

Please attach a wireshark trace and also debug level 10 log from smbd during the client activity.

Thanks,

Jeremy.

Created attachment 6162 [details]
client side wireshark trace

Created attachment 6163 [details]
server side level 10 smbd log for that client (kork), zipped

Steps recorded in those trace-/log-files:

On the client:

1) create file 'a.txt' on the share
2) hardlink 'a.txt' to 'a.link.txt'
3) open 'a.txt' with FILE_SHARE_DELETE share mode
4) delete 'a.link.txt'
5) close 'a.txt'

After that, file 'a.txt' was missing (!) and file 'a.link.txt' was still there (which is not expected).

Client: Windows 7 Ultimate x64
Server: Samba 3.5.6 (compiled from source) on Ubuntu Linux 10.10

(In reply to comment #5)
> Steps recorded in those trace-/log-files:
> 
> On the client:
> 
> 1) create file 'a.txt' on the share
> 2) hardlink 'a.txt' to 'a.link.txt'
> 3) open 'a.txt' with FILE_SHARE_DELETE share mode
> 4) delete 'a.link.txt'
> 5) close 'a.txt'
> 
> After that, file 'a.txt' was missing (!) and file 'a.link.txt' was still there
> (which is not expected).
> 
> Client: Windows 7 Ultimate x64
> Server: Samba 3.5.6 (compiled from source) on Ubuntu Linux 10.10
> 

If I leave away steps 3 and 5, then 'a.link.txt' is gone and 'a.txt' preserved (fine). So, steps 3 and 5 are required to provoke the bad outcome.

Yep, I can see how that happens. Both opens point to the locking.tdb entry. Step 4 is also implemented with a open/del-on-close/close triple which is not effective, as the other handle on the same inode is still open.

This one is tough given the current Samba architecture.

Thanks for the very to-the-point log and sniff.

Not sure how to handle it though. Jeremy?

Volker

Ok, I have an idea as to how we might handle this.

We can reasonably easily fix this specific case. Inside close.c when DELETE_ON_CLOSE is set we check if the pathname associated with the file being closed is the same as the one stored along with the share mode entry, and if not we delete it directly (similar to what we do with posix_unlink).

The problem is we only store one sharename/path pair per open share mode entry, and there could be multiple opens onto multiple symlinks, some of which have delete on close set, and some of which do not.

In order to fix that we'll need to separate out the stored pathname, delete on close flag, and credentials used to set it so each share mode entry can have their own versions.

We can store the delete on close flag as an unused bit in the uint16 flags field inside struct share_mode_entry.

As for sharename/pathname pair - can we store it as a 64-bit hash as another element in the struct share_mode_entry array ? Remember, we don't need the full string to call unlink as we can guarantee that the last closer of the fsp will have the correct sharename/pathname pair if the 64-bit hash matches...

Thoughts ? I think I can code up a version of this for v3-6-test over the holidays for you to look at ?

Jeremy.

Arg. In the comment above "onto multiple symlinks" should be "onto multiple hard or symbolic links" of course :-).

Jeremy.