Re: [PATCH v4 1/2] xfs: avoid LR buffer overrun due to crafted h_len

[Gao Xiang <hsiangkao@redhat.com>]

On Wed, Sep 23, 2020 at 09:35:40AM -0700, Darrick J. Wong wrote:
> On Thu, Sep 17, 2020 at 01:13:40PM +0800, Gao Xiang wrote:
> > Currently, crafted h_len has been blocked for the log
> > header of the tail block in commit a70f9fe52daa ("xfs:
> > detect and handle invalid iclog size set by mkfs").
> > 
> > However, each log record could still have crafted h_len
> > and cause log record buffer overrun. So let's check
> > h_len vs buffer size for each log record as well.
> > 
> > Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
> 
> /me squints real hard and thinks he understands what this patch does.
> 
> Tighten up xlog_valid_rec_header, and add a new callsite in the middle
> of xlog_do_recovery_pass instead of the insufficient length checking.
> Assuming that's right,
> 
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

Thanks for the review!

The main point is to check each individual LR h_len against LR buffer
size allocated first in xlog_do_recovery_pass() to avoid buffer
overflow triggered by CRC calc or follow-up steps (details in
https://lore.kernel.org/linux-xfs/20200902224726.GB4671@xiangao.remote.csb/ ).

no new callsite (just move the callsite after original workaround
code). Anyway, that is just a buffer overflow issue can be triggered
by corrupted log. Just a minor stuff but security guyes could also
keep eyes on such thing. I think that is right. :)

Thanks,
Gao Xiang

> 
> --D