linux-kernel - Re: [PATCH v4 5/5] md: protect md

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 for Android: free password hash cracker in your pocket

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

Date:   Tue, 4 Apr 2023 09:34:46 +0800
From:   Yu Kuai <yukuai1@...weicloud.com>
To:     Logan Gunthorpe <logang@...tatee.com>,
        Yu Kuai <yukuai1@...weicloud.com>, song@...nel.org
Cc:     linux-kernel@...r.kernel.org, linux-raid@...r.kernel.org,
        yi.zhang@...wei.com, yangerkun@...wei.com,
        "yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [PATCH v4 5/5] md: protect md_thread with rcu

Hi, Logan!

在 2023/04/03 23:53, Logan Gunthorpe 写道:
>>   
>>   /* caller need to make sured returned md_thread won't be freed */
>> -static inline struct md_thread *get_md_thread(struct md_thread *t)
>> +static inline struct md_thread *get_md_thread(struct md_thread __rcu *t)
>>   {
>> -	return t;
>> +	return rcu_access_pointer(t);
> 
> This should not be using rcu_access_pointer(). That function is only
> appropriate when the value of t is not being dereferenced. This should
> be using rcu_dereference_protected() with some reasoning as to why it's
> safe to use this function. It might make sense to open code this for
> every call site if the reasoning is different in each location.
> Preferrably the second argument in the check should be some lockdep
> condition that ensures this. If that's not possible, a comment
> explaining the reasoning why it is safe in all the call sites should be
> added here.

Yes, it's right rcu_dereference_protected() should be used here, I need
to take a look at each call site from patch 3 and figure out if they're
safe without rcu protection.

> 
> On one hand this is looking like my idea of using RCU is producing more
> churn than the spin lock. On the other hand I think it's cleaning up and
> documenting more unsafe use cases (like other potentially unsafe
> accesses of the the thread pointer). So I still think the RCU is a good
> approach here.

Yes, some other unsafe accesses is protected now in this patch. I'll
send a new version soon.

Thanks,
Kuai