Module Name:    src
Committed By:   riz
Date:           Mon Oct  1 19:55:22 UTC 2012

Modified Files:
        src/sys/ufs/ufs [netbsd-6]: ufs_quota2.c
        src/tests/fs/ffs [netbsd-6]: h_quota2_tests.c t_miscquota.sh

Log Message:
Pull up following revision(s) (requested by bouyer in ticket #580):
        tests/fs/ffs/h_quota2_tests.c: revision 1.4
        tests/fs/ffs/t_miscquota.sh: revision 1.7
        sys/ufs/ufs/ufs_quota2.c: revision 1.35
Fix quota2 list corruption issue when defaultquotas are 0 (deny any file
and block allocation).
When quota2_check() is called with an uid not yet in the list,
getinoquota2() will call quota2_q2ealloc() to allocate a new entry for this
uid. quota2_q2ealloc() will remove an entry from the free list and
put it at the head of the corresponding hash list, and flush the block
containing the header if it's not the one also containing the allocated entry.
quota2_q2ealloc() then return the alocated entry and corresponding block
to caller (getinoquota2() here), which returns it to quota2_check().
quota2_check() then checks if the allocation can succeed, and returns and
error if not and calls brelse() on the buffer (because from his POW no
change was made to the entry), effectively discarding changes
to the entry that may have been made by quota2_q2ealloc().
Fix by always bwrite()ing the entry in quota2_q2ealloc(), and re-reading
the entry in caller.
Add test cases for the bug fixed in sys/ufs/ufs/ufs_quota2.c 1.35:
when a on-disk block/inode allocation triggers allocating a new
quota entry, the new quota entry is not in the quota2 header block,
and the allocation will later be denied, the changes to the quota block would
not be flushed to disk, leading to list corruption (detected by fsck).


To generate a diff of this commit:
cvs rdiff -u -r1.34 -r1.34.2.1 src/sys/ufs/ufs/ufs_quota2.c
cvs rdiff -u -r1.3 -r1.3.4.1 src/tests/fs/ffs/h_quota2_tests.c
cvs rdiff -u -r1.6 -r1.6.2.1 src/tests/fs/ffs/t_miscquota.sh

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/ufs/ufs/ufs_quota2.c
diff -u src/sys/ufs/ufs/ufs_quota2.c:1.34 src/sys/ufs/ufs/ufs_quota2.c:1.34.2.1
--- src/sys/ufs/ufs/ufs_quota2.c:1.34	Mon Feb 13 06:23:41 2012
+++ src/sys/ufs/ufs/ufs_quota2.c	Mon Oct  1 19:55:22 2012
@@ -1,4 +1,4 @@
-/* $NetBSD: ufs_quota2.c,v 1.34 2012/02/13 06:23:41 dholland Exp $ */
+/* $NetBSD: ufs_quota2.c,v 1.34.2.1 2012/10/01 19:55:22 riz Exp $ */
 /*-
   * Copyright (c) 2010 Manuel Bouyer
   * All rights reserved.
@@ -26,7 +26,7 @@
   */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ufs_quota2.c,v 1.34 2012/02/13 06:23:41 dholland Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ufs_quota2.c,v 1.34.2.1 2012/10/01 19:55:22 riz Exp $");
 
 #include <sys/buf.h>
 #include <sys/param.h>
@@ -290,8 +290,7 @@ quota2_umount(struct mount *mp, int flag
 }
 
 static int 
-quota2_q2ealloc(struct ufsmount *ump, int type, uid_t uid, struct dquot *dq,
-    struct buf **bpp, struct quota2_entry **q2ep)
+quota2_q2ealloc(struct ufsmount *ump, int type, uid_t uid, struct dquot *dq)
 {
 	int error, error2;
 	struct buf *hbp, *bp;
@@ -361,8 +360,7 @@ quota2_q2ealloc(struct ufsmount *ump, in
 	if (hbp != bp) {
 		bwrite(hbp);
 	}
-	*q2ep = q2e;
-	*bpp = bp;
+	bwrite(bp);
 	return 0;
 }
 
@@ -416,18 +414,17 @@ getinoquota2(struct inode *ip, bool allo
 			}
 			/* need to alloc a new on-disk quot */
 			mutex_enter(&dqlock);
-			error = quota2_q2ealloc(ump, i, ino_ids[i], dq,
-			    &bpp[i], &q2ep[i]);
+			error = quota2_q2ealloc(ump, i, ino_ids[i], dq);
 			mutex_exit(&dqlock);
 			if (error)
 				return error;
-		} else {
-			error = getq2e(ump, i, dq->dq2_lblkno,
-			    dq->dq2_blkoff, &bpp[i], &q2ep[i],
-			    modify ? B_MODIFY : 0);
-			if (error)
-				return error;
 		}
+		KASSERT(dq->dq2_lblkno != 0 || dq->dq2_blkoff != 0);
+		error = getq2e(ump, i, dq->dq2_lblkno,
+		    dq->dq2_blkoff, &bpp[i], &q2ep[i],
+		    modify ? B_MODIFY : 0);
+		if (error)
+			return error;
 	}
 	return 0;
 }
@@ -622,13 +619,14 @@ quota2_handle_cmd_put(struct ufsmount *u
 	if (dq->dq2_lblkno == 0 && dq->dq2_blkoff == 0) {
 		/* need to alloc a new on-disk quot */
 		mutex_enter(&dqlock);
-		error = quota2_q2ealloc(ump, key->qk_idtype, key->qk_id, dq,
-		    &bp, &q2ep);
+		error = quota2_q2ealloc(ump, key->qk_idtype, key->qk_id, dq);
 		mutex_exit(&dqlock);
-	} else {
-		error = getq2e(ump, key->qk_idtype, dq->dq2_lblkno,
-		    dq->dq2_blkoff, &bp, &q2ep, B_MODIFY);
+		if (error)
+			goto out_il;
 	}
+	KASSERT(dq->dq2_lblkno != 0 || dq->dq2_blkoff != 0);
+	error = getq2e(ump, key->qk_idtype, dq->dq2_lblkno,
+	    dq->dq2_blkoff, &bp, &q2ep, B_MODIFY);
 	if (error)
 		goto out_il;
 	

Index: src/tests/fs/ffs/h_quota2_tests.c
diff -u src/tests/fs/ffs/h_quota2_tests.c:1.3 src/tests/fs/ffs/h_quota2_tests.c:1.3.4.1
--- src/tests/fs/ffs/h_quota2_tests.c:1.3	Sat Jun 11 18:03:17 2011
+++ src/tests/fs/ffs/h_quota2_tests.c	Mon Oct  1 19:55:22 2012
@@ -1,4 +1,4 @@
-/*	$NetBSD: h_quota2_tests.c,v 1.3 2011/06/11 18:03:17 christos Exp $	*/
+/*	$NetBSD: h_quota2_tests.c,v 1.3.4.1 2012/10/01 19:55:22 riz Exp $	*/
 
 /*
  * rump server for advanced quota tests
@@ -34,7 +34,19 @@ quota_test0(const char *testopts)
 	static char buf[512];
 	int fd;
 	int error;
-	rump_sys_chown(".", TEST_NONROOT_ID, TEST_NONROOT_ID);
+	unsigned int i;
+	int chowner = 1;
+	for (i =0; testopts && i < strlen(testopts); i++) {
+		switch(testopts[i]) {
+		case 'C':
+			chowner = 0;
+			break;
+		default:
+			errx(1, "test4: unknown option %c", testopts[i]);
+		}
+	}
+	if (chowner)
+		rump_sys_chown(".", TEST_NONROOT_ID, TEST_NONROOT_ID);
 	rump_sys_chmod(".", 0777);
 	if (rump_sys_setegid(TEST_NONROOT_ID) != 0) {
 		error = errno;

Index: src/tests/fs/ffs/t_miscquota.sh
diff -u src/tests/fs/ffs/t_miscquota.sh:1.6 src/tests/fs/ffs/t_miscquota.sh:1.6.2.1
--- src/tests/fs/ffs/t_miscquota.sh:1.6	Mon Feb 13 17:55:12 2012
+++ src/tests/fs/ffs/t_miscquota.sh	Mon Oct  1 19:55:22 2012
@@ -1,4 +1,4 @@
-# $NetBSD: t_miscquota.sh,v 1.6 2012/02/13 17:55:12 dholland Exp $ 
+# $NetBSD: t_miscquota.sh,v 1.6.2.1 2012/10/01 19:55:22 riz Exp $ 
 #
 #  Copyright (c) 2011 Manuel Bouyer
 #  All rights reserved.
@@ -53,6 +53,13 @@ test_case log_unlink_remount quota_log \
     -oRL le 1 user
 
 
+test_case_root defaut_deny_user quota_defaut_deny \
+    "new quota entry denied by default entry" 5 -b le 1 user
+
+test_case_root defaut_deny_user_big quota_defaut_deny \
+    "new quota entry denied by default entry, with list on more than one block" 5000 -b le 1 user
+
+
 quota_walk_list()
 {
 	create_ffs_server $*
@@ -162,3 +169,45 @@ quota_log()
 	#shutdown and check filesystem
 	rump_quota_shutdown
 }
+
+quota_defaut_deny()
+{
+	local nusers=$1; shift
+	create_ffs_server $*
+	local q=$4
+	local expect
+
+	case ${q} in
+	user)
+		expect=u
+		fail=g
+		;;
+	group)
+		expect=g
+		fail=u
+		;;
+	*)
+		atf_fail "wrong quota type"
+		;;
+	esac
+
+	# create $nusers users, so we are sure the free list has entries
+	# from block 1. Start from 10, as non-root id is 1.
+	# set default to deny all
+	( echo "@format netbsd-quota-dump v1"
+	  echo "# idtype id objtype   hard soft usage expire grace"
+	  echo "$q default block   0 0 0 0 0"
+	  echo "$q default file   0 0 0 0 0"
+	  local i=10;
+	  while [ $i -lt $(($nusers + 10)) ]; do
+		echo "$q $i block   0 0 0 0 0"
+		echo "$q $i file   0 0 0 0 0"
+		i=$((i + 1))
+	  done
+	) | atf_check -s exit:0 \
+		   env LD_PRELOAD=/usr/lib/librumphijack.so RUMPHIJACK=vfs=getvfsstat,blanket=/mnt quotarestore -d /mnt
+	atf_check -s exit:0 rump.halt
+	#now start the server which does the limits tests
+	$(atf_get_srcdir)/h_quota2_tests -oC -b 0 ${IMG} ${RUMP_SERVER}
+	rump_quota_shutdown
+}

Reply via email to