| Message ID | 7481b076-dbc6-ddaa-4e3f-9a1bc2b94e26@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Thu, Feb 02, 2017 at 09:54:56AM -0600, Eric Sandeen wrote: > Another in the ongoing saga of attribute leaves with zero > entries; in this case, if we try to metadump an inode with > a zero-entries attribute leaf, the zeroing code will go off > the rails and segfault at: > > memset(&entries[nentries], 0, > first_name - (char *)&entries[nentries]); > > because first_name is null, and we try to memset a large > (negative) number. So what /does/ get metadumped in the !nentries case? Is it the block header and whatever else happens to be on disk? I guess that's useful for diagnostic purposes, since we don't know what's really "stale" when the block is obviously bad. (I was eyeing zero_stale_data but then convinced myself it was fine to just exit.) Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > --- > > diff --git a/db/metadump.c b/db/metadump.c > index 38519f1..66952f6 100644 > --- a/db/metadump.c > +++ b/db/metadump.c > @@ -1654,7 +1654,8 @@ process_attr_block( > xfs_attr3_leaf_hdr_from_disk(mp->m_attr_geo, &hdr, leaf); > > nentries = hdr.count; > - if (nentries * sizeof(xfs_attr_leaf_entry_t) + > + if (nentries == 0 || > + nentries * sizeof(xfs_attr_leaf_entry_t) + > xfs_attr3_leaf_hdr_size(leaf) > > XFS_ATTR3_RMT_BUF_SPACE(mp, bs)) { > if (show_warnings) > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2/15/17 9:31 PM, Darrick J. Wong wrote: > On Thu, Feb 02, 2017 at 09:54:56AM -0600, Eric Sandeen wrote: >> Another in the ongoing saga of attribute leaves with zero >> entries; in this case, if we try to metadump an inode with >> a zero-entries attribute leaf, the zeroing code will go off >> the rails and segfault at: >> >> memset(&entries[nentries], 0, >> first_name - (char *)&entries[nentries]); >> >> because first_name is null, and we try to memset a large >> (negative) number. > > So what /does/ get metadumped in the !nentries case? Um, nothing, because it segfaults? Oh you mean AFTER the patch. :P > Is it the block > header and whatever else happens to be on disk? I guess that's useful > for diagnostic purposes, since we don't know what's really "stale" when > the block is obviously bad. (I was eyeing zero_stale_data but then > convinced myself it was fine to just exit.) Hm, probably worth finding out what reall does happen for this block, but i'll go ahead & commit this for now just to avoid the segfault. > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> thanks, -Eric > --D > >> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com> >> --- >> >> diff --git a/db/metadump.c b/db/metadump.c >> index 38519f1..66952f6 100644 >> --- a/db/metadump.c >> +++ b/db/metadump.c >> @@ -1654,7 +1654,8 @@ process_attr_block( >> xfs_attr3_leaf_hdr_from_disk(mp->m_attr_geo, &hdr, leaf); >> >> nentries = hdr.count; >> - if (nentries * sizeof(xfs_attr_leaf_entry_t) + >> + if (nentries == 0 || >> + nentries * sizeof(xfs_attr_leaf_entry_t) + >> xfs_attr3_leaf_hdr_size(leaf) > >> XFS_ATTR3_RMT_BUF_SPACE(mp, bs)) { >> if (show_warnings) >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/db/metadump.c b/db/metadump.c index 38519f1..66952f6 100644 --- a/db/metadump.c +++ b/db/metadump.c @@ -1654,7 +1654,8 @@ process_attr_block( xfs_attr3_leaf_hdr_from_disk(mp->m_attr_geo, &hdr, leaf); nentries = hdr.count; - if (nentries * sizeof(xfs_attr_leaf_entry_t) + + if (nentries == 0 || + nentries * sizeof(xfs_attr_leaf_entry_t) + xfs_attr3_leaf_hdr_size(leaf) > XFS_ATTR3_RMT_BUF_SPACE(mp, bs)) { if (show_warnings)
Another in the ongoing saga of attribute leaves with zero entries; in this case, if we try to metadump an inode with a zero-entries attribute leaf, the zeroing code will go off the rails and segfault at: memset(&entries[nentries], 0, first_name - (char *)&entries[nentries]); because first_name is null, and we try to memset a large (negative) number. Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html