[Devel] [PATCH RHEL7 COMMIT] fs/fuse kio: don't use kio with uninitialized statistic

Vasily Averin vvs at virtuozzo.com
Thu Apr 1 10:46:51 MSK 2021


The commit is pushed to "branch-rh7-3.10.0-1160.21.1.vz7.174.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-1160.21.1.vz7.174.4
------>
commit 7a66d15f7d8d1d40c2ef96148e98f55320445583
Author: Ildar Ismagilov <ildar.ismagilov at virtuozzo.com>
Date:   Thu Apr 1 10:46:51 2021 +0300

    fs/fuse kio: don't use kio with uninitialized statistic
    
    The uninitialized pcs_cluster_core::stat leads to the following problem:
    
    Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: ffffffffc0a820bd
    Call Trace:
     [<ffffffff87fbaf0f>] dump_stack+0x19/0x1b
     [<ffffffff87fb3e86>] panic+0xe8/0x21f
     [<ffffffffc0a820bd>] ? pcs_fuse_stat_io_count+0x16d/0x180 [fuse_kio_pcs]
     [<ffffffff8789c41b>] __stack_chk_fail+0x1b/0x20
     [<ffffffffc0a820bd>] pcs_fuse_stat_io_count+0x16d/0x180 [fuse_kio_pcs]
     [<ffffffffc0a72ca5>] commit_sync_info+0x205/0x290 [fuse_kio_pcs]
     [<ffffffffc0a7983d>] pcs_map_verify_sync_state+0x7d/0x100 [fuse_kio_pcs]
     [<ffffffffc0a7cffa>] cs_response_done+0x28a/0x2c0 [fuse_kio_pcs]
     [<ffffffffc0a7c3bd>] ? cs_get_data+0xfd/0x190 [fuse_kio_pcs]
     [<ffffffffc0a6eaa7>] ? pcs_rpc_lookup_xid+0x97/0xd0 [fuse_kio_pcs]
     [<ffffffffc0a7103d>] rpc_work_input+0x27d/0x340 [fuse_kio_pcs]
     [<ffffffffc0a6dd47>] pcs_sockio_xmit+0x217/0x520 [fuse_kio_pcs]
     [<ffffffff87fc0a76>] ? __schedule+0x596/0x9c0
     [<ffffffff87fc0a82>] ? __schedule+0x5a2/0x9c0
     [<ffffffff87fc0a76>] ? __schedule+0x596/0x9c0
     [<ffffffffc0a6fdd6>] rpc_queue_work+0x1f6/0x380 [fuse_kio_pcs]
     [<ffffffff878c0ae5>] process_one_work+0x185/0x440
     [<ffffffff878c17a6>] worker_thread+0x126/0x3c0
     [<ffffffff878c1680>] ? manage_workers.isra.26+0x2a0/0x2a0
     [<ffffffff878c8df1>] kthread+0xd1/0xe0
     [<ffffffff878c8d20>] ? create_kthread+0x60/0x60
     [<ffffffff87fcde9d>] ret_from_fork_nospec_begin+0x7/0x21
     [<ffffffff878c8d20>] ? create_kthread+0x60/0x60
    
    How to reproduce:
    The fuse_iostat_count() calls req_stat_entry() with io == NULL, and in case of
    type == PCS_CS_WRITE_RESP the return value will be (NULL + sizeof(struct fuse_val_stat)) == 0x20.
    After checking of returned value the fuse_val_stat_update() will be called with
    se == 0x20, and corrupts the stack protector cookie whose address
    is percpu_base + 0x28.
    
    Let's abort initialization of kio in case of uninitialized stat, to
    prevent it.
    
    https://pmc.acronis.com/browse/VSTOR-42099
    v2: no need to call fuse_stat_rm_dentries() in case of fuse_control_sb == NULL,
    because the dentries have been removed.
    v3: Keep a reference to the fusectl superblock while kio is active,
    recommended by Alexey Kuznetsov
    Signed-off-by: Ildar Ismagilov <ildar.ismagilov at virtuozzo.com>
    [VvS: removed useless BUG_ON in req_stat_entry()
          replaced BUG_ON by WARN_ON in fuse_stat_add_dentry()]
    Acked-by: Andrey Zaitsev <Andrey.Zaitsev at acronis.com>
    Signed-off-by: Vasily Averin <vvs at virtuozzo.com>
---
 fs/fuse/kio/pcs/fuse_stat.c        | 142 ++++++++++++++++++++-----------------
 fs/fuse/kio/pcs/fuse_stat.h        |  12 ++--
 fs/fuse/kio/pcs/pcs_cluster_core.c |   8 ++-
 3 files changed, 88 insertions(+), 74 deletions(-)

diff --git a/fs/fuse/kio/pcs/fuse_stat.c b/fs/fuse/kio/pcs/fuse_stat.c
index e92bce8..8a513d6 100644
--- a/fs/fuse/kio/pcs/fuse_stat.c
+++ b/fs/fuse/kio/pcs/fuse_stat.c
@@ -778,17 +778,23 @@ static void pcs_fuse_stat_work(struct work_struct *w)
 	mod_delayed_work(cc->wq, &cc->stat.work, STAT_TIMER_PERIOD * HZ);
 }
 
-static struct dentry *fuse_kio_add_dentry(struct dentry *parent,
-					  struct fuse_conn *fc,
-					  const char *name,
-					  int mode, int nlink,
-					  const struct inode_operations *iop,
-					  const struct file_operations *fop,
-					  void *ctx)
+static struct dentry *fuse_stat_add_dentry(struct dentry *parent,
+					   struct pcs_fuse_stat *stat,
+					   const char *name,
+					   int mode, int nlink,
+					   const struct inode_operations *iop,
+					   const struct file_operations *fop,
+					   void *ctx)
 {
+	struct pcs_cluster_core *cc =
+		container_of(stat, struct pcs_cluster_core, stat);
+	struct fuse_conn *fc = container_of(cc,struct pcs_fuse_cluster, cc)->fc;
 	struct inode *inode;
-	struct dentry *dentry = d_alloc_name(parent, name);
+	struct dentry *dentry;
+
+	WARN_ON(stat->ctl_ndents >= STAT_NUM_DENTRIES);
 
+	dentry = d_alloc_name(parent, name);
 	if (!dentry)
 		return NULL;
 
@@ -810,14 +816,22 @@ static struct dentry *fuse_kio_add_dentry(struct dentry *parent,
 	inode->i_private = ctx;
 	d_add(dentry, inode);
 
+	stat->ctl_dentry[stat->ctl_ndents++] = dentry;
+
 	return dentry;
 }
 
-static void fuse_kio_rm_dentry(struct dentry *dentry)
+static void fuse_stat_rm_dentries(struct pcs_fuse_stat *stat)
 {
-	d_inode(dentry)->i_private = NULL;
-	d_drop(dentry);
-	dput(dentry);
+	int i;
+
+	for (i = stat->ctl_ndents - 1; i >= 0; i--) {
+		struct dentry *dentry = stat->ctl_dentry[i];
+		d_inode(dentry)->i_private = NULL;
+		d_drop(dentry);
+		dput(dentry);
+	}
+	stat->ctl_ndents = 0;
 }
 
 int pcs_fuse_fstat_alloc(struct pcs_fuse_io_lat_sync *lat)
@@ -874,86 +888,84 @@ void pcs_fuse_io_stat_free(struct pcs_fuse_io_stat_sync *iostat)
 	free_percpu(iostat->CURR(iostat));
 }
 
-void pcs_fuse_stat_init(struct pcs_fuse_stat *stat)
+int pcs_fuse_stat_init(struct pcs_fuse_stat *stat)
 {
 	struct pcs_cluster_core *cc =
 		container_of(stat, struct pcs_cluster_core, stat);
 	struct fuse_conn *fc = container_of(cc,struct pcs_fuse_cluster, cc)->fc;
+	struct dentry *kio_stat;
 
 	mutex_lock(&fuse_mutex);
+
+	stat->ctl_ndents = 0;
+
 	if (!fuse_control_sb)
-		goto fail1;
+		goto fail;
 
 	if (pcs_fuse_io_stat_alloc(&stat->io))
-		goto fail1;
-
-	stat->kio_stat = fuse_kio_add_dentry(fc->conn_ctl, fc, "kio_stat",
-					     S_IFDIR | S_IXUSR, 2,
-					     &simple_dir_inode_operations,
-					     &simple_dir_operations, fc);
-	if (!stat->kio_stat) {
-		pr_err("kio: can't create kio stat directory");
-		goto fail2;
-	}
+		goto fail;
 
 	INIT_DELAYED_WORK(&stat->work, pcs_fuse_stat_work);
-	mod_delayed_work(cc->wq, &stat->work, STAT_TIMER_PERIOD * HZ);
 
-	stat->iostat = fuse_kio_add_dentry(stat->kio_stat, fc, "iostat",
-					   S_IFREG | S_IRUSR, 1, NULL,
-					   &pcs_fuse_iostat_ops, stat);
-	stat->requests = fuse_kio_add_dentry(stat->kio_stat, fc, "requests",
-					     S_IFREG | S_IRUSR, 1, NULL,
-					     &pcs_fuse_requests_ops, stat);
-	stat->fstat = fuse_kio_add_dentry(stat->kio_stat, fc, "fstat",
-					  S_IFREG | S_IRUSR, 1, NULL,
-					  &pcs_fuse_fstat_ops, stat);
-	stat->fstat_lat = fuse_kio_add_dentry(stat->kio_stat, fc, "fstat_lat",
-					      S_IFREG | S_IRUSR, 1, NULL,
-					      &pcs_fuse_fstat_lat_ops, stat);
-	stat->cs_stats = fuse_kio_add_dentry(stat->kio_stat, fc, "cs_stats",
-					     S_IFREG | S_IRUSR, 1, NULL,
-					     &pcs_fuse_cs_stats_ops, stat);
-	stat->storage_version = fuse_kio_add_dentry(stat->kio_stat, fc,
-						    "storage_version",
-						    S_IFREG | S_IRUSR | S_IWUSR, 1, NULL,
-						    &pcs_fuse_storage_version_ops,
-						    stat);
+	kio_stat = fuse_stat_add_dentry(fc->conn_ctl, stat, "kio_stat",
+					S_IFDIR | S_IXUSR, 2,
+					&simple_dir_inode_operations,
+					&simple_dir_operations, fc);
+	if (!kio_stat)
+		goto fail_dentries;
+
+	if (!fuse_stat_add_dentry(kio_stat, stat, "iostat",
+				  S_IFREG | S_IRUSR, 1, NULL,
+				  &pcs_fuse_iostat_ops, stat) ||
+	    !fuse_stat_add_dentry(kio_stat, stat, "requests",
+				  S_IFREG | S_IRUSR, 1, NULL,
+				  &pcs_fuse_requests_ops, stat) ||
+	    !fuse_stat_add_dentry(kio_stat, stat, "fstat",
+				  S_IFREG | S_IRUSR, 1, NULL,
+				  &pcs_fuse_fstat_ops, stat) ||
+	    !fuse_stat_add_dentry(kio_stat, stat, "fstat_lat",
+				  S_IFREG | S_IRUSR, 1, NULL,
+				  &pcs_fuse_fstat_lat_ops, stat) ||
+	    !fuse_stat_add_dentry(kio_stat, stat, "cs_stats",
+				  S_IFREG | S_IRUSR, 1, NULL,
+				  &pcs_fuse_cs_stats_ops, stat) ||
+	    !fuse_stat_add_dentry(kio_stat, stat,
+				  "storage_version",
+				  S_IFREG | S_IRUSR | S_IWUSR, 1, NULL,
+				  &pcs_fuse_storage_version_ops,
+				  stat))
+		goto fail_dentries;
+
+	atomic_inc(&fuse_control_sb->s_active);
+
 	mutex_unlock(&fuse_mutex);
-	return;
 
-fail2:
+	mod_delayed_work(cc->wq, &stat->work, STAT_TIMER_PERIOD * HZ);
+
+	return 0;
+
+fail_dentries:
+	fuse_stat_rm_dentries(stat);
 	pcs_fuse_io_stat_free(&stat->io);
-fail1:
-	stat->kio_stat = NULL;
+fail:
 	mutex_unlock(&fuse_mutex);
+
+	return -ENOMEM;
 }
 
 void pcs_fuse_stat_fini(struct pcs_fuse_stat *stat)
 {
 	mutex_lock(&fuse_mutex);
-	if (!stat->kio_stat) {
+	if (!stat->ctl_ndents) {
 		mutex_unlock(&fuse_mutex);
 		return;
 	}
 
-	if (fuse_control_sb) {
-		if (stat->iostat)
-			fuse_kio_rm_dentry(stat->iostat);
-		if (stat->requests)
-			fuse_kio_rm_dentry(stat->requests);
-		if (stat->fstat)
-			fuse_kio_rm_dentry(stat->fstat);
-		if (stat->fstat_lat)
-			fuse_kio_rm_dentry(stat->fstat_lat);
-		if (stat->cs_stats)
-			fuse_kio_rm_dentry(stat->cs_stats);
-		if (stat->storage_version)
-			fuse_kio_rm_dentry(stat->storage_version);
-		fuse_kio_rm_dentry(stat->kio_stat);
-	}
+	fuse_stat_rm_dentries(stat);
 	mutex_unlock(&fuse_mutex);
 
 	cancel_delayed_work_sync(&stat->work);
 	pcs_fuse_io_stat_free(&stat->io);
+
+	deactivate_super(fuse_control_sb);
 }
diff --git a/fs/fuse/kio/pcs/fuse_stat.h b/fs/fuse/kio/pcs/fuse_stat.h
index 36ff847..016877c 100644
--- a/fs/fuse/kio/pcs/fuse_stat.h
+++ b/fs/fuse/kio/pcs/fuse_stat.h
@@ -2,6 +2,7 @@
 #define _FUSE_STAT_H_ 1
 
 #define STAT_TIMER_PERIOD 5
+#define STAT_NUM_DENTRIES 7
 
 struct pcs_msg;
 struct pcs_int_request;
@@ -31,13 +32,8 @@ struct pcs_fuse_stat {
 	struct pcs_fuse_io_stat_sync io;
 	struct delayed_work work;
 
-	struct dentry *kio_stat;
-	struct dentry *iostat;
-	struct dentry *requests;
-	struct dentry *fstat;
-	struct dentry *fstat_lat;
-	struct dentry *cs_stats;
-	struct dentry *storage_version;
+	struct dentry *ctl_dentry[STAT_NUM_DENTRIES];
+	int ctl_ndents;
 };
 
 enum {
@@ -76,7 +72,7 @@ struct fuse_io_cnt {
 	abs_time_t created_ts;
 };
 
-void pcs_fuse_stat_init(struct pcs_fuse_stat *stat);
+int pcs_fuse_stat_init(struct pcs_fuse_stat *stat);
 void pcs_fuse_stat_fini(struct pcs_fuse_stat *stat);
 
 void pcs_fuse_stat_io_count(struct pcs_int_request *ireq, struct pcs_msg *resp,
diff --git a/fs/fuse/kio/pcs/pcs_cluster_core.c b/fs/fuse/kio/pcs/pcs_cluster_core.c
index a2b16f8..a314b8b 100644
--- a/fs/fuse/kio/pcs/pcs_cluster_core.c
+++ b/fs/fuse/kio/pcs/pcs_cluster_core.c
@@ -165,7 +165,13 @@ int pcs_cc_init(struct pcs_cluster_core *cc, struct workqueue_struct *wq,
 	 * pcs_srandomdev(&cc->rng);
 	 */
 
-	pcs_fuse_stat_init(&cc->stat);
+	err = pcs_fuse_stat_init(&cc->stat);
+	if (err) {
+		pcs_csset_fini(&cc->css);
+		pcs_mapset_fini(&cc->maps);
+		pcs_rpc_engine_fini(&cc->eng);
+		return err;
+	}
 
 	memset(&cc->cfg,   0, sizeof(cc->cfg));
 	memset(&cc->op,	   0, sizeof(cc->op));


More information about the Devel mailing list