On Wed, 25 Dec 2024, Yang Erkun wrote:
> We can access exp->ex_stats or exp->ex_uuid in rcu context(c_show and
> e_show). All these resources should be released using kfree_rcu. Fix this
> by using call_rcu, clean them all after a rcu grace period.
>
> ==================================================================
> BUG: KASAN: slab-use-after-free in svc_export_show+0x362/0x430 [nfsd]
> Read of size 1 at addr ff11000010fdc120 by task cat/870
>
> CPU: 1 UID: 0 PID: 870 Comm: cat Not tainted 6.12.0-rc3+ #1
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> 1.16.1-2.fc37 04/01/2014
> Call Trace:
> <TASK>
> dump_stack_lvl+0x53/0x70
> print_address_description.constprop.0+0x2c/0x3a0
> print_report+0xb9/0x280
> kasan_report+0xae/0xe0
> svc_export_show+0x362/0x430 [nfsd]
> c_show+0x161/0x390 [sunrpc]
> seq_read_iter+0x589/0x770
> seq_read+0x1e5/0x270
> proc_reg_read+0xe1/0x140
> vfs_read+0x125/0x530
> ksys_read+0xc1/0x160
> do_syscall_64+0x5f/0x170
> entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> Allocated by task 830:
> kasan_save_stack+0x20/0x40
> kasan_save_track+0x14/0x30
> __kasan_kmalloc+0x8f/0xa0
> __kmalloc_node_track_caller_noprof+0x1bc/0x400
> kmemdup_noprof+0x22/0x50
> svc_export_parse+0x8a9/0xb80 [nfsd]
> cache_do_downcall+0x71/0xa0 [sunrpc]
> cache_write_procfs+0x8e/0xd0 [sunrpc]
> proc_reg_write+0xe1/0x140
> vfs_write+0x1a5/0x6d0
> ksys_write+0xc1/0x160
> do_syscall_64+0x5f/0x170
> entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> Freed by task 868:
> kasan_save_stack+0x20/0x40
> kasan_save_track+0x14/0x30
> kasan_save_free_info+0x3b/0x60
> __kasan_slab_free+0x37/0x50
> kfree+0xf3/0x3e0
> svc_export_put+0x87/0xb0 [nfsd]
> cache_purge+0x17f/0x1f0 [sunrpc]
> nfsd_destroy_serv+0x226/0x2d0 [nfsd]
> nfsd_svc+0x125/0x1e0 [nfsd]
> write_threads+0x16a/0x2a0 [nfsd]
> nfsctl_transaction_write+0x74/0xa0 [nfsd]
> vfs_write+0x1a5/0x6d0
> ksys_write+0xc1/0x160
> do_syscall_64+0x5f/0x170
> entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> Fixes: ae74136b4bb6 ("SUNRPC: Allow cache lookups to use RCU protection rather than the r/w spinlock")
> Reviewed-by: NeilBrown <neilb@xxxxxxx>
> Signed-off-by: Yang Erkun <yangerkun@xxxxxxxxxx>
> ---
> fs/nfsd/export.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> index c6168bccfb6c..0363720280d4 100644
> --- a/fs/nfsd/export.c
> +++ b/fs/nfsd/export.c
> @@ -355,16 +355,25 @@ static void export_stats_destroy(struct export_stats *stats)
> EXP_STATS_COUNTERS_NUM);
> }
>
> -static void svc_export_put(struct kref *ref)
> +static void svc_export_release(struct rcu_head *rcu_head)
> {
> - struct svc_export *exp = container_of(ref, struct svc_export, h.ref);
> - path_put(&exp->ex_path);
> - auth_domain_put(exp->ex_client);
> + struct svc_export *exp = container_of(rcu_head, struct svc_export,
> + ex_rcu);
> +
> nfsd4_fslocs_free(&exp->ex_fslocs);
> export_stats_destroy(exp->ex_stats);
> kfree(exp->ex_stats);
> kfree(exp->ex_uuid);
> - kfree_rcu(exp, ex_rcu);
> + kfree(exp);
> +}
> +
> +static void svc_export_put(struct kref *ref)
> +{
> + struct svc_export *exp = container_of(ref, struct svc_export, h.ref);
> +
> + path_put(&exp->ex_path);
> + auth_domain_put(exp->ex_client);
> + call_rcu(&exp->ex_rcu, svc_export_release);
> }
>
I think that ip_map_put() needs to be fixed for the same reason that
svc_export_put() needed to be fixed.
ip_map_put() calls auth_domain_put() in ->im_client immediately, but
ip_map_show() accesses ->im_client.
So ip_map_put() needs to delay the auth_domain_put() using rcu.
These two fixes should really be *before* patch 3/4 as without these
fixes 3/4 introduces a bug.
Thanks,
NeilBrown
Thanks,
