| Message ID | 20200825202853.GE6096@magnolia (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: initialize the shortform attr header padding entry | expand |
On 8/25/20 3:28 PM, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Don't leak kernel memory contents into the shortform attr fork. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> I noticed this too, thanks. thought I wonder if a) others lurk and b) if we should just be memsetting if_data to zero to avoid the need to carefully initialize all of everything always? Anyway it fixes the problem we noticed so Reviewed-by: Eric Sandeen <sandeen@redhat.com> > --- > fs/xfs/libxfs/xfs_attr_leaf.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c > index 8623c815164a..e1a3d225a77d 100644 > --- a/fs/xfs/libxfs/xfs_attr_leaf.c > +++ b/fs/xfs/libxfs/xfs_attr_leaf.c > @@ -656,6 +656,7 @@ xfs_attr_shortform_create( > hdr = (xfs_attr_sf_hdr_t *)ifp->if_u1.if_data; > hdr->count = 0; > hdr->totsize = cpu_to_be16(sizeof(*hdr)); > + hdr->padding = 0; > xfs_trans_log_inode(args->trans, dp, XFS_ILOG_CORE | XFS_ILOG_ADATA); > } > >
On Tue, Aug 25, 2020 at 01:28:53PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Don't leak kernel memory contents into the shortform attr fork. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > fs/xfs/libxfs/xfs_attr_leaf.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c > index 8623c815164a..e1a3d225a77d 100644 > --- a/fs/xfs/libxfs/xfs_attr_leaf.c > +++ b/fs/xfs/libxfs/xfs_attr_leaf.c > @@ -656,6 +656,7 @@ xfs_attr_shortform_create( > hdr = (xfs_attr_sf_hdr_t *)ifp->if_u1.if_data; > hdr->count = 0; > hdr->totsize = cpu_to_be16(sizeof(*hdr)); > + hdr->padding = 0; memset(hdr, 0, sizeof(*hdr)), please. Cheers, Dave.
On Tue, Aug 25, 2020 at 03:48:13PM -0500, Eric Sandeen wrote: > On 8/25/20 3:28 PM, Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > Don't leak kernel memory contents into the shortform attr fork. > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > I noticed this too, thanks. > > thought I wonder if > > a) others lurk and > b) if we should just be memsetting if_data to zero to avoid the need > to carefully initialize all of everything always? > > Anyway it fixes the problem we noticed so Yes, I think fully clearing the thing would be better. We can do a memset for now, but splitting the non-existent data case out of xfs_idata_realloc into a new helper that does kzmalloc sounds best long term.
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c index 8623c815164a..e1a3d225a77d 100644 --- a/fs/xfs/libxfs/xfs_attr_leaf.c +++ b/fs/xfs/libxfs/xfs_attr_leaf.c @@ -656,6 +656,7 @@ xfs_attr_shortform_create( hdr = (xfs_attr_sf_hdr_t *)ifp->if_u1.if_data; hdr->count = 0; hdr->totsize = cpu_to_be16(sizeof(*hdr)); + hdr->padding = 0; xfs_trans_log_inode(args->trans, dp, XFS_ILOG_CORE | XFS_ILOG_ADATA); }