Subject: kern/36878: overflow in raidframe parity rebuild status
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: None <dieter.NetBSD@pandora.be>
List: netbsd-bugs
Date: 09/01/2007 13:50:00
>Number:         36878
>Category:       kern
>Synopsis:       overflow in raidframe parity rebuild status
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Sep 01 13:50:00 +0000 2007
>Originator:     Dieter Roelants
>Release:        NetBSD 4.0_BETA2
>Organization:
>Environment:
System: NetBSD bsdusr.net 4.0_BETA2 NetBSD 4.0_BETA2 (GENERIC) #0: Tue Aug 28 12:16:38 CEST 2007 dieter@simult.amelgem.be:/build/obj.amd64.4/sys/arch/amd64/compile/GENERIC amd64
Architecture: x86_64
Machine: amd64
>Description:
	When rebuilding the parity of a RAID set, it can happen
	that querying the status of this process returns a (large)
	negative percentage.
>How-To-Repeat:
	Build a RAID set which is either large enough or has a low
	number of sectors per stripe unit. Check the status with
	raidctl -s while the parity is being written.
>Fix:

I have 2 patches. The first one is only cosmetic, it wraps a long
line, and unnests some conditions. The second changes the way the
percentage is calculated when there is a possibility of overflowing
(but only then, because it's less precise for small numbers and
could maybe even divide by 0). Note that  the patch changes copyback,
and reconstruction percentage calculations because I believe the
problem occurs there also. I should probably also mention that I
have only compile-tested this. :)

Index: rf_netbsdkintf.c
===================================================================
RCS file: /cvsroot/src/sys/dev/raidframe/rf_netbsdkintf.c,v
retrieving revision 1.224
diff -u -r1.224 rf_netbsdkintf.c
--- rf_netbsdkintf.c	30 Nov 2006 23:01:50 -0000	1.224
+++ rf_netbsdkintf.c	1 Sep 2007 13:26:23 -0000
@@ -1400,15 +1400,17 @@
 			*(int *) data = 100;
 			return(0);
 		}
-		if (raidPtr->status != rf_rs_reconstructing)
+		if (raidPtr->status != rf_rs_reconstructing) {
 			*(int *) data = 100;
-		else {
-			if (raidPtr->reconControl->numRUsTotal > 0) {
-				*(int *) data = (raidPtr->reconControl->numRUsComplete * 100 / raidPtr->reconControl->numRUsTotal);
-			} else {
-				*(int *) data = 0;
-			}
+			return 0;
 		}
+		if (raidPtr->reconControl->numRUsTotal <= 0) {
+			*(int *) data = 0;
+			return 0;
+		}
+		*(int *) data = 100 *
+			raidPtr->reconControl->numRUsComplete /
+			raidPtr->reconControl->numRUsTotal;
 		return (0);
 	case RAIDFRAME_CHECK_RECON_STATUS_EXT:
 		progressInfoPtr = (RF_ProgressInfo_t **) data;


--- rf_netbsdkintf.c.cosm	2007-08-31 23:32:07.000000000 +0200
+++ rf_netbsdkintf.c	2007-08-31 23:32:07.000000000 +0200
@@ -1408,9 +1408,14 @@
 			*(int *) data = 0;
 			return 0;
 		}
-		*(int *) data = 100 *
-			raidPtr->reconControl->numRUsComplete /
-			raidPtr->reconControl->numRUsTotal;
+		if (raidPtr->reconControl->numRUsComplete < (1 << 24)) {
+			*(int *) data = 100 *
+				raidPtr->reconControl->numRUsComplete /
+				raidPtr->reconControl->numRUsTotal;
+		} else {
+			*(int *) data = raidPtr->reconControl->numRUsComplete /
+				(raidPtr->reconControl->numRUsTotal / 100);
+		}
 		return (0);
 	case RAIDFRAME_CHECK_RECON_STATUS_EXT:
 		progressInfoPtr = (RF_ProgressInfo_t **) data;
@@ -1438,9 +1443,15 @@
 			return(0);
 		}
 		if (raidPtr->parity_rewrite_in_progress == 1) {
-			*(int *) data = 100 *
-				raidPtr->parity_rewrite_stripes_done /
-				raidPtr->Layout.numStripe;
+			if (raidPtr->parity_rewrite_stripes_done < (1 << 24)) {
+				*(int *) data = 100 *
+					raidPtr->parity_rewrite_stripes_done /
+					raidPtr->Layout.numStripe;
+			} else {
+				*(int *) data =
+					raidPtr->parity_rewrite_stripes_done /
+					(raidPtr->Layout.numStripe / 100);
+			}
 		} else {
 			*(int *) data = 100;
 		}
@@ -1470,8 +1481,14 @@
 			return(0);
 		}
 		if (raidPtr->copyback_in_progress == 1) {
-			*(int *) data = 100 * raidPtr->copyback_stripes_done /
-				raidPtr->Layout.numStripe;
+			if (raidPtr->copyback_stripes_done < (1 << 24)) {
+				*(int *) data = 100 *
+					raidPtr->copyback_stripes_done /
+					raidPtr->Layout.numStripe;
+			} else {
+				*(int *) data = raidPtr->copyback_stripes_done /
+					(raidPtr->Layout.numStripe / 100);
+			}
 		} else {
 			*(int *) data = 100;
 		}