Re: [Xen-devel] missing lock in percpu_rwlock? (Was: Re: New Defects reported by Coverity Scan for XenProject)

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Wed, 2016-02-03 at 10:45 +0000, Ian Campbell wrote:
> On Tue, 2016-02-02 at 20:23 -0800, scan-admin@xxxxxxxxxxxx wrote:
> > * CID 1351223: Concurrent data access violations (MISSING_LOCK)
> > /xen/include/xen/spinlock.h: 362 in _percpu_write_unlock()
> 
> Coverity seems to think this is new inÂ41b0aa569adb..9937763265d,
> presumably due toÂ
> 
> commit f9dd43dddc0a31a4343a58072935c1b5c0cbbee
> Author: Malcolm Crossley <malcolm.crossley@xxxxxxxxxx>
> Date:ÂÂÂFri Jan 22 16:04:41 2016 +0100
> 
> ÂÂÂÂrwlock: add per-cpu reader-writer lock infrastructure

It also reports this one, but I suppose this is a false +ve given the name
of the function.

(Also note "simulatenously" should be "simultaneously")

    ** CID 1351220:ÂÂProgram hangsÂÂ(LOCK)
    /xen/include/xen/spinlock.h: 310 in _percpu_read_lock()


    
________________________________________________________________________________________________________
    *** CID 1351220:ÂÂProgram hangsÂÂ(LOCK)
    /xen/include/xen/spinlock.h: 310 in _percpu_read_lock()
    304ÂÂÂÂÂÂÂÂÂÂ* Detect using a second percpu_rwlock_t simulatenously and 
fallback
    305ÂÂÂÂÂÂÂÂÂÂ* to standard read_lock.
    306ÂÂÂÂÂÂÂÂÂÂ*/
    307ÂÂÂÂÂÂÂÂÂif ( unlikely(this_cpu_ptr(per_cpudata) != NULL ) )
    308ÂÂÂÂÂÂÂÂÂ{
    309ÂÂÂÂÂÂÂÂÂÂÂÂÂread_lock(&percpu_rwlock->rwlock);
    >>>ÂÂÂÂÂCID 1351220:ÂÂProgram hangsÂÂ(LOCK)
    >>>ÂÂÂÂÂReturning without unlocking "percpu_rwlock->rwlock".
    310ÂÂÂÂÂÂÂÂÂÂÂÂÂreturn;
    311ÂÂÂÂÂÂÂÂÂ}
    312ÂÂÂÂÂ
    313ÂÂÂÂÂÂÂÂÂ/* Indicate this cpu is reading. */
    314ÂÂÂÂÂÂÂÂÂthis_cpu_ptr(per_cpudata) = percpu_rwlock;
    315ÂÂÂÂÂÂÂÂÂsmp_mb();


> 
> > _______________________________________________________________________
> > __
> > _______________________________
> > *** CID 1351223: Concurrent data access violations (MISSING_LOCK)
> > /xen/include/xen/spinlock.h: 362 in _percpu_write_unlock()
> > 356ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ percpu_rwlock_t *percpu_rwlock)
> > 357ÂÂÂÂ {
> > 358ÂÂÂÂÂÂÂÂ /* Validate the correct per_cpudata variable has been
> > provided. */
> > 359ÂÂÂÂÂÂÂÂ _percpu_rwlock_owner_check(per_cpudata, percpu_rwlock);
> > 360ÂÂÂÂ 
> > 361ÂÂÂÂÂÂÂÂ ASSERT(percpu_rwlock->writer_activating);
> > > > > ÂÂÂÂ CID 1351223:Â Concurrent data access violationsÂ
> > > > > (MISSING_LOCK)
> > > > > ÂÂÂÂ Accessing "percpu_rwlock->writer_activating" without holding
> > > > > lock
> > "percpu_rwlock.rwlock". Elsewhere, "percpu_rwlock.writer_activating" is
> > accessed with "percpu_rwlock.rwlock" held 1 out of 2 times (1 of these
> > accesses strongly imply that it is necessary).
> > 362ÂÂÂÂÂÂÂÂ percpu_rwlock->writer_activating = 0;
> > 363ÂÂÂÂÂÂÂÂ write_unlock(&percpu_rwlock->rwlock);
> > 364ÂÂÂÂ }
> > 365ÂÂÂÂ 
> > 366ÂÂÂÂ #define percpu_rw_is_write_locked(l)ÂÂÂÂÂÂÂÂ
> > _rw_is_write_locked(&((l)->rwlock))

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel