On Fri, 21 Nov 2025 12:21:04 +0100
Corinna Vinschen wrote:
> On Nov 21 19:00, Takashi Yano via Cygwin wrote:
> > On Fri, 14 Nov 2025 11:27:04 -0800
> > Nahor wrote:
> > > If `flock()` was used on the same file descriptor, then this might
> > > have been a valid point. However, each thread has its own file
> > > descriptor in this case, so this would be very surprising if it wasn't
> > > thread-safe.
> >
> > IIUC, flock() locks file itself, but not file descriptor. Usually,
> > flock() is used for inter-process file protection, isn't it?
> >
> > > Moreover, it's not just `flock()` failing, it's also (and mostly!)
> > > `open()` that fails. And it's the `open()` for a completely different
> > > file than the one being locked. So that would suggest that `open()` is
> > > not also not MT-safe. And not safe when using different files. And not
> > > safe across multiple different functions (flock+open).
> >
> > Indeed, this is really weird. I looked into this, and found 'upath' in
> > path.cc is destroyed after 'NtCreateFile()' call at the following line.
> >
> > I added assertion as follows:
> >
> > diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
> > index 710775e38..562100161 100644
> > --- a/winsup/cygwin/path.cc
> > +++ b/winsup/cygwin/path.cc
> > @@ -3189,6 +3189,8 @@ restart:
> > symlink (which would spoil the task of this method quite a bit).
> > Fortunately it's ignored on most other file systems so we don't have
> > to special case NFS too much. */
> > + wchar_t c;
> > + c = upath.Buffer[0];
> > status = NtCreateFile (&h,
> > READ_CONTROL | FILE_READ_ATTRIBUTES | FILE_READ_EA,
> > &attr, &io, NULL, 0, FILE_SHARE_VALID_FLAGS,
> > @@ -3196,6 +3198,7 @@ restart:
> > FILE_OPEN_REPARSE_POINT
> > | FILE_OPEN_FOR_BACKUP_INTENT,
> > eabuf, easize);
> > + assert (upath.Buffer[0] == c);
> > debug_printf ("%y = NtCreateFile (%S)", status, &upath);
> > /* No right to access EAs or EAs not supported? */
> > if (!NT_SUCCESS (status)
> >
> > then, the assertion fails for your test case like:
> > tmp_dir: /tmp/flockAQ4Hbb
> > assertion "upath.Buffer[0] == c" failed: file
> > "../../.././winsup/cygwin/path.cc", line 3201, function: int
> > symlink_info::check(char*, const suffix_info*, fs_info&, path_conv_handle&)
> > Abort
> >
> > Does another thread destroy the puthbuf? But pathbuf is thread local, IIUC.
> > Corinna, have you noticed anything?
>
> No, I haven't. The tmp_pathbuf buffers are malloced and reused, but they
> are only ever used in the same thread. So afaics, either the buffer gets
> incorrect stored in a global datastructure and overwritten, or there's
> a buffer overflow in the allocation preceeding the upath.Buffer. That
> could be an application allocation just as well as a DLL allocation.
I found the cause. In flock.cc, lf_setlock() may access tmp_pathbuf
that is already released in lf_clearlock(). See:
https://cygwin.com/pipermail/cygwin-patches/2025q4/014366.html
--
Takashi Yano <[email protected]>
--
Problem reports: https://cygwin.com/problems.html
FAQ: https://cygwin.com/faq/
Documentation: https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple
