[Devel] [PATCH RHEL8 COMMIT] fs/fuse kio: don't use kio with uninitialized statistic
Konstantin Khorenko
khorenko at virtuozzo.com
Fri Apr 23 11:55:02 MSK 2021
The commit is pushed to "branch-rh8-4.18.0-240.1.1.vz8.5.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh8-4.18.0-240.1.1.vz8.5.19
------>
commit 1a3c9b35fae1781c4ce9119a1153926d9c2691ef
Author: Ildar Ismagilov <ildar.ismagilov at virtuozzo.com>
Date: Fri Apr 23 11:55:02 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 f180d27b705b..68e8e75843a7 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 36ff8472a117..016877c963b5 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 8b7f33abddd8..f2b6d106e6b7 100644
--- a/fs/fuse/kio/pcs/pcs_cluster_core.c
+++ b/fs/fuse/kio/pcs/pcs_cluster_core.c
@@ -167,7 +167,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