diff mbox series

[1/7] xfs: fix uninitialized variables in xrep_calc_ag_resblks

Message ID 161472412192.3421582.514508996639938538.stgit@magnolia (mailing list archive)
State Accepted
Headers show
Series xfs: small fixes and cleanups | expand

Commit Message

Darrick J. Wong March 2, 2021, 10:28 p.m. UTC 
From: Darrick J. Wong <djwong@kernel.org>

If we can't read the AGF header, we never actually set a value for
freelen and usedlen.  These two variables are used to make the worst
case estimate of btree size, so it's safe to set them to the AG size as
a fallback.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/scrub/repair.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Christoph Hellwig March 5, 2021, 8:23 a.m. UTC | #1 
On Tue, Mar 02, 2021 at 02:28:42PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> If we can't read the AGF header, we never actually set a value for
> freelen and usedlen.  These two variables are used to make the worst
> case estimate of btree size, so it's safe to set them to the AG size as
> a fallback.

Do we actually want to continue with the rest of the funtion at all
in this case?
Darrick J. Wong March 5, 2021, 5:49 p.m. UTC | #2 
On Fri, Mar 05, 2021 at 08:23:00AM +0000, Christoph Hellwig wrote:
> On Tue, Mar 02, 2021 at 02:28:42PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > If we can't read the AGF header, we never actually set a value for
> > freelen and usedlen.  These two variables are used to make the worst
> > case estimate of btree size, so it's safe to set them to the AG size as
> > a fallback.
> 
> Do we actually want to continue with the rest of the funtion at all
> in this case?

We do, because this function computes the amount of block reservation to
feed to xfs_trans_alloc when userspace said it wants us to try to repair
something AG-related.

Although... I suppose we don't really need a block reservation to repair
superblocks and AG headers, so we could special-case those four scrub
types to return 0.

(OTOH this is all mostly academic because repair requires rmapbt, which
means that the fs won't even mount with a busted AGF...)

--D
Christoph Hellwig March 6, 2021, 7:15 a.m. UTC | #3 
On Tue, Mar 02, 2021 at 02:28:42PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> If we can't read the AGF header, we never actually set a value for
> freelen and usedlen.  These two variables are used to make the worst
> case estimate of btree size, so it's safe to set them to the AG size as
> a fallback.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>
diff mbox series

Patch

diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index 25e86c71e7b9..61bc43418a2a 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -207,7 +207,11 @@  xrep_calc_ag_resblks(
 
 	/* Now grab the block counters from the AGF. */
 	error = xfs_alloc_read_agf(mp, NULL, sm->sm_agno, 0, &bp);
-	if (!error) {
+	if (error) {
+		aglen = xfs_ag_block_count(mp, sm->sm_agno);
+		freelen = aglen;
+		usedlen = aglen;
+	} else {
 		struct xfs_agf	*agf = bp->b_addr;
 
 		aglen = be32_to_cpu(agf->agf_length);