[Bug libc/11449] crypt/crypt_util.c: __init_des_r() needs memory barrier

[Bug libc/11449] crypt/crypt_util.c: __init_des_r() needs memory barrier

[prince.cse99 at gmail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=11449

Abdullah Muzahid <prince.cse99 at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
                 CC|                            |prince.cse99 at gmail dot
                   |                            |com
         Resolution|FIXED                       |

--- Comment #3 from Abdullah Muzahid <prince.cse99 at gmail dot com> 2011-07-22 22:28:12 UTC ---
Hi,
I am a phd student in University of Illinois in CS dept. Recently I have been
working on memory model related bugs in software. I was experimenting with this
bug. And I found out that the bug is not properly fixed. __init_des_r() uses
a double checked locking(DCL) pattern. To make DCL work, we need to use 2
barrier - one before setting small_tables_initialized flag and one after
reading it in line 365 (just before acquiring the lock). The fix puts the first
barrier but not the second one. Now consider the following scenario
     Thread 1                                          Thread 2
eperm32tab[..] |= BITMASK[bit % 24];     if(small_tables_initialized == 0) 
atomic_write_barrier();
small_tables_initialized = 1;             sb[sg][inx  ]  = eperm32tab[...];

Now in Power-PC memory model, it is perfectly valid to execute read operations
to different addresses out of order as long as there is no barrier in between
them. Although thread 2 issues the instructions in order, it is possible that
the second read (...= experm32tab[...]) will execute before the first read of
the flag. This is shown here.

     Thread 1                                          Thread 2
                                         sb[sg][inx  ]  = eperm32tab[...];
eperm32tab[..] |= BITMASK[bit % 24];      
atomic_write_barrier();
small_tables_initialized = 1;            
                                         if(small_tables_initialized == 0) 

As a result, although the condition of the if statement for thread 2 becomes 
true, it will end up using wrong value of eperm32tab[...]. So, you need to put
a read_barrier after the if statement (i.e at line 491 before clearmem
operation). More on DCL can be found here 
http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html
Thanks.
-Abdullah Muzahid
 PhD Student
 CS, UIUC

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/11449] crypt/crypt_util.c: __init_des_r() needs memory barrier

[drepper.fsp at gmail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=11449

Ulrich Drepper <drepper.fsp at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |FIXED

--- Comment #4 from Ulrich Drepper <drepper.fsp at gmail dot com> 2011-07-23 03:27:04 UTC ---
(In reply to comment #3)
> Now in Power-PC memory model, it is perfectly valid to execute read operations
> to different addresses out of order as long as there is no barrier in between
> them. Although thread 2 issues the instructions in order, it is possible that
> the second read (...= experm32tab[...]) will execute before the first read of
> the flag. This is shown here.
> 
>      Thread 1                                          Thread 2
>                                          sb[sg][inx  ]  = eperm32tab[...];
> eperm32tab[..] |= BITMASK[bit % 24];      
> atomic_write_barrier();
> small_tables_initialized = 1;            

The only reason I made a change is because it doesn't disturb x86.  Your models
need adjustment to reality since you completely ignore the memset calls.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/11449] crypt/crypt_util.c: __init_des_r() needs memory barrier

[prince.cse99 at gmail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=11449

--- Comment #5 from Abdullah Muzahid <prince.cse99 at gmail dot com> 2011-07-23 05:19:35 UTC ---
If you consider only x86 machines, then you dont need the write barrier also
because in x86 writes do not get reordered.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/11449] crypt/crypt_util.c: __init_des_r() needs memory barrier

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=11449

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fweimer at redhat dot com
              Flags|                            |security-

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/11449] crypt/crypt_util.c: __init_des_r() needs memory barrier

[drepper at redhat dot com]

------- Additional Comments From drepper at redhat dot com  2010-04-03 20:45 -------
Fixed in git.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED


http://sourceware.org/bugzilla/show_bug.cgi?id=11449

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/11449] crypt/crypt_util.c: __init_des_r() needs memory barrier

[solar at openwall dot com]

------- Additional Comments From solar at openwall dot com  2010-03-29 20:49 -------
Created an attachment (id=4688)
 --> (http://sourceware.org/bugzilla/attachment.cgi?id=4688&action=view)
Proposed patch (adds #include's, uses sig_atomic_t, uses atomic_write_barrier)


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=11449

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.