On 5/15/14, 1:59 PM, Eric Sandeen wrote:
> 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 <sandeen@xxxxxxxxxx>
> ---
FWIW, I'm not sure this can actually be hit; see below.
>
> diff --git a/repair/prefetch.c b/repair/prefetch.c
> index 4595310..b6d4755 100644
> --- a/repair/prefetch.c
> +++ b/repair/prefetch.c
> @@ -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.
> - 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.
That's my take anyhoo.
-Eric
> + 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
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
>
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
