[Samba] PATCH: Samba/Win2K renaming bug
Flynn
flynn at kodachi.com
Mon Feb 25 00:57:04 GMT 2002
Hi there. I'm running Samba 2.2.3a on RedHat 7.1 + kernel 2.4.17. My
girlfriend's laptop is running Windows 2000 + service pack 2. She has
about a thousand MP3s stored on a Samba share on my RedHat machine. We
were finding that she couldn't rename any MP3s on the Samba share: she
would always get a sharing violation.
After a day of reading Samba logs at debug 10 (_such_ fun [ ;) ]) and
adding debug information so I could figure out what was up, I found some
oddness in check_file_sharing. It was always reporting the MP3s as having
a single share mode of op_type _0_, locked by the Samba server that was
trying to do the rename.
op_type 0 seemed to imply an unlocked share, so I went in to see if I
could tweak the server to work in that case. A full patch (including a
TON of DEBUG() changes) is included below -- in brief, though, I changed
the test in check_file_sharing (open.c:1234) from
if (BATCH_OPLOCK_TYPE(share_entry->op_type))
to
if (BATCH_OPLOCK_TYPE(share_entry->op_type) ||
NO_OPLOCK_TYPE(share_entry->op_type))
after adding a NO_OPLOCK_TYPE macro in include/smb.h. I then had to
re-enable the code that JRA removed (open.c:1237 - 1273) -- presumably the
server ends up trying to break a lock that's not held without that change.
I don't know much about CIFS or Samba, so I don't know if this is actually
the right solution. Please advise -- I'd like to fix this problem for
real if my hac-- err, modification isn't correct. [ :) ]
Thanks,
Flynn
<flynn at kodachi.com>
==== begin patch ====
--- ../samba-2.2.3a/source/smbd/open.c Sat Feb 2 16:46:56 2002
+++ source/smbd/open.c Mon Feb 25 00:11:06 2002
@@ -1200,15 +1200,38 @@
SMB_DEV_T dev;
SMB_INO_T inode;
- if (vfs_stat(conn,fname,&sbuf) == -1)
+ DEBUG(3, ("check_file_sharing: entering, checking %s\n", fname));
+
+ if (vfs_stat(conn,fname,&sbuf) == -1) {
+ DEBUG(3, ("check_file_sharing: can't stat %s, returning TRUE\n",
+ fname));
+
return(True);
+ }
dev = sbuf.st_dev;
inode = sbuf.st_ino;
+ DEBUG(3, ("check_file_sharing: call lock_share_entry\n"));
lock_share_entry(conn, dev, inode);
+
+ DEBUG(3, ("check_file_sharing: call get_share_modes\n"));
num_share_modes = get_share_modes(conn, dev, inode, &old_shares);
+ DEBUG(3, ("check_file_sharing: num_share_modes: %d\n", num_share_modes));
+
+ /* This code was shamelessly lifted from locking/locking.c. */
+ for (i = 0; i < num_share_modes; i++) {
+ share_mode_entry *entry_p = &old_shares[i];
+
+ DEBUG(5, ("check_file_sharing: share mode [%d]:\n"
+ " pid = %u, share_mode = 0x%x, port = 0x%x, type = 0x%x,\n"
+ " file_id = %lu, dev = 0x%x, inode = %.0f\n",
+ i, entry_p->pid, entry_p->share_mode, entry_p->op_port,
+ entry_p->op_type, entry_p->share_file_id,
+ (unsigned int) entry_p->dev, (double) entry_p->inode));
+ }
+
/*
* Check if the share modes will give us access.
*/
@@ -1231,10 +1254,12 @@
* Check if someone has an oplock on this file. If so we must
* break it before continuing.
*/
- if(BATCH_OPLOCK_TYPE(share_entry->op_type))
+
+ if (BATCH_OPLOCK_TYPE(share_entry->op_type) ||
+ NO_OPLOCK_TYPE(share_entry->op_type))
{
-#if 0
+#if 1
/* JRA. Try removing this code to see if the new oplock changes
fix the problem. I'm dubious, but Andrew is recommending we
@@ -1297,22 +1322,26 @@
* this to proceed. This takes precedence over share modes.
*/
- if(!rename_op && GET_ALLOW_SHARE_DELETE(share_entry->share_mode))
+ if(!rename_op && GET_ALLOW_SHARE_DELETE(share_entry->share_mode)) {
+ DEBUG(3, ("check_file_sharing: allowing delete\n"));
continue;
+ }
/*
* Someone else has a share lock on it, check to see
* if we can too.
*/
- if ((GET_DENY_MODE(share_entry->share_mode) != DENY_DOS) ||
- (share_entry->pid != pid))
+ if ((GET_DENY_MODE(share_entry->share_mode) != DENY_DOS) ||
+ (share_entry->pid != pid)) {
+ DEBUG(3, ("check_file_sharing: bailing early\n"));
goto free_and_exit;
-
+ }
} /* end for */
if(broke_oplock)
{
+ DEBUG(3, ("check_file_sharing: reloading share modes\n"));
SAFE_FREE(old_shares);
num_share_modes = get_share_modes(conn, dev, inode, &old_shares);
}
@@ -1332,7 +1361,7 @@
ret = True;
free_and_exit:
-
+ DEBUG(3, ("check_file_sharing: returning %s\n", ret ? "True" : "False"));
unlock_share_entry(conn, dev, inode);
SAFE_FREE(old_shares);
return(ret);
--- ../samba-2.2.3a/source/smbd/reply.c Sat Feb 2 16:46:56 2002
+++ source/smbd/reply.c Mon Feb 25 00:02:37 2002
@@ -3621,12 +3621,19 @@
static NTSTATUS can_rename(char *fname,connection_struct *conn)
{
- if (!CAN_WRITE(conn))
+ DEBUG(4, ("can_rename: checking %s\n", fname));
+
+ if (!CAN_WRITE(conn)) {
+ DEBUG(4, ("can_rename: cannot write\n"));
return NT_STATUS_ACCESS_DENIED;
+ }
- if (!check_file_sharing(conn,fname,True))
+ if (!check_file_sharing(conn,fname,True)) {
+ DEBUG(4, ("can_rename: sharing violation\n"));
return NT_STATUS_SHARING_VIOLATION;
+ }
+ DEBUG(4, ("can_rename: returning OK\n"));
return NT_STATUS_OK;
}
@@ -3651,9 +3658,16 @@
*directory = *mask = 0;
+ DEBUG(3, ("rename_internals: entering\n"
+ " name == %s\n"
+ " newname == %s\n",
+ name, newname));
+
rc = unix_convert(name,conn,0,&bad_path1,&sbuf1);
unix_convert(newname,conn,newname_last_component,&bad_path2,&sbuf2);
+ DEBUG(3, ("rename_internals: both names unix_converted, rc %d\n", rc));
+
/*
* Split the old name into directory and last component
* strings. Note that unix_convert may have stripped off a
@@ -3683,11 +3697,21 @@
* Tine Smukavec <valentin.smukavec at hermes.si>.
*/
- if (!rc && is_mangled(mask))
- check_mangled_cache( mask );
+ if (!rc) {
+ DEBUG(3, ("rename_internals: checking for mangling\n"));
+
+ if (is_mangled(mask)) {
+ DEBUG(3,
+ ("rename_internals: mangled, checking mangled cache\n"));
+
+ check_mangled_cache( mask );
+ }
+ }
has_wild = ms_has_wild(mask);
+ DEBUG(3, ("rename_internals: has_wild %d\n", has_wild));
+
if (!has_wild) {
pstring zdirectory;
pstring znewname;
@@ -3710,10 +3734,16 @@
pstrcpy(newname, tmpstr);
}
- DEBUG(3,("rename_internals: case_sensitive = %d, case_preserve = %d, short case preserve = %d, \
-directory = %s, newname = %s, newname_last_component = %s, is_8_3 = %d\n",
- case_sensitive, case_preserve, short_case_preserve, directory,
- newname, newname_last_component, is_short_name));
+ DEBUG(3,
+ ("rename_internals:"
+ " no wildcards, case_sensitive %d,"
+ " case_preserve %d, short case preserve %d, is_8_3 %d\n"
+ " directory = %s\n"
+ " newname = %s\n"
+ " newname_last_component = %s\n",
+ case_sensitive, case_preserve, short_case_preserve,
+ is_short_name,
+ directory, newname, newname_last_component));
/*
* Check for special case with case preserving and not
@@ -3731,6 +3761,8 @@
strcsequal(directory, newname)) {
pstring newname_modified_last_component;
+ DEBUG(3, ("rename_internals: just a case change\n"));
+
/*
* Get the last component of the modified name.
* Note that we guarantee that newname contains a '/'
@@ -3749,9 +3781,12 @@
}
}
+ DEBUG(3, ("rename_internals: resolving wildcards (why?)\n"));
resolve_wildcards(directory,newname);
+ DEBUG(3, ("rename_internals: done resolving wildcards\n"));
+
/*
* The source object must exist.
*/
@@ -3781,11 +3816,14 @@
return error;
}
+ DEBUG(3, ("rename_internals: checking can_rename\n"));
error = can_rename(directory,conn);
if (!NT_STATUS_IS_OK(error)) {
- DEBUG(3,("rename_internals: Error %s rename %s -> %s\n",
- get_nt_error_msg(error), directory,newname));
+ DEBUG(3,
+ ("rename_internals:"
+ " Error from can_rename: %s rename %s -> %s\n",
+ get_nt_error_msg(error), directory,newname));
return error;
}
@@ -3808,6 +3846,8 @@
return NT_STATUS_OBJECT_NAME_COLLISION;
}
+ DEBUG(3, ("rename_internals: calling vfs_ops.rename\n"));
+
if(conn->vfs_ops.rename(conn,zdirectory, znewname) == 0) {
DEBUG(3,("rename_internals: succeeded doing rename on %s -> %s\n",
directory,newname));
@@ -3819,8 +3859,10 @@
else
error = map_nt_error_from_unix(errno);
- DEBUG(3,("rename_internals: Error %s rename %s -> %s\n",
- get_nt_error_msg(error), directory,newname));
+ DEBUG(3,
+ ("rename_internals:"
+ " Error from vfs_ops.rename: %s rename %s -> %s\n",
+ get_nt_error_msg(error), directory,newname));
return error;
} else {
--- ../samba-2.2.3a/source/include/smb.h Sat Feb 2 16:46:40 2002
+++ source/include/smb.h Sun Feb 24 23:25:37 2002
@@ -1426,6 +1426,7 @@
#define BATCH_OPLOCK 2
#define LEVEL_II_OPLOCK 4
+#define NO_OPLOCK_TYPE(lck) ((lck) == NO_OPLOCK)
#define EXCLUSIVE_OPLOCK_TYPE(lck) ((lck) & (EXCLUSIVE_OPLOCK|BATCH_OPLOCK))
#define BATCH_OPLOCK_TYPE(lck) ((lck) & BATCH_OPLOCK)
#define LEVEL_II_OPLOCK_TYPE(lck) ((lck) & LEVEL_II_OPLOCK)
==== end patch ====
--
Don't reinvent the flat tire.
(From the "Can't Happen" paper by Ian Darwin and Geoff Collyer)
More information about the samba
mailing list