netdev - Re: [PATCH v2 4/4] nfsd: fix UAF when access ex_uuid or ex

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3edb6771-9b61-2efc-1295-48d962af7acc@huaweicloud.com>
Date: Fri, 27 Dec 2024 08:54:21 +0800
From: yangerkun <yangerkun@...weicloud.com>
To: NeilBrown <neilb@...e.de>, Yang Erkun <yangerkun@...wei.com>
Cc: chuck.lever@...cle.com, jlayton@...nel.org, okorniev@...hat.com,
 Dai.Ngo@...cle.com, tom@...pey.com, trondmy@...nel.org, anna@...nel.org,
 davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com, horms@...nel.org, linux-nfs@...r.kernel.org,
 netdev@...r.kernel.org, liumingrui@...wei.com
Subject: Re: [PATCH v2 4/4] nfsd: fix UAF when access ex_uuid or ex_stats



在 2024/12/27 7:15, NeilBrown 写道:
> 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@...e.de>
>> Signed-off-by: Yang Erkun <yangerkun@...wei.com>
>> ---
>>   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.

Thanks for your detail review!

auth_domain_put will eventually call auth_domain_release, which all
.domain_release will call call_rcu to free auth_domain. Maybe it is safe
now?


> 
> These two fixes should really be *before* patch 3/4 as without these
> fixes 3/4 introduces a bug.
> 
> Thanks,
> NeilBrown
> 
> Thanks,