xfs: validate quota log items during log recovery

Arkadiusz has seen really strange crashes in xfs_qm_dqcheck that I can only explain by a log item being too smal to actually fit the xfs_dqblk_t we're dereferencing all over xfs_qm_dqcheck. So add graceful checks for NULL or too small quota items to the log recovery code. Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Eric Sandeen <sandeen@sandeen.net>

xfs: validate quota log items during log recovery
Arkadiusz has seen really strange crashes in xfs_qm_dqcheck that I can only explain by a log item being too smal to actually fit the xfs_dqblk_t we're dereferencing all over xfs_qm_dqcheck. So add graceful checks for NULL or too small quota items to the log recovery code. Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Eric Sandeen <sandeen@sandeen.net>
Christoph Hellwig · Christoph Hellwig
1 parent e1696834e8
Showing 1 changed file with 32 additions and 6 deletions Side-by-side Diff
fs/xfs/xfs_log_recover.c
@@ -1975,16 +1975,30 @@
 		error = 0;
 		if (buf_f->blf_flags &
 		   (XFS_BLI_UDQUOT_BUF|XFS_BLI_PDQUOT_BUF|XFS_BLI_GDQUOT_BUF)) {
+			if (item->ri_buf[i].i_addr == NULL) {
+				cmn_err(CE_ALERT,
+					"XFS: NULL dquot in %s.", __func__);
+				goto next;
+			}
+			if (item->ri_buf[i].i_len < sizeof(xfs_dqblk_t)) {
+				cmn_err(CE_ALERT,
+					"XFS: dquot too small (%d) in %s.",
+					item->ri_buf[i].i_len, __func__);
+				goto next;
+			}
 			error = xfs_qm_dqcheck((xfs_disk_dquot_t *)
 					       item->ri_buf[i].i_addr,
 					       -1, 0, XFS_QMOPT_DOWARN,
 					       "dquot_buf_recover");
+			if (error)
+				goto next;
 		}
-		if (!error)
-			memcpy(xfs_buf_offset(bp,
-				(uint)bit << XFS_BLI_SHIFT),	/* dest */
-				item->ri_buf[i].i_addr,		/* source */
-				nbits<<XFS_BLI_SHIFT);		/* length */
+
+		memcpy(xfs_buf_offset(bp,
+			(uint)bit << XFS_BLI_SHIFT),	/* dest */
+			item->ri_buf[i].i_addr,		/* source */
+			nbits<<XFS_BLI_SHIFT);		/* length */
+ next:
 		i++;
 		bit += nbits;
 	}
@@ -2615,7 +2629,19 @@
 		return (0);
  
 	recddq = (xfs_disk_dquot_t *)item->ri_buf[1].i_addr;
-	ASSERT(recddq);
+
+	if (item->ri_buf[1].i_addr == NULL) {
+		cmn_err(CE_ALERT,
+			"XFS: NULL dquot in %s.", __func__);
+		return XFS_ERROR(EIO);
+	}
+	if (item->ri_buf[1].i_len < sizeof(xfs_dqblk_t)) {
+		cmn_err(CE_ALERT,
+			"XFS: dquot too small (%d) in %s.",
+			item->ri_buf[1].i_len, __func__);
+		return XFS_ERROR(EIO);
+	}
+
 	/*
 	 * This type of quotas was turned off, so ignore this record.
 	 */
...	...	@@ -1975,16 +1975,30 @@
1975	1975	error = 0;
1976	1976	if (buf_f->blf_flags &
1977	1977	(XFS_BLI_UDQUOT_BUF\|XFS_BLI_PDQUOT_BUF\|XFS_BLI_GDQUOT_BUF)) {
	1978	+ if (item->ri_buf[i].i_addr == NULL) {
	1979	+ cmn_err(CE_ALERT,
	1980	+ "XFS: NULL dquot in %s.", __func__);
	1981	+ goto next;
	1982	+ }
	1983	+ if (item->ri_buf[i].i_len < sizeof(xfs_dqblk_t)) {
	1984	+ cmn_err(CE_ALERT,
	1985	+ "XFS: dquot too small (%d) in %s.",
	1986	+ item->ri_buf[i].i_len, __func__);
	1987	+ goto next;
	1988	+ }
1978	1989	error = xfs_qm_dqcheck((xfs_disk_dquot_t *)
1979	1990	item->ri_buf[i].i_addr,
1980	1991	-1, 0, XFS_QMOPT_DOWARN,
1981	1992	"dquot_buf_recover");
	1993	+ if (error)
	1994	+ goto next;
1982	1995	}
1983		- if (!error)
1984		- memcpy(xfs_buf_offset(bp,
1985		- (uint)bit << XFS_BLI_SHIFT), /* dest */
1986		- item->ri_buf[i].i_addr, /* source */
1987		- nbits<<XFS_BLI_SHIFT); /* length */
	1996	+
	1997	+ memcpy(xfs_buf_offset(bp,
	1998	+ (uint)bit << XFS_BLI_SHIFT), /* dest */
	1999	+ item->ri_buf[i].i_addr, /* source */
	2000	+ nbits<<XFS_BLI_SHIFT); /* length */
	2001	+ next:
1988	2002	i++;
1989	2003	bit += nbits;
1990	2004	}
...	...	@@ -2615,7 +2629,19 @@
2615	2629	return (0);
2616	2630
2617	2631	recddq = (xfs_disk_dquot_t *)item->ri_buf[1].i_addr;
2618		- ASSERT(recddq);
	2632	+
	2633	+ if (item->ri_buf[1].i_addr == NULL) {
	2634	+ cmn_err(CE_ALERT,
	2635	+ "XFS: NULL dquot in %s.", __func__);
	2636	+ return XFS_ERROR(EIO);
	2637	+ }
	2638	+ if (item->ri_buf[1].i_len < sizeof(xfs_dqblk_t)) {
	2639	+ cmn_err(CE_ALERT,
	2640	+ "XFS: dquot too small (%d) in %s.",
	2641	+ item->ri_buf[1].i_len, __func__);
	2642	+ return XFS_ERROR(EIO);
	2643	+ }
	2644	+
2619	2645	/*
2620	2646	* This type of quotas was turned off, so ignore this record.
2621	2647	*/