[linux-cifs-client] unable to chmod

[Jeremy Allison]

On Wed, Jan 31, 2007 at 08:51:58PM +0200, Juran David wrote:
> On Wed, 2007-01-31 at 10:28 -0800, ext Jeremy Allison wrote:
> > On Wed, Jan 31, 2007 at 07:54:42PM +0200, Juran David wrote:
> > > 
> > > I think you misunderstood the question. I just stumbled upon this
> > > behaviour as well and the point is that this is not a create. The file
> > > already exists, but still the 'create mask' blocks the 'chmod' operation
> > > (regardless of the security mask). Is that intended?
> > 
> > Yes. "create mask" is a misnomer - it's a generic mask
> > on operations to ensure things filtered by the administrator
> > remain filtered.
> 
> I see... But isn't that function supposed to be covered by the 'security
> mask' parameter? As I see it, the drawback of having the 'create mask'
> as a "generic" mask is that one might want to have a certain default
> mode for newly created files, but still allow users to change to
> "higher" permissions on files once they are created. 

Ok, here's the patch I just applied to SVN source. Due to the
refactoring you'll need to change it for earlier versions of
Samba, but the intent should be very clear. Let me know if you
need help porting it to the version of Samba you're using.

Jeremy.
-------------- next part --------------
Index: smbd/trans2.c
===================================================================
--- smbd/trans2.c	(revision 21095)
+++ smbd/trans2.c	(working copy)
@@ -1005,12 +1005,24 @@
  Map wire perms onto standard UNIX permissions. Obey share restrictions.
 ****************************************************************************/
 
-static mode_t unix_perms_from_wire( connection_struct *conn, SMB_STRUCT_STAT *pst, uint32 perms)
+enum perm_type { PERM_NEW_FILE, PERM_NEW_DIR, PERM_EXISTING_FILE, PERM_EXISTING_DIR};
+
+static NTSTATUS unix_perms_from_wire( connection_struct *conn,
+				SMB_STRUCT_STAT *psbuf,
+				uint32 perms,
+				enum perm_type ptype,
+				mode_t *ret_perms)
 {
 	mode_t ret = 0;
 
-	if (perms == SMB_MODE_NO_CHANGE)
-		return pst->st_mode;
+	if (perms == SMB_MODE_NO_CHANGE) {
+		if (!VALID_STAT(*psbuf)) {
+			return NT_STATUS_INVALID_PARAMETER;
+		} else {
+			*ret_perms = psbuf->st_mode;
+			return NT_STATUS_OK;
+		}
+	}
 
 	ret |= ((perms & UNIX_X_OTH ) ? S_IXOTH : 0);
 	ret |= ((perms & UNIX_W_OTH ) ? S_IWOTH : 0);
@@ -1031,18 +1043,34 @@
 	ret |= ((perms & UNIX_SET_UID ) ? S_ISUID : 0);
 #endif
 
-	if (VALID_STAT(*pst) && S_ISDIR(pst->st_mode)) {
+	switch (ptype) {
+	case PERM_NEW_FILE:
+		/* Apply mode mask */
+		ret &= lp_create_mask(SNUM(conn));
+		/* Add in force bits */
+		ret |= lp_force_create_mode(SNUM(conn));
+		break;
+	case PERM_NEW_DIR:
 		ret &= lp_dir_mask(SNUM(conn));
 		/* Add in force bits */
 		ret |= lp_force_dir_mode(SNUM(conn));
-	} else {
+		break;
+	case PERM_EXISTING_FILE:
 		/* Apply mode mask */
-		ret &= lp_create_mask(SNUM(conn));
+		ret &= lp_security_mask(SNUM(conn));
 		/* Add in force bits */
-		ret |= lp_force_create_mode(SNUM(conn));
+		ret |= lp_force_security_mode(SNUM(conn));
+		break;
+	case PERM_EXISTING_DIR:
+		/* Apply mode mask */
+		ret &= lp_dir_security_mask(SNUM(conn));
+		/* Add in force bits */
+		ret |= lp_force_dir_security_mode(SNUM(conn));
+		break;
 	}
 
-	return ret;
+	*ret_perms = ret;
+	return NT_STATUS_OK;
 }
 
 /****************************************************************************
@@ -4589,18 +4617,18 @@
 #endif
 	SMB_DEV_T dev = (SMB_DEV_T)0;
 	uint32 raw_unixmode = IVAL(pdata,84);
+	NTSTATUS status;
 	mode_t unixmode;
 
 	if (total_data < 100) {
 		return NT_STATUS_INVALID_PARAMETER;
 	}
 
-	if (raw_unixmode == SMB_MODE_NO_CHANGE) {
-		return NT_STATUS_INVALID_PARAMETER;
+	status = unix_perms_from_wire(conn, psbuf, raw_unixmode, PERM_NEW_FILE, &unixmode);
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
 	}
 
-	unixmode = unix_perms_from_wire(conn, psbuf, raw_unixmode);
-
 #if defined(HAVE_MAKEDEV)
 	dev = makedev(dev_major, dev_minor);
 #endif
@@ -4649,7 +4677,7 @@
 	}
 
 	if (SMB_VFS_STAT(conn, fname, psbuf) != 0) {
-		NTSTATUS status = map_nt_error_from_unix(errno);
+		status = map_nt_error_from_unix(errno);
 		SMB_VFS_UNLINK(conn,fname);
 		return status;
 	}
@@ -4675,6 +4703,7 @@
 	gid_t set_grp = (uid_t)SMB_GID_NO_CHANGE;
 	NTSTATUS status = NT_STATUS_OK;
 	BOOL delete_on_fail = False;
+	enum perm_type ptype;
 
 	if (total_data < 100) {
 		return NT_STATUS_INVALID_PARAMETER;
@@ -4698,8 +4727,22 @@
 	set_owner = (uid_t)IVAL(pdata,40);
 	set_grp = (gid_t)IVAL(pdata,48);
 	raw_unixmode = IVAL(pdata,84);
-	unixmode = unix_perms_from_wire(conn, psbuf, raw_unixmode);
 
+	if (VALID_STAT(*psbuf)) {
+		if (S_ISDIR(psbuf->st_mode)) {
+			ptype = PERM_EXISTING_DIR;
+		} else {
+			ptype = PERM_EXISTING_FILE;
+		}
+	} else {
+		ptype = PERM_NEW_FILE;
+	}
+
+	status = unix_perms_from_wire(conn, psbuf, raw_unixmode, ptype, &unixmode);
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
+	}
+
 	DEBUG(10,("smb_set_file_unix_basic: SMB_SET_FILE_UNIX_BASIC: name = %s \
 size = %.0f, uid = %u, gid = %u, raw perms = 0%o\n",
 		fname, (double)size, (unsigned int)set_owner, (unsigned int)set_grp, (int)raw_unixmode));