Mail Archives: cygwin/2015/09/08/04:59:15
| X-Recipient: | archive-cygwin AT delorie DOT com
|
| DomainKey-Signature: | a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id
|
| :list-unsubscribe:list-subscribe:list-archive:list-post
|
| :list-help:sender:date:from:to:subject:message-id:reply-to
|
| :references:mime-version:content-type:in-reply-to; q=dns; s=
|
| default; b=XNm/0De5OqCwe+IFqQ7g5jH8bWv7C8GhMnXRtMIkA638AWLx7semr
|
| NjVRx1AFzMSclCz9W8sOEgaQUN+ZRnfViGXwO2Qcef1xEfZ79fI9TGd0tMG2ssnz
|
| Yl/unQehIlaQIsvXh+PIZuA+Ipfu7+bVZxpt4ON40AFnEB75t0wzTU=
|
| DKIM-Signature: | v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id
|
| :list-unsubscribe:list-subscribe:list-archive:list-post
|
| :list-help:sender:date:from:to:subject:message-id:reply-to
|
| :references:mime-version:content-type:in-reply-to; s=default;
|
| bh=8olV/JJxGoaFuJenHAw+34DGu3U=; b=SpFKH+A2gzZCDybSQJupa8G7UYJp
|
| VyF0GYWGxgSABkcBPhKC5ZdpqP7WjJM5EpyaSe8cbq0r3uxXZt80dYIORQV5Y+kb
|
| Haj5DEd8AkQ/GuwyhKitEvtO4ydtoy0amY+zBUZQheefXsv65f9cnXdO72xvINHg
|
| ra7J3F5i2uloP78=
|
| Mailing-List: | contact cygwin-help AT cygwin DOT com; run by ezmlm
|
| List-Id: | <cygwin.cygwin.com>
|
| List-Subscribe: | <mailto:cygwin-subscribe AT cygwin DOT com>
|
| List-Archive: | <http://sourceware.org/ml/cygwin/>
|
| List-Post: | <mailto:cygwin AT cygwin DOT com>
|
| List-Help: | <mailto:cygwin-help AT cygwin DOT com>, <http://sourceware.org/ml/#faqs>
|
| Sender: | cygwin-owner AT cygwin DOT com
|
| Mail-Followup-To: | cygwin AT cygwin DOT com
|
| Delivered-To: | mailing list cygwin AT cygwin DOT com
|
| Authentication-Results: | sourceware.org; auth=none
|
| X-Virus-Found: | No
|
| X-Spam-SWARE-Status: | No, score=-4.5 required=5.0 tests=AWL,BAYES_20,KAM_LAZY_DOMAIN_SECURITY autolearn=no version=3.3.2
|
| X-HELO: | calimero.vinschen.de
|
| Date: | Tue, 8 Sep 2015 10:58:54 +0200
|
| From: | Corinna Vinschen <corinna-cygwin AT cygwin DOT com>
|
| To: | cygwin AT cygwin DOT com
|
| Subject: | Re: Question about flock - potential memory corruption?
|
| Message-ID: | <20150908085854.GB24811@calimero.vinschen.de>
|
| Reply-To: | cygwin AT cygwin DOT com
|
| Mail-Followup-To: | cygwin AT cygwin DOT com
|
| References: | <CALd+sZRo_Nyv=adF5DeeHiShJsxGD+KUPqkDMKb3q47a2Nm=8Q AT mail DOT gmail DOT com>
|
| MIME-Version: | 1.0
|
| In-Reply-To: | <CALd+sZRo_Nyv=adF5DeeHiShJsxGD+KUPqkDMKb3q47a2Nm=8Q@mail.gmail.com>
|
| User-Agent: | Mutt/1.5.23 (2014-03-12)
|
--L6iaP+gRLNZHKoI4
Content-Type: multipart/mixed; boundary="z6Eq5LdranGa6ru8"
Content-Disposition: inline
--z6Eq5LdranGa6ru8
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
Hi Qian,
sorry for the late reply, somehow I missed this mail.
On Sep 5 04:22, Qian Hong wrote:
> Dear list,
>=20
> When testing Cygwin/MSYS2 on Wine, I found randomly failure of flock():
> https://bugs.wine-staging.com/show_bug.cgi?id=3D466#c13
>=20
> I ran MSYS2 with Wine+Valgrind, and found warnings like below when
> calling flock():
I'd still be glad to stick to upstream Cygwin, but, oh well.
> 7 =3D=3D19315=3D=3D Conditional jump or move depends on uninitialised v=
alue(s)
> 8 =3D=3D19315=3D=3D at 0x7BC82750: RtlGetOwnerSecurityDescriptor (se=
c.c:740)
> 9 =3D=3D19315=3D=3D by 0x7BC9222A: NTDLL_create_struct_sd (sync.c:96)
> 10 =3D=3D19315=3D=3D by 0x7BC92CE4: NtCreateEvent (sync.c:294)
> 11 =3D=3D19315=3D=3D by 0x6107B687: ???
> 12 =3D=3D19315=3D=3D by 0x612FC347: ???
>=20
> Then I read Cygwin/MSYS2 source code, and found:
>=20
> --- snip ---
> extern PSECURITY_DESCRIPTOR _everyone_sd (void *buf, ACCESS_MASK access);
> #define everyone_sd(access) (_everyone_sd (alloca (SD_MIN_SIZE), (access)=
))
> --- snip ---
>=20
> 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.
>=20
> However, Cygwin/MSYS2 seems passed a freed pointer to NtCreateEvent:
> https://cygwin.com/git/gitweb.cgi?p=3Dnewlib-cygwin.git;a=3Dblob;f=3Dwins=
up/cygwin/flock.cc;h=3D2332f5467e37d124acfd12c0f85a30281f10a952;hb=3DHEAD#l=
773
>=20
> 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 }
>=20
> 772 status =3D NtCreateEvent (&lf_obj, CYG_EVENT_ACCESS,
> 773 create_lock_obj_attr (&attr, OBJ_INHER=
IT),
> 774 NotificationEvent, FALSE);
Yuk! You're right. This is certainly a bug.
> 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.
>=20
> I write a hack as attachment 0001-cygwin-flock-user-static-buffer.txt
> and recompile MSYS2, then the bug seems go away.
Right, but you can't use a static buffer here.
> 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.
Please try the attached patch.
Thanks,
Corinna
--=20
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat
--z6Eq5LdranGa6ru8
Content-Type: text/plain; charset=utf-8
Content-Disposition: attachment; filename="0001-flock.cc-Fix-stack-allocation-from-callee-used-in-ca.patch"
Content-Transfer-Encoding: quoted-printable
=46rom 51d38004b2f51ac659f7ccc663c222f5ffe24b80 Mon Sep 17 00:00:00 2001
From: Corinna Vinschen <corinna AT vinschen DOT de>
Date: Tue, 8 Sep 2015 10:57:54 +0200
Subject: [PATCH] flock.cc: Fix stack allocation from callee used in caller
* flock.cc (lockf_t::create_lock_obj_attr): Add buffer parameter.
Call _everyone_sd with buffer argument from caller rather than
everyone_sd with locally allocated stack buffer.
(lockf_t::create_lock_obj): Call create_lock_obj_attr only once
outside the loop and with additional buffer argument.
(lockf_t::open_lock_obj): Call create_lock_obj_attr with additional
buffer argument.
---
winsup/cygwin/flock.cc | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/winsup/cygwin/flock.cc b/winsup/cygwin/flock.cc
index 2332f54..f26a76a 100644
--- a/winsup/cygwin/flock.cc
+++ b/winsup/cygwin/flock.cc
@@ -290,7 +290,7 @@ class lockf_t
{ cfree (p); }
=20
POBJECT_ATTRIBUTES create_lock_obj_attr (lockfattr_t *attr,
- ULONG flags);
+ ULONG flags, void *sd_buf);
=20
void create_lock_obj ();
bool open_lock_obj ();
@@ -636,7 +636,7 @@ inode_t::get_all_locks_list ()
/* Create the lock object name. The name is constructed from the lock
properties which identify it uniquely, all values in hex. */
POBJECT_ATTRIBUTES
-lockf_t::create_lock_obj_attr (lockfattr_t *attr, ULONG flags)
+lockf_t::create_lock_obj_attr (lockfattr_t *attr, ULONG flags, void *sd_bu=
f)
{
__small_swprintf (attr->name, LOCK_OBJ_NAME_FMT,
lf_flags & (F_POSIX | F_FLOCK), lf_type, lf_start, lf_end,
@@ -644,7 +644,7 @@ lockf_t::create_lock_obj_attr (lockfattr_t *attr, ULONG=
flags)
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 (sd_buf, FLOCK_EVENT_ACCESS));
return &attr->attr;
}
=20
@@ -766,11 +766,13 @@ lockf_t::create_lock_obj ()
{
lockfattr_t attr;
NTSTATUS status;
+ POBJECT_ATTRIBUTES lock_obj_attr;
=20
+ lock_obj_attr =3D create_lock_obj_attr (&attr, OBJ_INHERIT,
+ alloca (SD_MIN_SIZE));
do
{
- status =3D NtCreateEvent (&lf_obj, CYG_EVENT_ACCESS,
- create_lock_obj_attr (&attr, OBJ_INHERIT),
+ status =3D NtCreateEvent (&lf_obj, CYG_EVENT_ACCESS, lock_obj_attr,
NotificationEvent, FALSE);
if (!NT_SUCCESS (status))
{
@@ -852,7 +854,7 @@ lockf_t::open_lock_obj ()
NTSTATUS status;
=20
status =3D NtOpenEvent (&lf_obj, FLOCK_EVENT_ACCESS,
- create_lock_obj_attr (&attr, 0));
+ create_lock_obj_attr (&attr, 0, alloca (SD_MIN_SIZE)));
if (!NT_SUCCESS (status))
{
SetLastError (RtlNtStatusToDosError (status));
--=20
2.1.0
--z6Eq5LdranGa6ru8--
--L6iaP+gRLNZHKoI4
Content-Type: application/pgp-signature
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQIcBAEBCAAGBQJV7qNOAAoJEPU2Bp2uRE+gDIwP/3/7qngEXk24Sl8WNBmO71e7
icrLINR2QV6gaLJCmmiUmgwCA0WsteoIfz8qHshrC9R58J+9ZJvZ/5a0++8Uinvb
hiMLONJ3JljqN2eaQbNF5RzxCp3h2cChbkkm7GMD6cDtW5LfbcnQuPWE22gNdD4w
AtpFO3pP5V8CaTLwVj+1qKZwBrJRzviaPHoMQScw0R6CwiH/KLWGX2RYH7YR9PIr
kLbMM+EzVdX2LXnfwGN05uPevE6DbylhBNR9IIIIQDNXP4G2c2iaygqKmBTnNU8v
Y2FyNHgGknlT/az7OWQWBGNYrwm/XtLVQQVIfnc2CrhkzMN1TKb291AjtFoVRRGZ
sVGdHrEM6NsPRzpgVI9qE6CcNIUZG5ajjd+fzYKHeZUSE5DLSShIuqcKFHJmQea9
WikEoIHhomXGKgDh1aKQQnCcjRNuoDy0nuyaxyUKrDk6uejRcOW1xB6jIB3D+63z
rbrv3Tr7USd9bqmoZ9jJ2FBCBkfbzrcA4pFqk/qXeJDRJnWPs/9W4D2N3Z68X4xH
Of5Ize3Dck0X25qGk3AoETS3NPEImKqoCNZFfJhRn1nq4IuEGzD48pgC2c3V910T
lj455Agh5b2+4xMbcRjqX9gOVHDJHh9sIjvfXQ5IuzZBYfC/EPAY+RRoD56KrJH3
ly15EdQuV9e9DHLBNw2r
=pzTG
-----END PGP SIGNATURE-----
--L6iaP+gRLNZHKoI4--
- Raw text -