Question about flock - potential memory corruption?

[Qian Hong <fracting@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2704 bytes --]

Dear list,

When testing Cygwin/MSYS2 on Wine, I found randomly failure of flock():
https://bugs.wine-staging.com/show_bug.cgi?id=466#c13

I ran MSYS2 with Wine+Valgrind, and found warnings like below when
calling flock():

  7 ==19315== Conditional jump or move depends on uninitialised value(s)
  8 ==19315==    at 0x7BC82750: RtlGetOwnerSecurityDescriptor (sec.c:740)
  9 ==19315==    by 0x7BC9222A: NTDLL_create_struct_sd (sync.c:96)
 10 ==19315==    by 0x7BC92CE4: NtCreateEvent (sync.c:294)
 11 ==19315==    by 0x6107B687: ???
 12 ==19315==    by 0x612FC347: ???

Then I read Cygwin/MSYS2 source code, and found:

--- snip ---
extern PSECURITY_DESCRIPTOR _everyone_sd (void *buf, ACCESS_MASK access);
#define everyone_sd(access) (_everyone_sd (alloca (SD_MIN_SIZE), (access)))
--- snip ---

man alloca says:
       The alloca() function allocates size bytes of space in the
stack frame of the caller.  This temporary space is automatically
freed when
       the function that called alloca() returns to its caller.

However, Cygwin/MSYS2 seems passed a freed pointer to NtCreateEvent:
https://cygwin.com/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/flock.cc;h=2332f5467e37d124acfd12c0f85a30281f10a952;hb=HEAD#l773

 638 POBJECT_ATTRIBUTES
 639 lockf_t::create_lock_obj_attr (lockfattr_t *attr, ULONG flags)
 640 {
 641   __small_swprintf (attr->name, LOCK_OBJ_NAME_FMT,
 642                     lf_flags & (F_POSIX | F_FLOCK), lf_type,
lf_start, lf_end,
 643                     lf_id, lf_wid, lf_ver);
 644   RtlInitCountedUnicodeString (&attr->uname, attr->name,
 645                                LOCK_OBJ_NAME_LEN * sizeof (WCHAR));
 646   InitializeObjectAttributes (&attr->attr, &attr->uname, flags,
lf_inode->i_dir,
 647                               everyone_sd (FLOCK_EVENT_ACCESS));
 648   return &attr->attr;
 649 }

 772       status = NtCreateEvent (&lf_obj, CYG_EVENT_ACCESS,
 773                               create_lock_obj_attr (&attr, OBJ_INHERIT),
 774                               NotificationEvent, FALSE);

It seems flock() works very stable on Windows according to my previous
testing, however, I have feeling that as a kernel function,
NtCreateEvent on Windows doesn't have terrible affects to the user
space stack of the process, while Wine implements NtCreateEvent as a
user space function, so the old stack was easier to be destroyed.

I write a hack as attachment 0001-cygwin-flock-user-static-buffer.txt
and recompile MSYS2, then the bug seems go away.

Could someone confirm whether there is a potential Cygwin bug? If true
I'd love to leave the bug for Cygwin devs to write a fix.

Thanks very much!



-- 
Regards,
Qian Hong

-
http://www.winehq.org

[-- Attachment #2: 0001-cygwin-flock-use-static-buffer.txt --]
[-- Type: text/plain, Size: 783 bytes --]

diff --git a/winsup/cygwin/flock.cc b/winsup/cygwin/flock.cc
index 2332f54..ac3f772 100644
--- a/winsup/cygwin/flock.cc
+++ b/winsup/cygwin/flock.cc
@@ -638,13 +638,14 @@ inode_t::get_all_locks_list ()
 POBJECT_ATTRIBUTES
 lockf_t::create_lock_obj_attr (lockfattr_t *attr, ULONG flags)
 {
+  static char buf[SD_MIN_SIZE];
   __small_swprintf (attr->name, LOCK_OBJ_NAME_FMT,
 		    lf_flags & (F_POSIX | F_FLOCK), lf_type, lf_start, lf_end,
 		    lf_id, lf_wid, lf_ver);
   RtlInitCountedUnicodeString (&attr->uname, attr->name,
 			       LOCK_OBJ_NAME_LEN * sizeof (WCHAR));
   InitializeObjectAttributes (&attr->attr, &attr->uname, flags, lf_inode->i_dir,
-			      everyone_sd (FLOCK_EVENT_ACCESS));
+			      _everyone_sd (buf, FLOCK_EVENT_ACCESS));
   return &attr->attr;
 }
 

[-- Attachment #3: Type: text/plain, Size: 218 bytes --]

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple