On Mon, May 27, 2024 at 07:22:49 PM +1000, Dave Chinner wrote: > On Mon, May 27, 2024 at 10:05:17AM +0530, Chandan Babu R wrote: >> >> [CC-ing linux-xfs mailing list] >> >> On Sat, May 25, 2024 at 12:41:19 AM +0800, lei lu wrote: >> > Add a check to make sure xfs_dir2_data_unused and xfs_dir2_data_entry >> > don't stray beyond valid memory region. > > How was this found? What symptoms did it have? i.e. How do we know > if we've tripped over the same problem on an older LTS/distro kernel > and need to backport it? > >> > Tested-by: lei lu <llfamsec@xxxxxxxxx> >> > Signed-off-by: lei lu <llfamsec@xxxxxxxxx> >> >> Also adding the missing RVB from Darrick, >> >> Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx> > > That's not really normal process - adding third party tags like this > are kinda frowned upon because there's no actual public record of > Darrick saying this. Ok. The patch was posted on security@xxxxxxxxxx with me on CC. Hence, I had decided to forward the patch to linux-xfs for any reviews from the wider audience. > > i.e. patches send privately should really be reposted to the public > list by the submitter and everyone then adds their rvb/acks, etc on > list themselves. > Sorry, I didn't know about the last part i.e. rvbs need to be added once again after reposting the patch. >> >> > --- >> > fs/xfs/libxfs/xfs_dir2_data.c | 7 +++++++ >> > 1 file changed, 7 insertions(+) >> > >> > diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c >> > index dbcf58979a59..08c18e0c1baa 100644 >> > --- a/fs/xfs/libxfs/xfs_dir2_data.c >> > +++ b/fs/xfs/libxfs/xfs_dir2_data.c >> > @@ -178,6 +178,9 @@ __xfs_dir3_data_check( >> > struct xfs_dir2_data_unused *dup = bp->b_addr + offset; >> > struct xfs_dir2_data_entry *dep = bp->b_addr + offset; >> > >> > + if (offset + sizeof(*dup) > end) >> > + return __this_address; >> > + >> > /* >> > * If it's unused, look for the space in the bestfree table. >> > * If we find it, account for that, else make sure it >> > @@ -210,6 +213,10 @@ __xfs_dir3_data_check( >> > lastfree = 1; >> > continue; >> > } >> > + >> > + if (offset + sizeof(*dep) > end) >> > + return __this_address; >> > + >> > /* >> > * It's a real entry. Validate the fields. >> > * If this is a block directory then make sure it's > > Nothing wrong with the code change, though. > -- Chandan
