public inbox for cygwin@cygwin.com
 help / color / mirror / Atom feed
* Question about flock - potential memory corruption?
@ 2015-09-04 20:22 Qian Hong
  2015-09-07 18:42 ` Qian Hong
  2015-09-08  8:58 ` Corinna Vinschen
  0 siblings, 2 replies; 9+ messages in thread
From: Qian Hong @ 2015-09-04 20:22 UTC (permalink / raw)
  To: cygwin

[-- 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-09-08 15:54 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-04 20:22 Question about flock - potential memory corruption? Qian Hong
2015-09-07 18:42 ` Qian Hong
2015-09-07 18:46   ` Qian Hong
2015-09-08  4:41     ` Sam Edge
2015-09-08  5:54       ` Qian Hong
2015-09-08  8:58 ` Corinna Vinschen
2015-09-08 10:36   ` Qian Hong
2015-09-08 15:31     ` Corinna Vinschen
2015-09-08 15:54       ` Qian Hong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).