Re: [PATCH 1/1] shmem: fix unicode_map leak on remount with casefold=

[Gabriel Krisman Bertazi]

Mohammed EL Kadiri <med08elkadiri@xxxxxxxxx> writes:

> shmem_parse_opt_casefold() calls utf8_load(), which allocates a fresh
> struct unicode_map, and stores it in ctx->encoding.
>
> On mount, shmem_fill_super() takes ownership of it via sb->s_encoding,
> and shmem_put_super() frees it with utf8_unload().  On remount nothing
> takes ownership: shmem_reconfigure() never looks at ctx->encoding, and
> shmem_free_fc() only frees ctx itself.  The map is leaked.
>
> 50000 x "mount -o remount,casefold=utf8-12.1.0 /t" on a casefolded tmpfs
> grows kmalloc-32 from 660 to 50574 active objects, and nothing is
> reclaimed on umount.  A tmpfs mounted without casefold stays flat.  With
> this patch kmalloc-32 stays flat too.
>
> Clearing ctx->encoding after the transfer is what makes the unload in
> shmem_free_fc() safe: otherwise a failure later in mount would free the
> same map twice, once via put_super and once via fc->free.  This mirrors
> what shmem_reconfigure() already does with ctx->mpol.
>
> This is easy to hit once casefold= is reported in /proc/mounts, since
> mount(8) then passes it back on every remount of a casefolded tmpfs.
>
> Signed-off-by: Mohammed EL Kadiri <med08elkadiri@xxxxxxxxx>

Thanks,

I gave this a spin as well and it looks good.  Feel free to add:


Reviewed-by: Gabriel Krisman Bertazi <krisman@xxxxxxx>

Thanks,

-- 
Gabriel Krisman Bertazi