| Message ID | 20190829143341.13920-1-connor.kuehl@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [X/B,SRU,CVE-2018-20976] xfs: clear sb->s_fs_info on mount failure | expand |
On 8/29/19 4:33 PM, Connor Kuehl wrote: > From: Dave Chinner <dchinner@redhat.com> > > CVE-2018-20976 > > We recently had an oops reported on a 4.14 kernel in > xfs_reclaim_inodes_count() where sb->s_fs_info pointed to garbage > and so the m_perag_tree lookup walked into lala land. > > Essentially, the machine was under memory pressure when the mount > was being run, xfs_fs_fill_super() failed after allocating the > xfs_mount and attaching it to sb->s_fs_info. It then cleaned up and > freed the xfs_mount, but the sb->s_fs_info field still pointed to > the freed memory. Hence when the superblock shrinker then ran > it fell off the bad pointer. > > With the superblock shrinker problem fixed at teh VFS level, this > stale s_fs_info pointer is still a problem - we use it > unconditionally in ->put_super when the superblock is being torn > down, and hence we can still trip over it after a ->fill_super > call failure. Hence we need to clear s_fs_info if > xfs-fs_fill_super() fails, and we need to check if it's valid in > the places it can potentially be dereferenced after a ->fill_super > failure. > > Signed-Off-By: Dave Chinner <dchinner@redhat.com> > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > (cherry picked from commit c9fbd7bbc23dbdd73364be4d045e5d3612cf6e82) > Signed-off-by: Connor Kuehl <connor.kuehl@canonical.com> Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > --- > fs/xfs/xfs_super.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > index ef64a1e1a66a..ff3f5812c0fd 100644 > --- a/fs/xfs/xfs_super.c > +++ b/fs/xfs/xfs_super.c > @@ -1572,6 +1572,7 @@ xfs_fs_fill_super( > out_close_devices: > xfs_close_devices(mp); > out_free_fsname: > + sb->s_fs_info = NULL; > xfs_free_fsname(mp); > kfree(mp); > out: > @@ -1589,6 +1590,10 @@ xfs_fs_put_super( > { > struct xfs_mount *mp = XFS_M(sb); > > + /* if ->fill_super failed, we have no mount to tear down */ > + if (!sb->s_fs_info) > + return; > + > xfs_notice(mp, "Unmounting Filesystem"); > xfs_filestream_unmount(mp); > xfs_unmountfs(mp); > @@ -1598,6 +1603,8 @@ xfs_fs_put_super( > xfs_destroy_percpu_counters(mp); > xfs_destroy_mount_workqueues(mp); > xfs_close_devices(mp); > + > + sb->s_fs_info = NULL; > xfs_free_fsname(mp); > kfree(mp); > } > @@ -1617,6 +1624,9 @@ xfs_fs_nr_cached_objects( > struct super_block *sb, > struct shrink_control *sc) > { > + /* Paranoia: catch incorrect calls during mount setup or teardown */ > + if (WARN_ON_ONCE(!sb->s_fs_info)) > + return 0; > return xfs_reclaim_inodes_count(XFS_M(sb)); > } > >
On 2019-08-29 07:33:41 , Connor Kuehl wrote: > From: Dave Chinner <dchinner@redhat.com> > > CVE-2018-20976 > > We recently had an oops reported on a 4.14 kernel in > xfs_reclaim_inodes_count() where sb->s_fs_info pointed to garbage > and so the m_perag_tree lookup walked into lala land. > > Essentially, the machine was under memory pressure when the mount > was being run, xfs_fs_fill_super() failed after allocating the > xfs_mount and attaching it to sb->s_fs_info. It then cleaned up and > freed the xfs_mount, but the sb->s_fs_info field still pointed to > the freed memory. Hence when the superblock shrinker then ran > it fell off the bad pointer. > > With the superblock shrinker problem fixed at teh VFS level, this > stale s_fs_info pointer is still a problem - we use it > unconditionally in ->put_super when the superblock is being torn > down, and hence we can still trip over it after a ->fill_super > call failure. Hence we need to clear s_fs_info if > xfs-fs_fill_super() fails, and we need to check if it's valid in > the places it can potentially be dereferenced after a ->fill_super > failure. > > Signed-Off-By: Dave Chinner <dchinner@redhat.com> > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > (cherry picked from commit c9fbd7bbc23dbdd73364be4d045e5d3612cf6e82) > Signed-off-by: Connor Kuehl <connor.kuehl@canonical.com> > --- > fs/xfs/xfs_super.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > index ef64a1e1a66a..ff3f5812c0fd 100644 > --- a/fs/xfs/xfs_super.c > +++ b/fs/xfs/xfs_super.c > @@ -1572,6 +1572,7 @@ xfs_fs_fill_super( > out_close_devices: > xfs_close_devices(mp); > out_free_fsname: > + sb->s_fs_info = NULL; > xfs_free_fsname(mp); > kfree(mp); > out: > @@ -1589,6 +1590,10 @@ xfs_fs_put_super( > { > struct xfs_mount *mp = XFS_M(sb); > > + /* if ->fill_super failed, we have no mount to tear down */ > + if (!sb->s_fs_info) > + return; > + > xfs_notice(mp, "Unmounting Filesystem"); > xfs_filestream_unmount(mp); > xfs_unmountfs(mp); > @@ -1598,6 +1603,8 @@ xfs_fs_put_super( > xfs_destroy_percpu_counters(mp); > xfs_destroy_mount_workqueues(mp); > xfs_close_devices(mp); > + > + sb->s_fs_info = NULL; > xfs_free_fsname(mp); > kfree(mp); > } > @@ -1617,6 +1624,9 @@ xfs_fs_nr_cached_objects( > struct super_block *sb, > struct shrink_control *sc) > { > + /* Paranoia: catch incorrect calls during mount setup or teardown */ > + if (WARN_ON_ONCE(!sb->s_fs_info)) > + return 0; > return xfs_reclaim_inodes_count(XFS_M(sb)); > } > > -- > 2.17.1 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On 2019-08-29 07:33:41 , Connor Kuehl wrote: > From: Dave Chinner <dchinner@redhat.com> > > CVE-2018-20976 > > We recently had an oops reported on a 4.14 kernel in > xfs_reclaim_inodes_count() where sb->s_fs_info pointed to garbage > and so the m_perag_tree lookup walked into lala land. > > Essentially, the machine was under memory pressure when the mount > was being run, xfs_fs_fill_super() failed after allocating the > xfs_mount and attaching it to sb->s_fs_info. It then cleaned up and > freed the xfs_mount, but the sb->s_fs_info field still pointed to > the freed memory. Hence when the superblock shrinker then ran > it fell off the bad pointer. > > With the superblock shrinker problem fixed at teh VFS level, this > stale s_fs_info pointer is still a problem - we use it > unconditionally in ->put_super when the superblock is being torn > down, and hence we can still trip over it after a ->fill_super > call failure. Hence we need to clear s_fs_info if > xfs-fs_fill_super() fails, and we need to check if it's valid in > the places it can potentially be dereferenced after a ->fill_super > failure. > > Signed-Off-By: Dave Chinner <dchinner@redhat.com> > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > (cherry picked from commit c9fbd7bbc23dbdd73364be4d045e5d3612cf6e82) > Signed-off-by: Connor Kuehl <connor.kuehl@canonical.com> > --- > fs/xfs/xfs_super.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > index ef64a1e1a66a..ff3f5812c0fd 100644 > --- a/fs/xfs/xfs_super.c > +++ b/fs/xfs/xfs_super.c > @@ -1572,6 +1572,7 @@ xfs_fs_fill_super( > out_close_devices: > xfs_close_devices(mp); > out_free_fsname: > + sb->s_fs_info = NULL; > xfs_free_fsname(mp); > kfree(mp); > out: > @@ -1589,6 +1590,10 @@ xfs_fs_put_super( > { > struct xfs_mount *mp = XFS_M(sb); > > + /* if ->fill_super failed, we have no mount to tear down */ > + if (!sb->s_fs_info) > + return; > + > xfs_notice(mp, "Unmounting Filesystem"); > xfs_filestream_unmount(mp); > xfs_unmountfs(mp); > @@ -1598,6 +1603,8 @@ xfs_fs_put_super( > xfs_destroy_percpu_counters(mp); > xfs_destroy_mount_workqueues(mp); > xfs_close_devices(mp); > + > + sb->s_fs_info = NULL; > xfs_free_fsname(mp); > kfree(mp); > } > @@ -1617,6 +1624,9 @@ xfs_fs_nr_cached_objects( > struct super_block *sb, > struct shrink_control *sc) > { > + /* Paranoia: catch incorrect calls during mount setup or teardown */ > + if (WARN_ON_ONCE(!sb->s_fs_info)) > + return 0; > return xfs_reclaim_inodes_count(XFS_M(sb)); > } > > -- > 2.17.1 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c index ef64a1e1a66a..ff3f5812c0fd 100644 --- a/fs/xfs/xfs_super.c +++ b/fs/xfs/xfs_super.c @@ -1572,6 +1572,7 @@ xfs_fs_fill_super( out_close_devices: xfs_close_devices(mp); out_free_fsname: + sb->s_fs_info = NULL; xfs_free_fsname(mp); kfree(mp); out: @@ -1589,6 +1590,10 @@ xfs_fs_put_super( { struct xfs_mount *mp = XFS_M(sb); + /* if ->fill_super failed, we have no mount to tear down */ + if (!sb->s_fs_info) + return; + xfs_notice(mp, "Unmounting Filesystem"); xfs_filestream_unmount(mp); xfs_unmountfs(mp); @@ -1598,6 +1603,8 @@ xfs_fs_put_super( xfs_destroy_percpu_counters(mp); xfs_destroy_mount_workqueues(mp); xfs_close_devices(mp); + + sb->s_fs_info = NULL; xfs_free_fsname(mp); kfree(mp); } @@ -1617,6 +1624,9 @@ xfs_fs_nr_cached_objects( struct super_block *sb, struct shrink_control *sc) { + /* Paranoia: catch incorrect calls during mount setup or teardown */ + if (WARN_ON_ONCE(!sb->s_fs_info)) + return 0; return xfs_reclaim_inodes_count(XFS_M(sb)); }