standards/154185: race condition in mb_dupcl

[Zhouyi Zhou]

>Number:         154185
>Category:       standards
>Synopsis:       race condition in mb_dupcl
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    freebsd-standards
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Fri Jan 21 05:40:06 UTC 2011
>Closed-Date:
>Last-Modified:
>Originator:     Zhouyi Zhou
>Release:        FreeBSD 8.0
>Organization:
ICT CAS
>Environment:
FreeBSD zzy 8.0-RELEASE FreeBSD 8.0-RELEASE #85: Fri Jan 21 12:56:40 UTC 2011     root at zzy:/root/sys/amd64/compile/GENERIC  amd64
>Description:
There is race condition in function mb_dupcl in src/sys/kern/uipc_mbuf.c 

The code to add reference count in mb_dupcl is as follows
     

 390         if (*(m->m_ext.ref_cnt) == 1)
 391                 *(m->m_ext.ref_cnt) += 1;

The x86 asm code for these two lines is:
0xffffffff807aa28d <mb_dupcl+13>:       sub    $0x1,%eax
0xffffffff807aa290 <mb_dupcl+16>:       je     0xffffffff807aa2f0 <mb_dupcl+112>
..
0xffffffff807aa2f0 <mb_dupcl+112>:      mov    (%rdx),%ecx
0xffffffff807aa2f2 <mb_dupcl+114>:      add    $0x1,%ecx
0xffffffff807aa2f5 <mb_dupcl+117>:      mov    %ecx,(%rdx)

It is appearant that multiple threads will race for the contents of (%rdx)
>How-To-Repeat:
Can write a kernel module like:

struct mbuf *m;
MGETHDR(m, M_DONTWAIT, MT_DATA);
MCLGET(m, M_DONTWAIT);

Then let multiple threads 
execute simulately:
 m1 = m_copypacket(m, M_DONTWAIT);
 m_freem(m1);
>Fix:
function mb_dupcl
    390 -        if (*(m->m_ext.ref_cnt) == 1)
    391 -                *(m->m_ext.ref_cnt) += 1;
    392 -        else
    393                 atomic_add_int(m->m_ext.ref_cnt, 1);



>Release-Note:
>Audit-Trail:
>Unformatted: