[PATCHES] ntquota - differentiate between "no quota" and error condition

Uri Simchoni uri at samba.org
Wed Mar 30 11:40:05 UTC 2016


Hi,

The attached patch set changes the way the VFS indicates "no quota" for
a user or a group. It is supposed not to change behavior (except for a
small bugfix in smbcquotas client tool).

Before the patches, the sysquota drivers would report an error if no
quota was assigned. After the patch they report zero quota. This is
meant to be the convention for VFS modules to follow as well.

The purpose of this change is to allow stacking of VFS modules - allow a
module to say "get the quota from underlying module but do something
special if there is no quota", but at the same time not cover-up failures.

When differentiating between real errors and no-quota in xfs and nfs, I
was inspired by the "quota (8)" utility which also has to cope with this.

Review appreciated.

Thanks,
Uri.
-------------- next part --------------
From 38920bd0893d381bc3eee55363ac19fb6bece3cd Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Tue, 29 Mar 2016 23:30:23 +0300
Subject: [PATCH 1/8] nt-quotas: vfs_get_ntquota() return NTSTATUS

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/smbd/ntquotas.c | 19 ++++++++++++-------
 source3/smbd/nttrans.c  |  4 +++-
 source3/smbd/proto.h    |  3 ++-
 source3/smbd/trans2.c   |  6 ++++--
 4 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/source3/smbd/ntquotas.c b/source3/smbd/ntquotas.c
index aa2ec3b..a36196b 100644
--- a/source3/smbd/ntquotas.c
+++ b/source3/smbd/ntquotas.c
@@ -76,7 +76,8 @@ static uint64_t limit_blk2inodes(uint64_t in)
 	return ret;	
 }
 
-int vfs_get_ntquota(files_struct *fsp, enum SMB_QUOTA_TYPE qtype, struct dom_sid *psid, SMB_NTQUOTA_STRUCT *qt)
+NTSTATUS vfs_get_ntquota(files_struct *fsp, enum SMB_QUOTA_TYPE qtype,
+			 struct dom_sid *psid, SMB_NTQUOTA_STRUCT *qt)
 {
 	int ret;
 	SMB_DISK_QUOTA D;
@@ -84,8 +85,9 @@ int vfs_get_ntquota(files_struct *fsp, enum SMB_QUOTA_TYPE qtype, struct dom_sid
 
 	ZERO_STRUCT(D);
 
-	if (!fsp||!fsp->conn||!qt)
-		return (-1);
+	if (!fsp || !fsp->conn || !qt) {
+		return NT_STATUS_INTERNAL_ERROR;
+	}
 
 	ZERO_STRUCT(*qt);
 
@@ -94,6 +96,7 @@ int vfs_get_ntquota(files_struct *fsp, enum SMB_QUOTA_TYPE qtype, struct dom_sid
 	if (psid && !sid_to_uid(psid, &id.uid)) {
 		DEBUG(0,("sid_to_uid: failed, SID[%s]\n",
 			 sid_string_dbg(psid)));
+		return NT_STATUS_NO_SUCH_USER;
 	}
 
 	ret = SMB_VFS_GET_QUOTA(fsp->conn, ".", qtype, id, &D);
@@ -102,7 +105,7 @@ int vfs_get_ntquota(files_struct *fsp, enum SMB_QUOTA_TYPE qtype, struct dom_sid
 		qt->sid    = *psid;
 
 	if (ret!=0) {
-		return ret;
+		return map_nt_error_from_unix(errno);
 	}
 		
 	qt->usedspace = (uint64_t)D.curblocks*D.bsize;
@@ -110,8 +113,7 @@ int vfs_get_ntquota(files_struct *fsp, enum SMB_QUOTA_TYPE qtype, struct dom_sid
 	qt->hardlim = limit_unix2nt(D.hardlimit, D.bsize);
 	qt->qflags = D.qflags;
 
-	
-	return 0;
+	return NT_STATUS_OK;
 }
 
 int vfs_set_ntquota(files_struct *fsp, enum SMB_QUOTA_TYPE qtype, struct dom_sid *psid, SMB_NTQUOTA_STRUCT *qt)
@@ -181,6 +183,7 @@ int vfs_get_user_ntquota_list(files_struct *fsp, SMB_NTQUOTA_LIST **qt_list)
 		SMB_NTQUOTA_STRUCT tmp_qt;
 		SMB_NTQUOTA_LIST *tmp_list_ent;
 		struct dom_sid	sid;
+		NTSTATUS status;
 
 		ZERO_STRUCT(tmp_qt);
 
@@ -191,7 +194,9 @@ int vfs_get_user_ntquota_list(files_struct *fsp, SMB_NTQUOTA_LIST **qt_list)
 
 		uid_to_sid(&sid, usr->pw_uid);
 
-		if (vfs_get_ntquota(fsp, SMB_USER_QUOTA_TYPE, &sid, &tmp_qt)!=0) {
+		status =
+		    vfs_get_ntquota(fsp, SMB_USER_QUOTA_TYPE, &sid, &tmp_qt);
+		if (!NT_STATUS_IS_OK(status)) {
 			DEBUG(5,("no quota entry for sid[%s] path[%s]\n",
 				 sid_string_dbg(&sid),
 				 fsp->conn->connectpath));
diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c
index 3a2c35f..232cc68 100644
--- a/source3/smbd/nttrans.c
+++ b/source3/smbd/nttrans.c
@@ -2509,7 +2509,9 @@ static void call_nt_transact_get_user_quota(connection_struct *conn,
 				return;
 			}
 
-			if (vfs_get_ntquota(fsp, SMB_USER_QUOTA_TYPE, &sid, &qt)!=0) {
+			nt_status = vfs_get_ntquota(fsp, SMB_USER_QUOTA_TYPE,
+						    &sid, &qt);
+			if (!NT_STATUS_IS_OK(nt_status)) {
 				ZERO_STRUCT(qt);
 				/*
 				 * we have to return zero's in all fields
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 8e8cbc3..3612034 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -600,7 +600,8 @@ NTSTATUS notify_walk(struct notify_context *notify,
 
 /* The following definitions come from smbd/ntquotas.c  */
 
-int vfs_get_ntquota(files_struct *fsp, enum SMB_QUOTA_TYPE qtype, struct dom_sid *psid, SMB_NTQUOTA_STRUCT *qt);
+NTSTATUS vfs_get_ntquota(files_struct *fsp, enum SMB_QUOTA_TYPE qtype,
+			 struct dom_sid *psid, SMB_NTQUOTA_STRUCT *qt);
 int vfs_set_ntquota(files_struct *fsp, enum SMB_QUOTA_TYPE qtype, struct dom_sid *psid, SMB_NTQUOTA_STRUCT *qt);
 int vfs_get_user_ntquota_list(files_struct *fsp, SMB_NTQUOTA_LIST **qt_list);
 void *init_quota_handle(TALLOC_CTX *mem_ctx);
diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index f351008..0e1c6d9 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -3712,9 +3712,11 @@ cBytesSector=%u, cUnitTotal=%u, cUnitAvail=%d\n", (unsigned int)bsize, (unsigned
 				return NT_STATUS_ACCESS_DENIED;
 			}
 
-			if (vfs_get_ntquota(&fsp, SMB_USER_FS_QUOTA_TYPE, NULL, &quotas)!=0) {
+			status = vfs_get_ntquota(&fsp, SMB_USER_FS_QUOTA_TYPE,
+						 NULL, &quotas);
+			if (!NT_STATUS_IS_OK(status)) {
 				DEBUG(0,("vfs_get_ntquota() failed for service [%s]\n",lp_servicename(talloc_tos(), SNUM(conn))));
-				return map_nt_error_from_unix(errno);
+				return status;
 			}
 
 			data_len = 48;
-- 
2.5.5


From ba8bb05d6a45b892f4eda443d8076c8ac44888b0 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Tue, 29 Mar 2016 23:05:09 +0300
Subject: [PATCH 2/8] nt-quotas: return 0 as indication of no quota

When getting user quota, the correct value to indicate "no quota"
is 0, not -1.

In [MS-FSCC] section 2.4.33 it is written that -1 designates no-quota.
However, careful read of that section shows that this designation is only
true when setting the quota, and this section says nothing about getting
the quota.

In [MS-FSA] section 2.1.5.20, it is written that "If SidList includes a SID
that does not map to an existing SID in the Open.File.Volume.QuotaInformation
list, the object store MUST return a FILE_QUOTA_INFORMATION structure
(as specified in [MS-FSCC] section 2.4.33) that is filled with zeros.

This is also verified experimentally and cleared with dochelp.

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/smbd/ntquotas.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/source3/smbd/ntquotas.c b/source3/smbd/ntquotas.c
index a36196b..b9fd2f1 100644
--- a/source3/smbd/ntquotas.c
+++ b/source3/smbd/ntquotas.c
@@ -53,14 +53,6 @@ static uint64_t limit_unix2nt(uint64_t in, uint64_t bsize)
 
 	ret = (uint64_t)(in*bsize);
 	
-	if (ret < in) {
-		/* we overflow */
-		ret = SMB_NTQUOTAS_NO_LIMIT;
-	}
-
-	if (in == SMB_QUOTAS_NO_LIMIT)
-		ret = SMB_NTQUOTAS_NO_LIMIT;
-
 	return ret;
 }
 
-- 
2.5.5


From 503a511c7a9119a38e0de3e9fbb8d04d8ac4b10c Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Wed, 30 Mar 2016 13:59:39 +0300
Subject: [PATCH 3/8] ntquotas - skip entry if the quota is zero

When listing user quotas, do not list the user
if the driver returned success with zero quota -
this signals that no quota is assigned for that
user.

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/smbd/ntquotas.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/source3/smbd/ntquotas.c b/source3/smbd/ntquotas.c
index b9fd2f1..9b2e39a 100644
--- a/source3/smbd/ntquotas.c
+++ b/source3/smbd/ntquotas.c
@@ -189,6 +189,11 @@ int vfs_get_user_ntquota_list(files_struct *fsp, SMB_NTQUOTA_LIST **qt_list)
 		status =
 		    vfs_get_ntquota(fsp, SMB_USER_QUOTA_TYPE, &sid, &tmp_qt);
 		if (!NT_STATUS_IS_OK(status)) {
+			DEBUG(5, ("failed getting quota for uid[%ld] - %s\n",
+				  (long)usr->pw_uid, nt_errstr(status)));
+			continue;
+		}
+		if (tmp_qt.softlim == 0 && tmp_qt.hardlim == 0) {
 			DEBUG(5,("no quota entry for sid[%s] path[%s]\n",
 				 sid_string_dbg(&sid),
 				 fsp->conn->connectpath));
-- 
2.5.5


From 10ddf36a0df6a88807c0e705564699fdfee1c4b4 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Wed, 30 Mar 2016 14:05:49 +0300
Subject: [PATCH 4/8] sys-quotas: do not fail if user has no quota

If the user/group has no quota, do not treat that as
error condition. Instead, return zero quota.

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/lib/sysquotas_4A.c    | 16 ++--------------
 source3/lib/sysquotas_linux.c |  9 +--------
 2 files changed, 3 insertions(+), 22 deletions(-)

diff --git a/source3/lib/sysquotas_4A.c b/source3/lib/sysquotas_4A.c
index 244b612..674c4ee 100644
--- a/source3/lib/sysquotas_4A.c
+++ b/source3/lib/sysquotas_4A.c
@@ -104,13 +104,7 @@ int sys_get_vfs_quota(const char *path, const char *bdev, enum SMB_QUOTA_TYPE qt
 				return ret;
 			}
 
-			if ((D.dqb_curblocks==0)&&
-				(D.dqb_bsoftlimit==0)&&
-				(D.dqb_bhardlimit==0)) {
-				/* the upper layer functions don't want empty quota records...*/
-				return -1;
-			}
-
+			ret = 0;
 			break;
 #ifdef HAVE_GROUP_QUOTA
 		case SMB_GROUP_QUOTA_TYPE:
@@ -121,13 +115,7 @@ int sys_get_vfs_quota(const char *path, const char *bdev, enum SMB_QUOTA_TYPE qt
 				return ret;
 			}
 
-			if ((D.dqb_curblocks==0)&&
-				(D.dqb_bsoftlimit==0)&&
-				(D.dqb_bhardlimit==0)) {
-				/* the upper layer functions don't want empty quota records...*/
-				return -1;
-			}
-
+			ret = 0;
 			break;
 #endif /* HAVE_GROUP_QUOTA */
 		case SMB_USER_FS_QUOTA_TYPE:
diff --git a/source3/lib/sysquotas_linux.c b/source3/lib/sysquotas_linux.c
index bf3504a..5984626 100644
--- a/source3/lib/sysquotas_linux.c
+++ b/source3/lib/sysquotas_linux.c
@@ -447,14 +447,7 @@ int sys_get_vfs_quota(const char *path, const char *bdev, enum SMB_QUOTA_TYPE qt
 					}
 				}
 			}
-
-			if ((dp->curblocks==0)&&
-				(dp->softlimit==0)&&
-				(dp->hardlimit==0)) {
-				/* the upper layer functions don't want empty quota records...*/
-				return -1;
-			}
-
+			ret = 0;
 			break;
 		case SMB_USER_FS_QUOTA_TYPE:
 			id.uid = getuid();
-- 
2.5.5


From 6784814c1a70c52375e6aaa64fbf8b98c70f0e83 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Wed, 30 Mar 2016 13:00:29 +0300
Subject: [PATCH 5/8] xfs-quota: do not fail if user has no quota

XFS fails quotactl(Q_XGETQUOTA) with ENOENT if the user
or group has no quota assigned to it. This is not an error
condition - simply report 0 quota in this case.

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/lib/sysquotas_xfs.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/source3/lib/sysquotas_xfs.c b/source3/lib/sysquotas_xfs.c
index ccc7fc0..bea86d5 100644
--- a/source3/lib/sysquotas_xfs.c
+++ b/source3/lib/sysquotas_xfs.c
@@ -90,16 +90,28 @@ int sys_get_xfs_quota(const char *path, const char *bdev, enum SMB_QUOTA_TYPE qt
 			DEBUG(10,("sys_get_xfs_quota: path[%s] bdev[%s] SMB_USER_QUOTA_TYPE uid[%u]\n",
 				path, bdev, (unsigned)id.uid));
 
-			if ((ret=quotactl(QCMD(Q_XGETQUOTA,USRQUOTA), bdev, id.uid, (caddr_t)&D)))
+			ret=quotactl(QCMD(Q_XGETQUOTA,USRQUOTA), bdev, id.uid, (caddr_t)&D);
+			/* XFS fails with ENOENT if the user has no
+			 * quota. Our protocol in that case is to
+			 * succeed and return 0 as quota.
+			 */
+			if (ret != 0 && errno != ENOENT) {
 				return ret;
+			}
 			break;
 #ifdef HAVE_GROUP_QUOTA
 		case SMB_GROUP_QUOTA_TYPE:
 			DEBUG(10,("sys_get_xfs_quota: path[%s] bdev[%s] SMB_GROUP_QUOTA_TYPE gid[%u]\n",
 				path, bdev, (unsigned)id.gid));
 
-			if ((ret=quotactl(QCMD(Q_XGETQUOTA,GRPQUOTA), bdev, id.gid, (caddr_t)&D)))
+			ret=quotactl(QCMD(Q_XGETQUOTA,GRPQUOTA), bdev, id.gid, (caddr_t)&D);
+			/* XFS fails with ENOENT if the user has no
+			 * quota. Our protocol in that case is to
+			 * succeed and return 0 as quota.
+			 */
+			if (ret != 0 && errno != ENOENT) {
 				return ret;
+			}
 			break;
 #endif /* HAVE_GROUP_QUOTA */
 		case SMB_USER_FS_QUOTA_TYPE:
-- 
2.5.5


From 3c9439c3f6e6ab3a2dca5403b0d75b6ba7571a1c Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Wed, 30 Mar 2016 13:21:58 +0300
Subject: [PATCH 6/8] nfs-quota: do not fail on ECONNREFUSED

Trying to differentiate between "no quota" and real
error conditions - if the connection to rpc.quotad
is refused it could simply mean that the remote host
has no quota and therefore report this as success with
no quota.

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/lib/sysquotas_nfs.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/source3/lib/sysquotas_nfs.c b/source3/lib/sysquotas_nfs.c
index 4b37e34..fe46d3f 100644
--- a/source3/lib/sysquotas_nfs.c
+++ b/source3/lib/sysquotas_nfs.c
@@ -180,8 +180,20 @@ int sys_get_nfs_quota(const char *path, const char *bdev,
 			      timeout);
 
 	if (clnt_stat != RPC_SUCCESS) {
-		DEBUG(3, ("sys_get_nfs_quotas: clnt_call failed\n"));
-		ret = -1;
+		if (errno == ECONNREFUSED) {
+			/* If we cannot connect with rpc.quotad, it may
+			 * simply be because there's no quota on the remote
+			 * system
+			 */
+			DBG_INFO("clnt_call failed with ECONNREFUSED - "
+				 "assuming no quotas on server\n");
+			ret = 0;
+		} else {
+			int save_errno = errno;
+			DBG_NOTICE("clnt_call failed - %s\n", strerror(errno));
+			errno = save_errno;
+			ret = -1;
+		}
 		goto out;
 	}
 
-- 
2.5.5


From e2ac257833811349983c1d2d2c1a902798738d77 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Wed, 30 Mar 2016 14:02:31 +0300
Subject: [PATCH 7/8] smbd: do not cover up VFS failures to get quota

Now that the VFS follows the convention that get-quota
returns error only on error condition, and success
with zero quota if there is no quota assigned,
reply with an error if failing to obtain a user's
quota.

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/smbd/nttrans.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c
index 232cc68..fa3f74c 100644
--- a/source3/smbd/nttrans.c
+++ b/source3/smbd/nttrans.c
@@ -2512,12 +2512,7 @@ static void call_nt_transact_get_user_quota(connection_struct *conn,
 			nt_status = vfs_get_ntquota(fsp, SMB_USER_QUOTA_TYPE,
 						    &sid, &qt);
 			if (!NT_STATUS_IS_OK(nt_status)) {
-				ZERO_STRUCT(qt);
-				/*
-				 * we have to return zero's in all fields
-				 * instead of returning an error here
-				 * --metze
-				 */
+				reply_nterror(req, nt_status);
 			}
 
 			/* Realloc the size of parameters and data we will return */
-- 
2.5.5


From b22c988d0e5c2d08eb1f9098dc04a18f9ba80ae4 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Wed, 30 Mar 2016 14:20:44 +0300
Subject: [PATCH 8/8] smbcquotas: print "NO LIMIT" only if returned quota value
 is 0.

If the user being queried has no quota, the server returns 0 as
its quota. This is the observed smbd and Windows behavior, which
is also documented in [MS-FSA] 2.5.1.20.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11815

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/utils/smbcquotas.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/utils/smbcquotas.c b/source3/utils/smbcquotas.c
index 9e64319..e6f1dfb 100644
--- a/source3/utils/smbcquotas.c
+++ b/source3/utils/smbcquotas.c
@@ -236,7 +236,7 @@ static const char *quota_str_static(uint64_t val, bool special, bool _numeric)
 {
 	const char *result;
 
-	if (!_numeric&&special&&(val == SMB_NTQUOTAS_NO_LIMIT)) {
+	if (!_numeric && special && val == 0) {
 		return "NO LIMIT";
 	}
 	result = talloc_asprintf(talloc_tos(), "%"PRIu64, val);
-- 
2.5.5



More information about the samba-technical mailing list