Re: [PATCH] fs/mbcache: cancel shrink work before destroying the cache

[Jan Kara]

On Tue 17-03-26 14:45:56, Hyungjung Joo wrote:
> From: HyungJung Joo <jhj140711@xxxxxxxxx>
> 
> mb_cache_destroy() calls shrinker_free() and then frees all cache
> entries and the cache itself, but it does not cancel the pending
> c_shrink_work work item first.
> 
> If mb_cache_entry_create() schedules c_shrink_work via schedule_work()
> and the work item is still pending or running when mb_cache_destroy()
> runs, mb_cache_shrink_worker() will access the cache after its memory
> has been freed, causing a use-after-free.
> 
> This is only reachable by a privileged user (root or CAP_SYS_ADMIN)
> who can trigger the last put of a mounted ext2/ext4/ocfs2 filesystem.
> 
> Cancel the work item with cancel_work_sync() before calling
> shrinker_free(), ensuring the worker has finished and will not be
> rescheduled before the cache is torn down.
> 
> Signed-off-by: Hyungjung Joo <jhj140711@xxxxxxxxx>

Thanks! The patch looks good to me. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza


> ---
>  fs/mbcache.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/mbcache.c b/fs/mbcache.c
> index 480d02d6ebf0..2a6319b4072c 100644
> --- a/fs/mbcache.c
> +++ b/fs/mbcache.c
> @@ -406,6 +406,7 @@ void mb_cache_destroy(struct mb_cache *cache)
>  {
>  	struct mb_cache_entry *entry, *next;
>  
> +	cancel_work_sync(&cache->c_shrink_work);
>  	shrinker_free(cache->c_shrink);
>  
>  	/*
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR