On Fri, Jun 19, 2015 at 10:21:36AM +0100, Mike Grant wrote:
> longform_dir2_entry_check in phase6.c has a calloc'ed array of xfs_buf
> pointers (bplist). It reallocs this array if there turns out to be more
> blocks than expected. However, realloc doesn't zero the new memory like
> calloc. In unusual circumstances*, things may then blow up later due to
> random data populating the new part of the array.
>
> This patch zeros the new part of the array.
>
> * (bit speculative) as dir_read_buf zeros the element it's looking at, I
> think this can only happen if the realloc adds several members and one
> of the first is corrupt. In my case, the realloc went from 35 to 37
> members, meaning db must have been 36 without being 35. A read error
> then caused it to goto out_fix. The crash then occurred in the
> libxfs_putbuf when looping through the bplist structure, checking it for
> NULL pointers (and presumably tripping over the non-zeroed data at
> position 35?)
>
> Signed-off-by: Mike Grant <mggr@xxxxxxxxx>
> ---
> repair/phase6.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/repair/phase6.c b/repair/phase6.c
> index 0d66f9c..4dd7551 100644
> --- a/repair/phase6.c
> +++ b/repair/phase6.c
> @@ -2326,6 +2326,7 @@ longform_dir2_entry_check(xfs_mount_t *mp,
> db = xfs_dir2_da_to_db(mp, da_bno);
> if (db >= num_bps) {
> /* more data blocks than expected */
> + int num_bps_prev = num_bps;
> num_bps = db + 1;
> bplist = realloc(bplist, num_bps *
> sizeof(struct xfs_buf*));
> @@ -2334,6 +2335,10 @@ longform_dir2_entry_check(xfs_mount_t *mp,
> _("realloc failed in %s (%zu bytes)\n"),
> __func__,
> num_bps * sizeof(struct xfs_buf*));
> + /* clear new memory as previous bplist was calloc'ed */
> + memset((void *) bplist + num_bps_prev
> + * sizeof(struct xfs_buf*), 0, (num_bps -
> + num_bps_prev) * sizeof(struct xfs_buf*));
Similar issue here with how the lines are split. In looking at it, I'm
not even sure how I would split this one up cleanly tbh.. ;) Maybe we
should just create another local variable for the bp size and clean up
the whole hunk. E.g.:
int bpsz = sizeof(struct xfs_buf *);
...
memset((void *)bplist + num_bps_prev * bpsz, 0,
(num_bps - num_bps_prev) * bpsz);
In fact, we could include bpsz in the first patch.
Brian
> }
>
> if (isblock)
> --
> 2.1.0
>
> (apologies for the following junk forcibly appended by my company)
>
>
> Please visit our new website at www.pml.ac.uk and follow us on Twitter
> @PlymouthMarine
>
> Winner of the Environment & Conservation category, the Charity Awards 2014.
>
> Plymouth Marine Laboratory (PML) is a company limited by guarantee registered
> in England & Wales, company number 4178503. Registered Charity No. 1091222.
> Registered Office: Prospect Place, The Hoe, Plymouth PL1 3DH, UK.
>
> This message is private and confidential. If you have received this message
> in error, please notify the sender and remove it from your system. You are
> reminded that e-mail communications are not secure and may contain viruses;
> PML accepts no liability for any loss or damage which may be caused by
> viruses.
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
|