On Fri, May 20, 2022 at 12:00:31PM -0700, Allison Henderson wrote:
> This patch implements a new set of log printing functions to print the
> ATTRI and ATTRD items and vectors in the log. These will be used during
> log dump and log recover operations.
>
> Though most attributes are strings, the attribute operations accept
> any binary payload, so we should not assume them printable. This was
> done intentionally in preparation for parent pointers. Until parent
> pointers get here, attributes have no discernible format. So the print
> routines are just a simple print or hex dump for now.
>
> Signed-off-by: Allison Henderson <allison.henderson@xxxxxxxxxx>
Oh, this reminds me how much I dislike logprint, having multiple,
very subtly different ways to print the same information in slightly
different formats.
Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
But this is a bug I needed to fix...
.....
> + if (src_f->alfi_value_len > 0) {
> + printf(_("\n"));
> + (*i)++;
> + head = (xlog_op_header_t *)*ptr;
> + xlog_print_op_header(head, *i, ptr);
> + error = xlog_print_trans_attri_value(ptr, be32_to_cpu(head->oh_len),
> + src_f->alfi_value_len);
> + }
So this passes the length of the region and the length of the
value. They are not the same, the value can be split across multiple
regions as the value is split across multiple log writes. so....
> +int
> +xlog_print_trans_attri_value(
> + char **ptr,
> + uint src_len,
> + int value_len)
> +{
> + int len = max(value_len, MAX_ATTR_VAL_PRINT);
> +
> + printf(_("ATTRI: value len:%u\n"), value_len);
> + print_or_dump(*ptr, len);
This dumps the value length from a buffer of src_len, overruns the
end of the buffer and Bad Things Happen. (i.e. logprint segv's)
This should be:
int len = min(value_len, src_len);
So that the dump doesn't overrun the region buffer....
I'll fix it directly in my stack, I think this is the only remaining
failure I'm seeing with my current libxfs 5.19 sync branch....
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
