xfs_repair: don't let bplist index go negative in prefetch

After:

bbd3275 repair: don't unlock prefetch tree to read discontig buffers

Coverity spotted that it's possible for us to arrive at the loop below with num == 1, and then we decrement it to 0, and try to index bplist[num-1].

I think this was possible before the change, i.e. it's probably not a regression.

Fix this by not trying to shrink the window unless we have more than one buffer in the array.

Signed-off-by Eric Sandeen <[email protected]>

---

repair/prefetch.c +1 -0

+		while (num > 1 && last_off - first_off > pf_max_bytes) {
 			num--;
 			last_off = LIBXFS_BBTOOFF64(XFS_BUF_ADDR(bplist[num-1])) +
 				XFS_BUF_SIZE(bplist[num-1]);

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Quoted 8 lines · Eric Sandeen

After:

bbd3275 repair: don't unlock prefetch tree to read discontig buffers

Coverity spotted that it's possible for us to arrive at the loop below with num == 1, and then we decrement it to 0, and try to index bplist[num-1].

Quoted 8 lines

I think this was possible before the change, i.e. it's probably not a regression.

Fix this by not trying to shrink the window unless we have more than one buffer in the array.

Signed-off-by Eric Sandeen <[email protected]>

---

FWIW, I'm not sure this can actually be hit; see below.

repair/prefetch.c pf_batch_read( · line 507

Quoted 9 lines

repair/prefetch.c +0 -0

index 4595310..b6d4755 100644
@@ -505,7 +505,7 @@ pf_batch_read(
 		first_off = LIBXFS_BBTOOFF64(XFS_BUF_ADDR(bplist[0]));
 		last_off = LIBXFS_BBTOOFF64(XFS_BUF_ADDR(bplist[num-1])) +
 			XFS_BUF_SIZE(bplist[num-1]);

Indexing bplist[num-1] after we do num-- is only a problem if num==1.

If num==1, then last_off - first_off == XFS_BUF_SIZE(bplist[0]) above.

repair/prefetch.c pf_batch_read( · line 508

Quoted 1 line

repair/prefetch.c +0 -1

-		while (last_off - first_off > pf_max_bytes) {

so we can only go here if XFS_BUF_SIZE(bplist[0] > pf_max_bytes, and

        pf_max_bytes = sysconf(_SC_PAGE_SIZE) << 7;

for a 4k page that's 512k.

So unless XFS_BUF_SIZE(bplist[0]) > 512k, we won't run into trouble.

And I don't ... think that can happen, right? So it's probably impossible to hit; worth being defensive, but not critical.