Re: [PATCH] nvmet: add missing locks around nvmet_ns_revalidate

[Bart Van Assche <bvanassche@acm.org>]

On 3/9/22 14:30, Niels Dossche wrote:
> On 09/03/2022 23:27, Bart Van Assche wrote:
>> On 3/9/22 12:34, Niels Dossche wrote:
>>> nvmet_ns_changed states via lockdep that the ns->subsys->lock
>>> must be held. The only caller of nvmet_ns_changed which does not
>>> acquire that lock is nvmet_ns_revalidate. The only 2 callers of
>>> nvmet_ns_revalidate which do not acquire that lock are
>>> nvmet_execute_identify_cns_cs_ns and nvmet_execute_identify_ns.
>>> Add a lock for around the call to nvmet_ns_revalidate in those 2
>>> functions.
>>> 
>>> Both of those identify functions are called from a common
>>> function nvmet_execute_identify, which itself is called
>>> indirectly via the req->execute function pointer.
>> 
>> Please mention in the patch description whether this has been
>> discovered by studying the source code or by software (static
>> source code analyzer? runtime data race detector?).
> 
> This was discovered by first using a static analyzer and then
> verifying it by manual inspection of the source code.

Hi Niels,

Are there any plans to make that static analyzer available to other 
kernel developers?

Is the static analyzer more powerful than clang thread safety 
annotations? If it is more powerful, is it possible to integrate the 
static analyzer in clang?

See also:
* "C/C++ Thread Safety Analysis" 
(https://static.googleusercontent.com/media/research.google.com/en//pubs/archive/42958.pdf).
* "Thread Safety Annotations for Clang" 
(https://llvm.org/devmtg/2011-11/Hutchins_ThreadSafety.pdf).

Thanks,

Bart.