[rh7,v2] fs/fuse kio: fix fuse_mutex leak in pcs_fuse_stat_fini()

Submitted by Konstantin Khorenko on July 4, 2019, 8:41 a.m.

Details

Message ID 20190704084106.8000-1-khorenko@virtuozzo.com
State New
Series "fs/fuse kio: fix fuse_mutex leak in pcs_fuse_stat_fini()"
Headers show

Commit Message

Konstantin Khorenko July 4, 2019, 8:41 a.m.
From: Pavel Butsykin <pbutsykin@virtuozzo.com>

stat->kio_stat is checked for NULL in order to prevent freeing "stat" struct
fields before they are initialized in pcs_fuse_stat_init() (or may be kio_stat
is not initialized due to previous fails).

A side note about removing dentries only in case fuse_control_sb exists:
in pcs_fuse_stat_init() kio related dentries are initialized only in case
fuse_control_sb != NULL, and in fuse_ctl_kill_sb() fuse_control_sb is set to
NULL first and after that sb is killed along with all related dentries.

And stat kio dentries pointers are not set to NULL after fuse_kio_rm_dentry()
because it does not matter - it's a destroy time and whole pcs_fuse_cluster
struct along with stat struct is freed.

Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Acked-by: Konstantin Khorenko <khorenko@virtuozzo.com>

--
v2: skip stat->kio_stat NULL-ify because stat struct is going to be freed right
now.
---
 fs/fuse/kio/pcs/fuse_stat.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/fs/fuse/kio/pcs/fuse_stat.c b/fs/fuse/kio/pcs/fuse_stat.c
index bc3879d33de9..25d5572d6061 100644
--- a/fs/fuse/kio/pcs/fuse_stat.c
+++ b/fs/fuse/kio/pcs/fuse_stat.c
@@ -848,8 +848,10 @@  void pcs_fuse_stat_init(struct pcs_fuse_stat *stat)
 void pcs_fuse_stat_fini(struct pcs_fuse_stat *stat)
 {
 	mutex_lock(&fuse_mutex);
-	if (!stat->kio_stat)
+	if (!stat->kio_stat) {
+		mutex_unlock(&fuse_mutex);
 		return;
+	}
 
 	if (fuse_control_sb) {
 		if (stat->iostat)

Comments

Pavel Butsykin July 4, 2019, 8:53 a.m.
On 04.07.2019 11:41, Konstantin Khorenko wrote:
> From: Pavel Butsykin <pbutsykin@virtuozzo.com>
> 
> stat->kio_stat is checked for NULL in order to prevent freeing "stat" struct
> fields before they are initialized in pcs_fuse_stat_init() (or may be kio_stat
> is not initialized due to previous fails).

It's impossible to free "stat" struct before initialization, because it's
protected by fc->kio.ctx check. And fc->kio.ctx will be initialized after full
initialization of pcs_fuse_cluster structure.

> 
> A side note about removing dentries only in case fuse_control_sb exists:
> in pcs_fuse_stat_init() kio related dentries are initialized only in case
> fuse_control_sb != NULL, and in fuse_ctl_kill_sb() fuse_control_sb is set to
> NULL first and after that sb is killed along with all related dentries.
> 
> And stat kio dentries pointers are not set to NULL after fuse_kio_rm_dentry()
> because it does not matter - it's a destroy time and whole pcs_fuse_cluster
> struct along with stat struct is freed.
> 
> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
> Acked-by: Konstantin Khorenko <khorenko@virtuozzo.com>
> 
> --
> v2: skip stat->kio_stat NULL-ify because stat struct is going to be freed right
> now.
> ---
>   fs/fuse/kio/pcs/fuse_stat.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/fuse/kio/pcs/fuse_stat.c b/fs/fuse/kio/pcs/fuse_stat.c
> index bc3879d33de9..25d5572d6061 100644
> --- a/fs/fuse/kio/pcs/fuse_stat.c
> +++ b/fs/fuse/kio/pcs/fuse_stat.c
> @@ -848,8 +848,10 @@ void pcs_fuse_stat_init(struct pcs_fuse_stat *stat)
>   void pcs_fuse_stat_fini(struct pcs_fuse_stat *stat)
>   {
>   	mutex_lock(&fuse_mutex);
> -	if (!stat->kio_stat)
> +	if (!stat->kio_stat) {
> +		mutex_unlock(&fuse_mutex);
>   		return;
> +	}
>   
>   	if (fuse_control_sb) {
>   		if (stat->iostat)
>