Problem: -------- 1. If file is renamed from client, new filename is propagated to file handlers (files_struct *fsp) opening renamed file in SAME smbd process only. File handers opening renamed file in OTEHR smbd processes are not affected. 2. DELETE_ON_CLOSE flag removes file based on filename. How to cause a problem on the real world: ----------------------------------------- Prepare: Put a MS-Excel file (e.g. file.xls) into a share on Samba server. Client1: Open file.xls with read/write mode. Client2: Open file.xls with read-only mode. Client1: Do "Save" on MS-Excel: Save new data to temporary file (e.g. file.new). Rename file.xls to temporary file (e.g. file.old). Rename file.new to file.xls. Set DELETE_ON_CLOSE flag to file.old. Close file.old (renamed old file.xls with DELETE_ON_CLOSE flag). But file.old is NOT removed here because it is opened by Client2 yet. Client2: Close file.old (renamed old file.xls with DELETE_ON_CLOSE flag). file.old is removed now. file.xls is NOT removed.
(In reply to comment #0) > Client2: > Close file.old (renamed old file.xls with DELETE_ON_CLOSE flag). > file.old is removed now. > file.xls is NOT removed. This (correct) result is occured on Windows share. But on Samba share, file.old is remained and file.xls is removed.
Created attachment 1606 [details] ad-hoc test code Compile: -------- $ tar xzf samba-3.0.20b.tar.gz $ cd samba-3.0.20b/source $ ./configure && make $ gcc -I. -Iinclude -Iubiqx -Ipopt -c /PATH/TO/t_del_on_close.c $ gcc -o t_del_on_close `sed -n 's/^LIBS=//p' Makefile` t_del_on_close.o bin/libsmbclient.a Setup: ------ 1. Make smb.conf: [share] path = /project/share write ok = yes 2. Put file.xls into share dir. $ touch /project/share/file.xls 3. Start smbd. Test: ----- $ ./t_del_on_close 127.0.0.1 share user password
Thanks for that detailed report. I'll see if I can reproduce it and try to find a fix. This will probably end up as another test in the Samba4 raw-delete test. Volker
Created attachment 1608 [details] proposed patch to fix removing old name file on DELETE_ON_CLOSE This patch fixes the following problems: * Remove file based on old filename by DELETE_ON_CLOSE flag for renamed file. * smbstatus(1) and SWAT shows old filename in status information for renamed file. This patch does NOT fix the following problems: * smbd(8) shows old filename in log file for renamed filename. * File processing (except remove on DELETE_ON_CLOSE case) based on filename for opened and renamed file access to old filename.
Created attachment 1609 [details] proposed patch to fix removing old name file on DELETE_ON_CLOSE Updated.
Thanks - this is pretty close to what I'm working on. The issue you've missed is if a rename takes place on a different share that is mapped to the same area of the filesystem. This then could cause data loss also. The original filename stored in the locking.tdb before the recent rewrite was a complete pathname on the system not a pathname relative to the share - we'll have to go back to that to protect against this issue (there is no perfect solution to this issue but at least we need to detect it). Jeremy.
Fixed in SAMBA_3_0 SVN and HEAD. Please check out and test. This is a generic fix that will fix the get delete on close trans2 calls also. Jeremy.
Looks like bad... There is a race condition. From SMB_VFS_RENAME() to rename_share_filename() must be atomic. Otherwise, 1. smbd A: SMB_VFS_RENAME() 2. smbd B: lck = get_share_mode_lock() 3. smbd A: lck = get_share_mode_lock() - wait for smbd B... 4. smbd B: Try to do something to file based on filename, but failed (or ocurred another problem?). 5. smbd B: talloc_free(lck) 6. smbd A: continued... I think smbd must do `lck = get_share_mode_lock()' before SMB_VFS_RENAME() and do `talloc_free(lck)' after rename_open_files().
This (rename) is inherently racy. Your modification would reduce the race, but not remove it, that's why I coded it this way instead of your way. The message passing code is not synchronous - there is no guarentee that the receiving smbd will have processed the message or not be in the close code at the point the message is sent. As tdb_chainlock and tdb_chainunlock are recursion safe within one smbd I can increate the lock region to reduce the race, but this *always* has the potential to fail - that's why there's the check in smbd/close.c Jeremy.
Check out the change I made to hold the locks longer. Jeremy.