cluster-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
* [Cluster-devel] [GFS2 PATCH 0/3] gfs2: Fix 'dont sleep' GPF from vfs
@ 2020-10-27 14:52 Bob Peterson
  2020-10-27 14:52 ` [Cluster-devel] [GFS2 PATCH 1/3] gfs2: Free rd_bits later in gfs2_clear-rgrpd to fix use-after-free Bob Peterson
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Bob Peterson @ 2020-10-27 14:52 UTC (permalink / raw)
  To: cluster-devel.redhat.com

This series of patches fixes problems discovered while testing patch
34244d711dea568f4a42c5b0d6b3d620f8cb6971 "Don't sleep during glock hash walk"
with various combinations of these xfstests under low memory conditions:

env SCRATCH_DEV="/dev/sda" SCRATCH_MNT="/mnt/scratch" TEST_DEV="/dev/sdb" TEST_DIR="/mnt/gfs2" ./check generic/076 generic/081 generic/224

and:

env SCRATCH_DEV="/dev/sda" SCRATCH_MNT="/mnt/scratch" TEST_DEV="/dev/sdb" TEST_DIR="/mnt/gfs2" ./check generic/0{34,35,38}

The bug led to "general protection fault, probably for non-canonical address"
errors called from either move_to_new_page() or shrink_page_list().
It was always a reference to an invalid address space pointer, page->mapping.

Bob Peterson (3):
  gfs2: Free rd_bits later in gfs2_clear-rgrpd to fix use-after-free
  gfs2: Add missing truncate_inode_pages_final for sd_aspace
  gfs2: Use filemap_fdatawrite_range for gfs2_meta_sync

 fs/gfs2/lops.c  | 14 +++++++++++---
 fs/gfs2/rgrp.c  |  2 +-
 fs/gfs2/super.c |  1 +
 3 files changed, 13 insertions(+), 4 deletions(-)

-- 
2.26.2



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Cluster-devel] [GFS2 PATCH 1/3] gfs2: Free rd_bits later in gfs2_clear-rgrpd to fix use-after-free
  2020-10-27 14:52 [Cluster-devel] [GFS2 PATCH 0/3] gfs2: Fix 'dont sleep' GPF from vfs Bob Peterson
@ 2020-10-27 14:52 ` Bob Peterson
  2020-10-27 14:52 ` [Cluster-devel] [GFS2 PATCH 2/3] gfs2: Add missing truncate_inode_pages_final for sd_aspace Bob Peterson
  2020-10-27 14:52 ` [Cluster-devel] [GFS2 PATCH 3/3] gfs2: Use filemap_fdatawrite_range for gfs2_meta_sync Bob Peterson
  2 siblings, 0 replies; 5+ messages in thread
From: Bob Peterson @ 2020-10-27 14:52 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Before this patch, function gfs2_clear_rgrpd ran through all the rgrps doing
cleanup, but it called kfree(rgd->rd_bits) then return_all_reservations().
But return_all_reservations calls __rs_deltree, which calls rbm_bi, and
rbm_bi references: rbm->rgd->rd_bits + rbm->bii. This is use-after-free.

This patch moves the call to kfree after the call to return_all_reservations
so it can reference it before it is freed.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/rgrp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index ee491bb9c1cc..eb1b29734b7f 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -719,9 +719,9 @@ void gfs2_clear_rgrpd(struct gfs2_sbd *sdp)
 		}
 
 		gfs2_free_clones(rgd);
+		return_all_reservations(rgd);
 		kfree(rgd->rd_bits);
 		rgd->rd_bits = NULL;
-		return_all_reservations(rgd);
 		kmem_cache_free(gfs2_rgrpd_cachep, rgd);
 	}
 }
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [Cluster-devel] [GFS2 PATCH 2/3] gfs2: Add missing truncate_inode_pages_final for sd_aspace
  2020-10-27 14:52 [Cluster-devel] [GFS2 PATCH 0/3] gfs2: Fix 'dont sleep' GPF from vfs Bob Peterson
  2020-10-27 14:52 ` [Cluster-devel] [GFS2 PATCH 1/3] gfs2: Free rd_bits later in gfs2_clear-rgrpd to fix use-after-free Bob Peterson
@ 2020-10-27 14:52 ` Bob Peterson
  2020-10-27 14:52 ` [Cluster-devel] [GFS2 PATCH 3/3] gfs2: Use filemap_fdatawrite_range for gfs2_meta_sync Bob Peterson
  2 siblings, 0 replies; 5+ messages in thread
From: Bob Peterson @ 2020-10-27 14:52 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Gfs2 creates an address space for its rgrps called sd_aspace, but it never
called truncate_inode_pages_final on it. This confused vfs greatly which
tried to reference the address space after gfs2 had freed the superblock
that contained it.

This patch adds a call to truncate_inode_pages_final for sd_aspace, thus
avoiding the use-after-free.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/super.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index b285192bd6b3..b3d951ab8068 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -738,6 +738,7 @@ static void gfs2_put_super(struct super_block *sb)
 	gfs2_jindex_free(sdp);
 	/*  Take apart glock structures and buffer lists  */
 	gfs2_gl_hash_clear(sdp);
+	truncate_inode_pages_final(&sdp->sd_aspace);
 	gfs2_delete_debugfs_file(sdp);
 	/*  Unmount the locking protocol  */
 	gfs2_lm_unmount(sdp);
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [Cluster-devel] [GFS2 PATCH 3/3] gfs2: Use filemap_fdatawrite_range for gfs2_meta_sync
  2020-10-27 14:52 [Cluster-devel] [GFS2 PATCH 0/3] gfs2: Fix 'dont sleep' GPF from vfs Bob Peterson
  2020-10-27 14:52 ` [Cluster-devel] [GFS2 PATCH 1/3] gfs2: Free rd_bits later in gfs2_clear-rgrpd to fix use-after-free Bob Peterson
  2020-10-27 14:52 ` [Cluster-devel] [GFS2 PATCH 2/3] gfs2: Add missing truncate_inode_pages_final for sd_aspace Bob Peterson
@ 2020-10-27 14:52 ` Bob Peterson
  2 siblings, 0 replies; 5+ messages in thread
From: Bob Peterson @ 2020-10-27 14:52 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Before this patch, function gfs2_meta_sync called filemap_fdatawrite to write
the address space for the metadata being synced. That's great for inodes, but
for resource groups, they all point to the same superblock-address space,
sdp->sd_aspace, with their own range. That meant every time an rgrp's metadata
was synced, it would write all of them instead of just the range.

This patch changes function gfs2_meta_sync so that instead it calls
filemap_fdatawrite_range and filemap_fdatawait_range with a range of addresses
appropriate to the object. For inodes, it specifies the whole range, as
per filemap_fdatawrite. For rgrps, it uses only the range required.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>

12345678901234567890123456789012345678901234567890123456789012345678901234567890
---
 fs/gfs2/lops.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
index ed69298dd824..94b0e89ee7b2 100644
--- a/fs/gfs2/lops.c
+++ b/fs/gfs2/lops.c
@@ -827,13 +827,21 @@ void gfs2_meta_sync(struct gfs2_glock *gl)
 {
 	struct address_space *mapping = gfs2_glock2aspace(gl);
 	struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
+	loff_t start = 0;
+	loff_t end = LLONG_MAX;
 	int error;
 
-	if (mapping == NULL)
+	if (mapping == NULL) {
+		struct gfs2_rgrpd *rgd = gfs2_glock2rgrp(gl);
+		const unsigned bsize = sdp->sd_sb.sb_bsize;
+
 		mapping = &sdp->sd_aspace;
+		start = (rgd->rd_addr * bsize) & PAGE_MASK;
+		end = PAGE_ALIGN((rgd->rd_addr + rgd->rd_length) * bsize) - 1;
+	}
 
-	filemap_fdatawrite(mapping);
-	error = filemap_fdatawait(mapping);
+	filemap_fdatawrite_range(mapping, start, end);
+	error = filemap_fdatawait_range(mapping, start, end);
 
 	if (error)
 		gfs2_io_error(gl->gl_name.ln_sbd);
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [Cluster-devel] [GFS2 PATCH 2/3] gfs2: Add missing truncate_inode_pages_final for sd_aspace
  2020-10-27 15:10 [Cluster-devel] [GFS2 PATCH 0/3] gfs2: Fix 'dont sleep' GPF from vfs Bob Peterson
@ 2020-10-27 15:10 ` Bob Peterson
  0 siblings, 0 replies; 5+ messages in thread
From: Bob Peterson @ 2020-10-27 15:10 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Gfs2 creates an address space for its rgrps called sd_aspace, but it never
called truncate_inode_pages_final on it. This confused vfs greatly which
tried to reference the address space after gfs2 had freed the superblock
that contained it.

This patch adds a call to truncate_inode_pages_final for sd_aspace, thus
avoiding the use-after-free.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/super.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index b285192bd6b3..b3d951ab8068 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -738,6 +738,7 @@ static void gfs2_put_super(struct super_block *sb)
 	gfs2_jindex_free(sdp);
 	/*  Take apart glock structures and buffer lists  */
 	gfs2_gl_hash_clear(sdp);
+	truncate_inode_pages_final(&sdp->sd_aspace);
 	gfs2_delete_debugfs_file(sdp);
 	/*  Unmount the locking protocol  */
 	gfs2_lm_unmount(sdp);
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-10-27 15:10 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-10-27 14:52 [Cluster-devel] [GFS2 PATCH 0/3] gfs2: Fix 'dont sleep' GPF from vfs Bob Peterson
2020-10-27 14:52 ` [Cluster-devel] [GFS2 PATCH 1/3] gfs2: Free rd_bits later in gfs2_clear-rgrpd to fix use-after-free Bob Peterson
2020-10-27 14:52 ` [Cluster-devel] [GFS2 PATCH 2/3] gfs2: Add missing truncate_inode_pages_final for sd_aspace Bob Peterson
2020-10-27 14:52 ` [Cluster-devel] [GFS2 PATCH 3/3] gfs2: Use filemap_fdatawrite_range for gfs2_meta_sync Bob Peterson
  -- strict thread matches above, loose matches on Subject: below --
2020-10-27 15:10 [Cluster-devel] [GFS2 PATCH 0/3] gfs2: Fix 'dont sleep' GPF from vfs Bob Peterson
2020-10-27 15:10 ` [Cluster-devel] [GFS2 PATCH 2/3] gfs2: Add missing truncate_inode_pages_final for sd_aspace Bob Peterson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).