Bump MIN_WINNT to 0x0600 (Vista) as minimal runtime in 16~
Hi all,
On the thread about the removal of VS 2013, Jose (in CC) has mentioned
that bumping MIN_WINNT independently would make sense, as the
simplication of locales would expose under MinGW some code for
GetLocaleInfoEx():
/messages/by-id/CAC+AXB3himFH+-pGRO1cYju6zF2hLH6VmwPbf5RAytF1UBm_nw@mail.gmail.com
Attached is a patch to set MIN_WINNT, the minimal version of Windows
allowed at run-time to 0x0600 for all environments, aka Vista. This
results in removing the support for XP at run-time when compiling with
anything else than VS >= 2015 (VS 2013, MinGW, etc.). We could cut
things more, I hope, but this bump makes sense in itself with the
business related to locales.
What I would like to do is to apply that at the beginning of the dev
cycle for v16, in parallel of the removal of VS 2013. This move is
rather independent of the other thread, which is why I am spawning a
new one here. And it is better than having to dig into the other
thread for a change like that.
Thoughts or opinions?
--
Michael
Attachments:
0001-Bump-WIN_MINNT-to-0x0600-everywhere.patchtext/x-diff; charset=us-asciiDownload
From 26da923e3007e7eb1b459826158623c9db47e983 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Thu, 26 May 2022 10:20:06 +0900
Subject: [PATCH] Bump WIN_MINNT to 0x0600 everywhere
---
src/include/port/win32.h | 10 +++-------
src/backend/utils/adt/pg_locale.c | 4 ++--
doc/src/sgml/installation.sgml | 2 +-
3 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/src/include/port/win32.h b/src/include/port/win32.h
index c6213c77c3..4d24c46812 100644
--- a/src/include/port/win32.h
+++ b/src/include/port/win32.h
@@ -11,15 +11,11 @@
/*
* Make sure _WIN32_WINNT has the minimum required value.
- * Leave a higher value in place. When building with at least Visual
- * Studio 2015 the minimum requirement is Windows Vista (0x0600) to
- * get support for GetLocaleInfoEx() with locales. For everything else
- * the minimum version is Windows XP (0x0501).
+ * Leave a higher value in place. The minimum requirement is Windows Vista
+ * (0x0600) to get support for GetLocaleInfoEx() with locales.
*/
-#if defined(_MSC_VER) && _MSC_VER >= 1900
+#if defined(_MSC_VER)
#define MIN_WINNT 0x0600
-#else
-#define MIN_WINNT 0x0501
#endif
#if defined(_WIN32_WINNT) && _WIN32_WINNT < MIN_WINNT
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index a0490a7522..4c39841b99 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -1729,7 +1729,7 @@ get_collation_actual_version(char collprovider, const char *collcollate)
else
ereport(ERROR,
(errmsg("could not load locale \"%s\"", collcollate)));
-#elif defined(WIN32) && _WIN32_WINNT >= 0x0600
+#elif defined(WIN32)
/*
* If we are targeting Windows Vista and above, we can ask for a name
* given a collation name (earlier versions required a location code
@@ -1757,7 +1757,7 @@ get_collation_actual_version(char collprovider, const char *collcollate)
collcollate,
GetLastError())));
}
- collversion = psprintf("%d.%d,%d.%d",
+ collversion = psprintf("%ld.%ld,%ld.%ld",
(version.dwNLSVersion >> 8) & 0xFFFF,
version.dwNLSVersion & 0xFF,
(version.dwDefinedVersion >> 8) & 0xFFFF,
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index c585078029..5e2541137b 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -2136,7 +2136,7 @@ export MANPATH
<para>
<productname>PostgreSQL</productname> can be expected to work on these operating
- systems: Linux (all recent distributions), Windows (XP and later),
+ systems: Linux (all recent distributions), Windows (Vista and later),
FreeBSD, OpenBSD, NetBSD, macOS, AIX, HP/UX, and Solaris.
Other Unix-like systems may also work but are not currently
being tested. In most cases, all CPU architectures supported by
--
2.36.1
On Thu, May 26, 2022 at 2:59 PM Michael Paquier <michael@paquier.xyz> wrote:
On the thread about the removal of VS 2013, Jose (in CC) has mentioned
that bumping MIN_WINNT independently would make sense, as the
simplication of locales would expose under MinGW some code for
GetLocaleInfoEx():
/messages/by-id/CAC+AXB3himFH+-pGRO1cYju6zF2hLH6VmwPbf5RAytF1UBm_nw@mail.gmail.comAttached is a patch to set MIN_WINNT, the minimal version of Windows
allowed at run-time to 0x0600 for all environments, aka Vista. This
results in removing the support for XP at run-time when compiling with
anything else than VS >= 2015 (VS 2013, MinGW, etc.). We could cut
things more, I hope, but this bump makes sense in itself with the
business related to locales.What I would like to do is to apply that at the beginning of the dev
cycle for v16, in parallel of the removal of VS 2013. This move is
rather independent of the other thread, which is why I am spawning a
new one here. And it is better than having to dig into the other
thread for a change like that.Thoughts or opinions?
I think we should drop everything older than Win 10 for PG16, as
argued in various threads where various pain points came up. For one
thing, that would make a lot of future work simpler (ie not needing to
test alternative code paths on dead computers without CI or BF, AKA
dead code), and also I don't think we really help anyone by allowing
new database deployments on operating systems that aren't receiving
vendor patches on the world's most attacked operating system. Doing
it incrementally is fine by me, too, though, if it makes the patches
and discussions easier...
On Thu, May 26, 2022 at 04:16:40PM +1200, Thomas Munro wrote:
I think we should drop everything older than Win 10 for PG16, as
argued in various threads where various pain points came up. For one
thing, that would make a lot of future work simpler (ie not needing to
test alternative code paths on dead computers without CI or BF, AKA
dead code), and also I don't think we really help anyone by allowing
new database deployments on operating systems that aren't receiving
vendor patches on the world's most attacked operating system. Doing
it incrementally is fine by me, too, though, if it makes the patches
and discussions easier...
Is there anything posted recently that would require that? Perhaps
the async work? FWIW, I agree to be much more aggressive, but there
is nothing in the tree now that depends on _WIN32_WINNT, except one
change for the locales.
--
Michael
On Thu, May 26, 2022 at 6:27 AM Michael Paquier <michael@paquier.xyz> wrote:
Is there anything posted recently that would require that? Perhaps
the async work? FWIW, I agree to be much more aggressive, but there
is nothing in the tree now that depends on _WIN32_WINNT, except one
change for the locales.
There have been a couple of discussions involving not only Windows
version10, but also the Release id:
https://commitfest.postgresql.org/38/3347/
https://commitfest.postgresql.org/38/3530/
/messages/by-id/6389b5a88e114bee85593af2853c08cd@dental-vision.de
Maybe this thread can push those others forward.
Regards,
Juan José Santamaría Flecha
Show quoted text
On Thu, May 26, 2022 at 4:27 PM Michael Paquier <michael@paquier.xyz> wrote:
Perhaps the async work?
(Checks code...) Looks like the experimental Windows native AIO code
we have today, namely io_method=windows_iocp, only needs Vista.
That's for GetQueueCompletionStatusEx() (before that you had to call
GetQueuedCompletionStatus() in a loop to read multiple IO completion
events from an IOCP), and otherwise it's all just ancient Windows
"overlapped" stuff. Not sure if we'll propose that io_method, or skip
directly to the new io_uring-style API that appeared in Win 11 (not
yet tried), or propose both as long as Win 10 is around, or ... I
dunno.
On Thu, May 26, 2022 at 10:17:08AM +0200, Juan José Santamaría Flecha wrote:
There have been a couple of discussions involving not only Windows
version10, but also the Release id:
This mentions 0x0A00, aka Windows 10, for atomic rename support.
Similarly 0x0A00, aka Windows 10, for fdatasync().
/messages/by-id/6389b5a88e114bee85593af2853c08cd@dental-vision.de
And Windows 10 1703, for large pages.
Maybe this thread can push those others forward.
Post Windows 8, the most annoying part is that manifests are required
to be able to check at run-time on which version you are running with
routines like IsWindowsXXOrGreater() if you compiled with a threshold
of MIN_WINNT lower than the version you expect compatibility for.
And each one of those things would mean cutting a lot of past support
if we want to eliminate the manifest part. Windows 8 ends its support
in 2023, it seems, so that sounds short even for PG16.
--
Michael
On Fri, May 27, 2022 at 3:53 PM Michael Paquier <michael@paquier.xyz> wrote:
Windows 8 ends its support
in 2023, it seems, so that sounds short even for PG16.
I guess you meant 8.1 here, and corresponding server release 2012 R2.
These will come to the end of their "extended" support phase in 2023,
before PG16 comes out. If I understand correctly (and I'm not a
Windows user, I just googled this), they will start showing blue
full-screen danger-Will-Robinson alerts about viruses and malware.
Why would we have explicit support for that in a new release? Do we
want people putting their users' data in such a system? Can you go to
GDPR jail for that in Europe? (Joking, I think).
We should go full Marie Kondo on EOL'd OSes that are not in our CI or
build farm, IMHO.
On 27 May 2022, at 23:07, Thomas Munro <thomas.munro@gmail.com> wrote:
We should go full Marie Kondo on EOL'd OSes that are not in our CI or
build farm, IMHO.
FWIW, +1
--
Daniel Gustafsson https://vmware.com/
On Sat, May 28, 2022 at 05:30:51PM +0200, Daniel Gustafsson wrote:
On 27 May 2022, at 23:07, Thomas Munro <thomas.munro@gmail.com> wrote:
We should go full Marie Kondo on EOL'd OSes that are not in our CI or
build farm, IMHO.FWIW, +1
Okay, I am outnumbered, and that would mean bumping MIN_WINNT to
0x0A00. So, ready to move to this version at full speed for 16? We
still have a couple of weeks ahead before the next dev cycle begins,
so feel free to comment, of course.
--
Michael
On Mon, May 30, 2022 at 03:59:52PM +0900, Michael Paquier wrote:
Okay, I am outnumbered, and that would mean bumping MIN_WINNT to
0x0A00. So, ready to move to this version at full speed for 16? We
still have a couple of weeks ahead before the next dev cycle begins,
so feel free to comment, of course.
And attached is an updated patch to do exactly that.
--
Michael
Attachments:
v2-0001-Bump-WIN_MINNT-to-0x0A00-everywhere-Windows-10.patchtext/x-diff; charset=us-asciiDownload
From 6ef8fbe7a4ef1c3f63d1d1b578f0d85129c07798 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Thu, 9 Jun 2022 12:54:07 +0900
Subject: [PATCH v2] Bump WIN_MINNT to 0x0A00 everywhere (Windows 10~)
---
src/include/port/win32.h | 11 +++--------
src/backend/utils/adt/pg_locale.c | 4 ++--
doc/src/sgml/installation.sgml | 2 +-
3 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/src/include/port/win32.h b/src/include/port/win32.h
index c6213c77c3..fe0829cedc 100644
--- a/src/include/port/win32.h
+++ b/src/include/port/win32.h
@@ -11,15 +11,10 @@
/*
* Make sure _WIN32_WINNT has the minimum required value.
- * Leave a higher value in place. When building with at least Visual
- * Studio 2015 the minimum requirement is Windows Vista (0x0600) to
- * get support for GetLocaleInfoEx() with locales. For everything else
- * the minimum version is Windows XP (0x0501).
+ * Leave a higher value in place. The minimum requirement is Windows 10.
*/
-#if defined(_MSC_VER) && _MSC_VER >= 1900
-#define MIN_WINNT 0x0600
-#else
-#define MIN_WINNT 0x0501
+#if defined(_MSC_VER)
+#define MIN_WINNT 0x0A00
#endif
#if defined(_WIN32_WINNT) && _WIN32_WINNT < MIN_WINNT
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index a0490a7522..4c39841b99 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -1729,7 +1729,7 @@ get_collation_actual_version(char collprovider, const char *collcollate)
else
ereport(ERROR,
(errmsg("could not load locale \"%s\"", collcollate)));
-#elif defined(WIN32) && _WIN32_WINNT >= 0x0600
+#elif defined(WIN32)
/*
* If we are targeting Windows Vista and above, we can ask for a name
* given a collation name (earlier versions required a location code
@@ -1757,7 +1757,7 @@ get_collation_actual_version(char collprovider, const char *collcollate)
collcollate,
GetLastError())));
}
- collversion = psprintf("%d.%d,%d.%d",
+ collversion = psprintf("%ld.%ld,%ld.%ld",
(version.dwNLSVersion >> 8) & 0xFFFF,
version.dwNLSVersion & 0xFF,
(version.dwDefinedVersion >> 8) & 0xFFFF,
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index c585078029..7e6e7cc6bd 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -2136,7 +2136,7 @@ export MANPATH
<para>
<productname>PostgreSQL</productname> can be expected to work on these operating
- systems: Linux (all recent distributions), Windows (XP and later),
+ systems: Linux (all recent distributions), Windows (10 and later),
FreeBSD, OpenBSD, NetBSD, macOS, AIX, HP/UX, and Solaris.
Other Unix-like systems may also work but are not currently
being tested. In most cases, all CPU architectures supported by
--
2.36.1
On Thu, Jun 9, 2022 at 3:55 PM Michael Paquier <michael@paquier.xyz> wrote:
On Mon, May 30, 2022 at 03:59:52PM +0900, Michael Paquier wrote:
Okay, I am outnumbered, and that would mean bumping MIN_WINNT to
0x0A00. So, ready to move to this version at full speed for 16? We
still have a couple of weeks ahead before the next dev cycle begins,
so feel free to comment, of course.And attached is an updated patch to do exactly that.
<productname>PostgreSQL</productname> can be expected to work on
these operating
- systems: Linux (all recent distributions), Windows (XP and later),
+ systems: Linux (all recent distributions), Windows (10 and later),
FreeBSD, OpenBSD, NetBSD, macOS, AIX, HP/UX, and Solaris.Cool. (I'm not sure what "all recent distributions" contributes but
that's not from your patch...).
The Cygwin stuff in installation.sgml also mentions NT, 2000, XP, but
it's not clear from the phrasing if it meant "and later" or "and
earlier", so I'm not sure if it needs adjusting or removing...
While looking for more stuff to vacuum, I found this:
<title>Special Considerations for 64-Bit Windows</title>
<para>
PostgreSQL will only build for the x64 architecture on 64-bit Windows, there
is no support for Itanium processors.
</para>
I think we can drop mention of Itanium (RIP): the ancient versions of
Windows that could run on that arch are desupported with your patch.
It might be more relevant to say that we can't yet run on ARM, and
Windows 11 is untested by us, but let's fix those problems instead of
documenting them :-)
Is the mention of "64-Bit" in that title sounds a little anachronistic
to me (isn't that the norm now?) but I'm not sure what to suggest.
There are more mentions of older Windows releases near the compiler
stuff but perhaps your MSVC version vacuuming work will take care of
those.
On Thu, Jun 09, 2022 at 04:55:45PM +1200, Thomas Munro wrote:
The Cygwin stuff in installation.sgml also mentions NT, 2000, XP, but
it's not clear from the phrasing if it meant "and later" or "and
earlier", so I'm not sure if it needs adjusting or removing...
Right. We could just remove the entire mention to "NT, 2000 or XP"
instead? There would be no loss in clarity IMO.
I think we can drop mention of Itanium (RIP): the ancient versions of
Windows that could run on that arch are desupported with your patch.
It might be more relevant to say that we can't yet run on ARM, and
Windows 11 is untested by us, but let's fix those problems instead of
documenting them :-)
Okay to remove the Itanium part for me.
Is the mention of "64-Bit" in that title sounds a little anachronistic
to me (isn't that the norm now?) but I'm not sure what to suggest.
Not sure. I think that I would leave this part alone for now.
There are more mentions of older Windows releases near the compiler
stuff but perhaps your MSVC version vacuuming work will take care of
those.
Yes, I have a few changes like the one in main.c for _M_AMD64. Are
you referring to something else?
--
Michael
On Thu, Jun 09, 2022 at 02:47:36PM +0900, Michael Paquier wrote:
On Thu, Jun 09, 2022 at 04:55:45PM +1200, Thomas Munro wrote:
I think we can drop mention of Itanium (RIP): the ancient versions of
Windows that could run on that arch are desupported with your patch.
It might be more relevant to say that we can't yet run on ARM, and
Windows 11 is untested by us, but let's fix those problems instead of
documenting them :-)Okay to remove the Itanium part for me.
install-windows.sgml has one extra spot mentioning Windows 7 and
Server 2008 that can be simplified on top of that.
There are more mentions of older Windows releases near the compiler
stuff but perhaps your MSVC version vacuuming work will take care of
those.Yes, I have a few changes like the one in main.c for _M_AMD64. Are
you referring to something else?
Actually, this can go with the bump of MIN_WINNT as it uses one of the
IsWindows*OrGreater() macros as a runtime check. And there are two
more places in pg_ctl.c that can be similarly cleaned up.
It is possible that I have missed some spots, of course.
--
Michael
Attachments:
v3-0001-Bump-WIN_MINNT-to-0x0A00-everywhere-Windows-10.patchtext/x-diff; charset=us-asciiDownload
From 2d51b7dd04e44ace0294531878676ff89e465c0b Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Thu, 16 Jun 2022 15:11:53 +0900
Subject: [PATCH v3] Bump WIN_MINNT to 0x0A00 everywhere (Windows 10~)
---
src/include/port/win32.h | 11 +++--------
src/backend/main/main.c | 17 -----------------
src/backend/utils/adt/pg_locale.c | 4 ++--
src/bin/pg_ctl/pg_ctl.c | 26 ++------------------------
doc/src/sgml/install-windows.sgml | 9 ++-------
doc/src/sgml/installation.sgml | 9 ++++-----
6 files changed, 13 insertions(+), 63 deletions(-)
diff --git a/src/include/port/win32.h b/src/include/port/win32.h
index c6213c77c3..fe0829cedc 100644
--- a/src/include/port/win32.h
+++ b/src/include/port/win32.h
@@ -11,15 +11,10 @@
/*
* Make sure _WIN32_WINNT has the minimum required value.
- * Leave a higher value in place. When building with at least Visual
- * Studio 2015 the minimum requirement is Windows Vista (0x0600) to
- * get support for GetLocaleInfoEx() with locales. For everything else
- * the minimum version is Windows XP (0x0501).
+ * Leave a higher value in place. The minimum requirement is Windows 10.
*/
-#if defined(_MSC_VER) && _MSC_VER >= 1900
-#define MIN_WINNT 0x0600
-#else
-#define MIN_WINNT 0x0501
+#if defined(_MSC_VER)
+#define MIN_WINNT 0x0A00
#endif
#if defined(_WIN32_WINNT) && _WIN32_WINNT < MIN_WINNT
diff --git a/src/backend/main/main.c b/src/backend/main/main.c
index c43a527d3f..dd82722ee3 100644
--- a/src/backend/main/main.c
+++ b/src/backend/main/main.c
@@ -290,23 +290,6 @@ startup_hacks(const char *progname)
_CrtSetReportFile(_CRT_ASSERT, _CRTDBG_FILE_STDERR);
_CrtSetReportMode(_CRT_WARN, _CRTDBG_MODE_FILE | _CRTDBG_MODE_DEBUG);
_CrtSetReportFile(_CRT_WARN, _CRTDBG_FILE_STDERR);
-
-#if defined(_M_AMD64) && _MSC_VER == 1800
-
- /*----------
- * Avoid crashing in certain floating-point operations if we were
- * compiled for x64 with MS Visual Studio 2013 and are running on
- * Windows prior to 7/2008R2 SP1 on an AVX2-capable CPU.
- *
- * Ref: https://connect.microsoft.com/VisualStudio/feedback/details/811093/visual-studio-2013-rtm-c-x64-code-generation-bug-for-avx2-instructions
- *----------
- */
- if (!IsWindows7SP1OrGreater())
- {
- _set_FMA3_enable(0);
- }
-#endif /* defined(_M_AMD64) && _MSC_VER == 1800 */
-
}
#endif /* WIN32 */
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index a0490a7522..4c39841b99 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -1729,7 +1729,7 @@ get_collation_actual_version(char collprovider, const char *collcollate)
else
ereport(ERROR,
(errmsg("could not load locale \"%s\"", collcollate)));
-#elif defined(WIN32) && _WIN32_WINNT >= 0x0600
+#elif defined(WIN32)
/*
* If we are targeting Windows Vista and above, we can ask for a name
* given a collation name (earlier versions required a location code
@@ -1757,7 +1757,7 @@ get_collation_actual_version(char collprovider, const char *collcollate)
collcollate,
GetLastError())));
}
- collversion = psprintf("%d.%d,%d.%d",
+ collversion = psprintf("%ld.%ld,%ld.%ld",
(version.dwNLSVersion >> 8) & 0xFFFF,
version.dwNLSVersion & 0xFF,
(version.dwDefinedVersion >> 8) & 0xFFFF,
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index dd78e5bc66..ef58883a5c 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -1896,17 +1896,8 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo, bool as_ser
/* Verify that we found all functions */
if (_IsProcessInJob == NULL || _CreateJobObject == NULL || _SetInformationJobObject == NULL || _AssignProcessToJobObject == NULL || _QueryInformationJobObject == NULL)
{
- /*
- * IsProcessInJob() is not available on < WinXP, so there is no need
- * to log the error every time in that case
- */
- if (IsWindowsXPOrGreater())
-
- /*
- * Log error if we can't get version, or if we're on WinXP/2003 or
- * newer
- */
- write_stderr(_("%s: WARNING: could not locate all job object functions in system API\n"), progname);
+ /* Log error if we can't get version */
+ write_stderr(_("%s: WARNING: could not locate all job object functions in system API\n"), progname);
}
else
{
@@ -1946,19 +1937,6 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo, bool as_ser
JOB_OBJECT_UILIMIT_EXITWINDOWS | JOB_OBJECT_UILIMIT_READCLIPBOARD |
JOB_OBJECT_UILIMIT_SYSTEMPARAMETERS | JOB_OBJECT_UILIMIT_WRITECLIPBOARD;
- if (as_service)
- {
- if (!IsWindows7OrGreater())
- {
- /*
- * On Windows 7 (and presumably later),
- * JOB_OBJECT_UILIMIT_HANDLES prevents us from
- * starting as a service. So we only enable it on
- * Vista and earlier (version <= 6.0)
- */
- uiRestrictions.UIRestrictionsClass |= JOB_OBJECT_UILIMIT_HANDLES;
- }
- }
_SetInformationJobObject(job, JobObjectBasicUIRestrictions, &uiRestrictions, sizeof(uiRestrictions));
securityLimit.SecurityLimitFlags = JOB_OBJECT_SECURITY_NO_ADMIN | JOB_OBJECT_SECURITY_ONLY_TOKEN;
diff --git a/doc/src/sgml/install-windows.sgml b/doc/src/sgml/install-windows.sgml
index bcfd5a1a10..66567d17a0 100644
--- a/doc/src/sgml/install-windows.sgml
+++ b/doc/src/sgml/install-windows.sgml
@@ -82,11 +82,7 @@
as well as standalone Windows SDK releases 8.1a to 10.
64-bit PostgreSQL builds are supported with
<productname>Microsoft Windows SDK</productname> version 8.1a to 10 or
- <productname>Visual Studio 2013</productname> and above. Compilation
- is supported down to <productname>Windows 7</productname> and
- <productname>Windows Server 2008 R2 SP1</productname> when building with
- <productname>Visual Studio 2013</productname> to
- <productname>Visual Studio 2022</productname>.
+ <productname>Visual Studio 2013</productname> and above.
<!--
For 2013 requirements:
https://docs.microsoft.com/en-us/visualstudio/productinfo/vs2013-sysrequirements-vs
@@ -358,8 +354,7 @@ $ENV{MSBFLAGS}="/m";
<title>Special Considerations for 64-Bit Windows</title>
<para>
- PostgreSQL will only build for the x64 architecture on 64-bit Windows, there
- is no support for Itanium processors.
+ PostgreSQL will only build for the x64 architecture on 64-bit Windows.
</para>
<para>
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index c585078029..c12de7896c 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -2136,7 +2136,7 @@ export MANPATH
<para>
<productname>PostgreSQL</productname> can be expected to work on these operating
- systems: Linux (all recent distributions), Windows (XP and later),
+ systems: Linux (all recent distributions), Windows (10 and later),
FreeBSD, OpenBSD, NetBSD, macOS, AIX, HP/UX, and Solaris.
Other Unix-like systems may also work but are not currently
being tested. In most cases, all CPU architectures supported by
@@ -2323,16 +2323,15 @@ ERROR: could not load library "/opt/dbs/pgsql/lib/plperl.so": Bad address
<listitem>
<para>
The <command>adduser</command> command is not supported; use
- the appropriate user management application on Windows NT,
- 2000, or XP. Otherwise, skip this step.
+ the appropriate user management application on Windows.
+ Otherwise, skip this step.
</para>
</listitem>
<listitem>
<para>
The <command>su</command> command is not supported; use ssh to
- simulate su on Windows NT, 2000, or XP. Otherwise, skip this
- step.
+ simulate su on Windows. Otherwise, skip this step.
</para>
</listitem>
--
2.36.1
On Thu, Jun 16, 2022 at 03:14:16PM +0900, Michael Paquier wrote:
Actually, this can go with the bump of MIN_WINNT as it uses one of the
IsWindows*OrGreater() macros as a runtime check. And there are two
more places in pg_ctl.c that can be similarly cleaned up.It is possible that I have missed some spots, of course.
It does not seem to be the case on a second look. The buildfarm
animals running Windows are made of:
- hamerkop, Windows server 2016 (based on Win10 AFAIK)
- drongo, Windows server 2019
- bowerbird, Windows 10 pro
- jacana, Windows 10
- fairywren, Msys server 2019
- bichir, Ubuntu/Windows 10 mix
Now that v16 is open for business, any objections to move on with this
patch and bump MIN_WINNT to 0x0A00 on HEAD? There are follow-up items
for large pages and more.
--
Michael
On Wed, Jul 6, 2022 at 7:28 PM Michael Paquier <michael@paquier.xyz> wrote:
On Thu, Jun 16, 2022 at 03:14:16PM +0900, Michael Paquier wrote:
Actually, this can go with the bump of MIN_WINNT as it uses one of the
IsWindows*OrGreater() macros as a runtime check. And there are two
more places in pg_ctl.c that can be similarly cleaned up.It is possible that I have missed some spots, of course.
It does not seem to be the case on a second look. The buildfarm
animals running Windows are made of:
- hamerkop, Windows server 2016 (based on Win10 AFAIK)
- drongo, Windows server 2019
- bowerbird, Windows 10 pro
- jacana, Windows 10
- fairywren, Msys server 2019
- bichir, Ubuntu/Windows 10 mixNow that v16 is open for business, any objections to move on with this
patch and bump MIN_WINNT to 0x0A00 on HEAD? There are follow-up items
for large pages and more.
+1 for proceeding. This will hopefully unblock a few things, and it's
good to update our claims to match the reality of what we are actually
testing and able to debug.
The build farm also has frogmouth and currawong, 32 bit systems
running Windows XP, but they are only testing REL_10_STABLE so I
assume Andrew will decommission them in November.
On 2022-07-06 We 16:46, Thomas Munro wrote:
On Wed, Jul 6, 2022 at 7:28 PM Michael Paquier <michael@paquier.xyz> wrote:
On Thu, Jun 16, 2022 at 03:14:16PM +0900, Michael Paquier wrote:
Actually, this can go with the bump of MIN_WINNT as it uses one of the
IsWindows*OrGreater() macros as a runtime check. And there are two
more places in pg_ctl.c that can be similarly cleaned up.It is possible that I have missed some spots, of course.
It does not seem to be the case on a second look. The buildfarm
animals running Windows are made of:
- hamerkop, Windows server 2016 (based on Win10 AFAIK)
- drongo, Windows server 2019
- bowerbird, Windows 10 pro
- jacana, Windows 10
- fairywren, Msys server 2019
- bichir, Ubuntu/Windows 10 mixNow that v16 is open for business, any objections to move on with this
patch and bump MIN_WINNT to 0x0A00 on HEAD? There are follow-up items
for large pages and more.+1 for proceeding. This will hopefully unblock a few things, and it's
good to update our claims to match the reality of what we are actually
testing and able to debug.The build farm also has frogmouth and currawong, 32 bit systems
running Windows XP, but they are only testing REL_10_STABLE so I
assume Andrew will decommission them in November.
Yeah, it's not capable of supporting anything newer, so it will finally
go to sleep this year.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
On Wed, Jul 06, 2022 at 05:13:27PM -0400, Andrew Dunstan wrote:
On 2022-07-06 We 16:46, Thomas Munro wrote:
The build farm also has frogmouth and currawong, 32 bit systems
running Windows XP, but they are only testing REL_10_STABLE so I
assume Andrew will decommission them in November.Yeah, it's not capable of supporting anything newer, so it will finally
go to sleep this year.
Okay, thanks for confirming. I think that I'll give it a try today
then, my schedule would fit nicely with the buildfarm monitoring.
--
Michael
On Thu, Jul 07, 2022 at 09:11:57AM +0900, Michael Paquier wrote:
Okay, thanks for confirming. I think that I'll give it a try today
then, my schedule would fit nicely with the buildfarm monitoring.
And I have applied that, after noticing that the MinGW was complaining
because _WIN32_WINNT was not getting set like previously and removing
_WIN32_WINNT as there is no need for it anymore. The CI has reported
green for all my tests, so I am rather confident to not have messed up
something. Now, let's see what the buildfarm members tell. This
should take a couple of hours..
--
Michael
On Thu, Jul 07, 2022 at 01:56:39PM +0900, Michael Paquier wrote:
And I have applied that, after noticing that the MinGW was complaining
because _WIN32_WINNT was not getting set like previously and removing
_WIN32_WINNT as there is no need for it anymore. The CI has reported
green for all my tests, so I am rather confident to not have messed up
something. Now, let's see what the buildfarm members tell. This
should take a couple of hours..
Since this has been applied, all the Windows members have reported a
green state except for jacana and bowerbird. Based on their
environment, I would not expect any issues though I may be wrong.
Andrew, is something happening on those environments? Is 495ed0e
causing problems?
--
Michael
On Thu, Jul 07, 2022 at 01:56:39PM +0900, Michael Paquier wrote:
On Thu, Jul 07, 2022 at 09:11:57AM +0900, Michael Paquier wrote:
Okay, thanks for confirming. I think that I'll give it a try today
then, my schedule would fit nicely with the buildfarm monitoring.And I have applied that, after noticing that the MinGW was complaining
because _WIN32_WINNT was not getting set like previously and removing
_WIN32_WINNT as there is no need for it anymore. The CI has reported
green for all my tests, so I am rather confident to not have messed up
something. Now, let's see what the buildfarm members tell. This
should take a couple of hours..
If I'm not wrong, there's some lingering comments which could be removed since
495ed0ef2.
src/bin/pg_ctl/pg_ctl.c: * on NT4. That way, we don't break on NT4.
src/bin/pg_ctl/pg_ctl.c: * On NT4, or any other system not containing the required functions, will
src/bin/pg_ctl/pg_ctl.c: * NT4 doesn't have CreateRestrictedToken, so just call ordinary
src/port/dirmod.c: * Win32 (NT4 and newer).
src/backend/port/win32/socket.c: /* No error, zero bytes (win2000+) or error+WSAEWOULDBLOCK (<=nt4) */
--
Justin
On Fri, Aug 26, 2022 at 06:26:37AM -0500, Justin Pryzby wrote:
If I'm not wrong, there's some lingering comments which could be removed since
495ed0ef2.
It seems to me that you are right. I have not thought about looking
at references to NT. Good catches!
src/bin/pg_ctl/pg_ctl.c: * on NT4. That way, we don't break on NT4.
src/bin/pg_ctl/pg_ctl.c: * On NT4, or any other system not containing the required functions, will
src/bin/pg_ctl/pg_ctl.c: * NT4 doesn't have CreateRestrictedToken, so just call ordinary
src/port/dirmod.c: * Win32 (NT4 and newer).
src/backend/port/win32/socket.c: /* No error, zero bytes (win2000+) or error+WSAEWOULDBLOCK (<=nt4) */
There is also a reference to Nt4 in win32.c, for a comment that is
irrelevant now, so it can be IMO removed.
There may be a point in enforcing CreateProcess() if
CreateRestrictedToken() cannot be loaded, but that would be a security
issue if Windows goes crazy as we should always expect the function,
so this had better return an error.
So, what do you think about the attached?
--
Michael
Attachments:
win32-more-cleanup.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/backend/port/win32/socket.c b/src/backend/port/win32/socket.c
index 52944a0d33..130b60af22 100644
--- a/src/backend/port/win32/socket.c
+++ b/src/backend/port/win32/socket.c
@@ -495,7 +495,7 @@ pgwin32_send(SOCKET s, const void *buf, int len, int flags)
return -1;
}
- /* No error, zero bytes (win2000+) or error+WSAEWOULDBLOCK (<=nt4) */
+ /* No error, zero bytes */
if (pgwin32_waitforsinglesocket(s, FD_WRITE | FD_CLOSE, INFINITE) == 0)
return -1;
diff --git a/src/port/dirmod.c b/src/port/dirmod.c
index 2818bfd2e9..ae6301dd6c 100644
--- a/src/port/dirmod.c
+++ b/src/port/dirmod.c
@@ -7,7 +7,7 @@
* Portions Copyright (c) 1994, Regents of the University of California
*
* This includes replacement versions of functions that work on
- * Win32 (NT4 and newer).
+ * Windows.
*
* IDENTIFICATION
* src/port/dirmod.c
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 73e20081d1..20d2a04b7f 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -1726,9 +1726,7 @@ pgwin32_doRunAsService(void)
/*
* Mingw headers are incomplete, and so are the libraries. So we have to load
- * a whole lot of API functions dynamically. Since we have to do this anyway,
- * also load the couple of functions that *do* exist in mingw headers but not
- * on NT4. That way, we don't break on NT4.
+ * a whole lot of API functions dynamically.
*/
typedef BOOL (WINAPI * __CreateRestrictedToken) (HANDLE, DWORD, DWORD, PSID_AND_ATTRIBUTES, DWORD, PLUID_AND_ATTRIBUTES, DWORD, PSID_AND_ATTRIBUTES, PHANDLE);
typedef BOOL (WINAPI * __IsProcessInJob) (HANDLE, HANDLE, PBOOL);
@@ -1768,9 +1766,6 @@ InheritStdHandles(STARTUPINFO *si)
*
* Returns 0 on success, non-zero on failure, same as CreateProcess().
*
- * On NT4, or any other system not containing the required functions, will
- * launch the process under the current token without doing any modifications.
- *
* NOTE! Job object will only work when running as a service, because it's
* automatically destroyed when pg_ctl exits.
*/
@@ -1815,14 +1810,9 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo, bool as_ser
if (_CreateRestrictedToken == NULL)
{
- /*
- * NT4 doesn't have CreateRestrictedToken, so just call ordinary
- * CreateProcess
- */
- write_stderr(_("%s: WARNING: cannot create restricted tokens on this platform\n"), progname);
- if (Advapi32Handle != NULL)
- FreeLibrary(Advapi32Handle);
- return CreateProcess(NULL, cmd, NULL, NULL, FALSE, 0, NULL, NULL, &si, processInfo);
+ /* Log error if we cannot get the function */
+ write_stderr(_("%s: WARNING: could not locate object function to create restricted token\n"), progname);
+ return 0;
}
/* Open the current token to use as a base for the restricted one */
diff --git a/src/interfaces/libpq/win32.c b/src/interfaces/libpq/win32.c
index e57b602476..447f64c072 100644
--- a/src/interfaces/libpq/win32.c
+++ b/src/interfaces/libpq/win32.c
@@ -271,10 +271,6 @@ struct MessageDLL
* Returns a description of the socket error by first trying
* to find it in the lookup table, and if that fails, tries
* to load any of the winsock dlls to find that message.
- * The DLL thing works from Nt4 (spX ?) up, but some special
- * versions of winsock might have this as well (seen on Win98 SE
- * special install) / Magnus Naeslund (mag@fbab.net)
- *
*/
const char *
On Sat, Aug 27, 2022 at 02:35:25PM +0900, Michael Paquier wrote:
There may be a point in enforcing CreateProcess() if
CreateRestrictedToken() cannot be loaded, but that would be a security
issue if Windows goes crazy as we should always expect the function,
so this had better return an error.
And applied as of b1ec7f4.
--
Michael
On Tue, Aug 30, 2022 at 12:57 PM Michael Paquier <michael@paquier.xyz> wrote:
On Sat, Aug 27, 2022 at 02:35:25PM +0900, Michael Paquier wrote:
There may be a point in enforcing CreateProcess() if
CreateRestrictedToken() cannot be loaded, but that would be a security
issue if Windows goes crazy as we should always expect the function,
so this had better return an error.And applied as of b1ec7f4.
This reminds me of 24c3ce8f1, which replaced a dlopen()-ish thing with
a direct function call. Can you just call all these functions
directly these days?
On Tue, Aug 30, 2022 at 01:29:24PM +1200, Thomas Munro wrote:
This reminds me of 24c3ce8f1, which replaced a dlopen()-ish thing with
a direct function call. Can you just call all these functions
directly these days?
Hmm. Some tests in the CI show that attempting to call directly
MiniDumpWriteDump() causes a linking failure at compilation. Anyway,
in the same fashion, I can get some simplifications done right for
pg_ctl.c, auth.c and restricted_token.c. And I am seeing all these
functions listed in the headers of MinGW, meaning that all these
should work out of the box in this case, no?
The others are library-dependent, and I not really confident about
ldap_start_tls_sA(). So, at the end, I am finishing with the
attached, what do you think? This cuts some code, which is nice:
3 files changed, 48 insertions(+), 159 deletions(-)
--
Michael
Attachments:
win32_direct_funccalls.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index b2b0b83a97..b3e51698dc 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -1201,11 +1201,8 @@ pg_SSPI_recvauth(Port *port)
DWORD accountnamesize = sizeof(accountname);
DWORD domainnamesize = sizeof(domainname);
SID_NAME_USE accountnameuse;
- HMODULE secur32;
char *authn_id;
- QUERY_SECURITY_CONTEXT_TOKEN_FN _QuerySecurityContextToken;
-
/*
* Acquire a handle to the server credentials.
*/
@@ -1358,36 +1355,12 @@ pg_SSPI_recvauth(Port *port)
*
* Get the name of the user that authenticated, and compare it to the pg
* username that was specified for the connection.
- *
- * MingW is missing the export for QuerySecurityContextToken in the
- * secur32 library, so we have to load it dynamically.
*/
- secur32 = LoadLibrary("SECUR32.DLL");
- if (secur32 == NULL)
- ereport(ERROR,
- (errmsg("could not load library \"%s\": error code %lu",
- "SECUR32.DLL", GetLastError())));
-
- _QuerySecurityContextToken = (QUERY_SECURITY_CONTEXT_TOKEN_FN) (pg_funcptr_t)
- GetProcAddress(secur32, "QuerySecurityContextToken");
- if (_QuerySecurityContextToken == NULL)
- {
- FreeLibrary(secur32);
- ereport(ERROR,
- (errmsg_internal("could not locate QuerySecurityContextToken in secur32.dll: error code %lu",
- GetLastError())));
- }
-
- r = (_QuerySecurityContextToken) (sspictx, &token);
+ r = QuerySecurityContextToken(sspictx, &token);
if (r != SEC_E_OK)
- {
- FreeLibrary(secur32);
pg_SSPI_error(ERROR,
_("could not get token from SSPI security context"), r);
- }
-
- FreeLibrary(secur32);
/*
* No longer need the security context, everything from here on uses the
diff --git a/src/common/restricted_token.c b/src/common/restricted_token.c
index 82b74b565e..8f4b76b329 100644
--- a/src/common/restricted_token.c
+++ b/src/common/restricted_token.c
@@ -28,8 +28,6 @@
/* internal vars */
char *restrict_env;
-typedef BOOL (WINAPI * __CreateRestrictedToken) (HANDLE, DWORD, DWORD, PSID_AND_ATTRIBUTES, DWORD, PLUID_AND_ATTRIBUTES, DWORD, PSID_AND_ATTRIBUTES, PHANDLE);
-
/* Windows API define missing from some versions of MingW headers */
#ifndef DISABLE_MAX_PRIVILEGE
#define DISABLE_MAX_PRIVILEGE 0x1
@@ -52,36 +50,15 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo)
HANDLE restrictedToken;
SID_IDENTIFIER_AUTHORITY NtAuthority = {SECURITY_NT_AUTHORITY};
SID_AND_ATTRIBUTES dropSids[2];
- __CreateRestrictedToken _CreateRestrictedToken;
- HANDLE Advapi32Handle;
ZeroMemory(&si, sizeof(si));
si.cb = sizeof(si);
- Advapi32Handle = LoadLibrary("ADVAPI32.DLL");
- if (Advapi32Handle == NULL)
- {
- pg_log_error("could not load library \"%s\": error code %lu",
- "ADVAPI32.DLL", GetLastError());
- return 0;
- }
-
- _CreateRestrictedToken = (__CreateRestrictedToken) (pg_funcptr_t) GetProcAddress(Advapi32Handle, "CreateRestrictedToken");
-
- if (_CreateRestrictedToken == NULL)
- {
- pg_log_error("cannot create restricted tokens on this platform: error code %lu",
- GetLastError());
- FreeLibrary(Advapi32Handle);
- return 0;
- }
-
/* Open the current token to use as a base for the restricted one */
if (!OpenProcessToken(GetCurrentProcess(), TOKEN_ALL_ACCESS, &origToken))
{
pg_log_error("could not open process token: error code %lu",
GetLastError());
- FreeLibrary(Advapi32Handle);
return 0;
}
@@ -97,22 +74,20 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo)
pg_log_error("could not allocate SIDs: error code %lu",
GetLastError());
CloseHandle(origToken);
- FreeLibrary(Advapi32Handle);
return 0;
}
- b = _CreateRestrictedToken(origToken,
- DISABLE_MAX_PRIVILEGE,
- sizeof(dropSids) / sizeof(dropSids[0]),
- dropSids,
- 0, NULL,
- 0, NULL,
- &restrictedToken);
+ b = CreateRestrictedToken(origToken,
+ DISABLE_MAX_PRIVILEGE,
+ sizeof(dropSids) / sizeof(dropSids[0]),
+ dropSids,
+ 0, NULL,
+ 0, NULL,
+ &restrictedToken);
FreeSid(dropSids[1].Sid);
FreeSid(dropSids[0].Sid);
CloseHandle(origToken);
- FreeLibrary(Advapi32Handle);
if (!b)
{
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index bea2470d86..4fb24c8a39 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -1724,17 +1724,6 @@ pgwin32_doRunAsService(void)
}
-/*
- * Mingw headers are incomplete, and so are the libraries. So we have to load
- * a whole lot of API functions dynamically.
- */
-typedef BOOL (WINAPI * __CreateRestrictedToken) (HANDLE, DWORD, DWORD, PSID_AND_ATTRIBUTES, DWORD, PLUID_AND_ATTRIBUTES, DWORD, PSID_AND_ATTRIBUTES, PHANDLE);
-typedef BOOL (WINAPI * __IsProcessInJob) (HANDLE, HANDLE, PBOOL);
-typedef HANDLE (WINAPI * __CreateJobObject) (LPSECURITY_ATTRIBUTES, LPCTSTR);
-typedef BOOL (WINAPI * __SetInformationJobObject) (HANDLE, JOBOBJECTINFOCLASS, LPVOID, DWORD);
-typedef BOOL (WINAPI * __AssignProcessToJobObject) (HANDLE, HANDLE);
-typedef BOOL (WINAPI * __QueryInformationJobObject) (HANDLE, JOBOBJECTINFOCLASS, LPVOID, DWORD, LPDWORD);
-
/*
* Set up STARTUPINFO for the new process to inherit this process' handles.
*
@@ -1777,20 +1766,11 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo, bool as_ser
STARTUPINFO si;
HANDLE origToken;
HANDLE restrictedToken;
+ BOOL inJob;
SID_IDENTIFIER_AUTHORITY NtAuthority = {SECURITY_NT_AUTHORITY};
SID_AND_ATTRIBUTES dropSids[2];
PTOKEN_PRIVILEGES delPrivs;
- /* Functions loaded dynamically */
- __CreateRestrictedToken _CreateRestrictedToken = NULL;
- __IsProcessInJob _IsProcessInJob = NULL;
- __CreateJobObject _CreateJobObject = NULL;
- __SetInformationJobObject _SetInformationJobObject = NULL;
- __AssignProcessToJobObject _AssignProcessToJobObject = NULL;
- __QueryInformationJobObject _QueryInformationJobObject = NULL;
- HANDLE Kernel32Handle;
- HANDLE Advapi32Handle;
-
ZeroMemory(&si, sizeof(si));
si.cb = sizeof(si);
@@ -1802,20 +1782,6 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo, bool as_ser
*/
InheritStdHandles(&si);
- Advapi32Handle = LoadLibrary("ADVAPI32.DLL");
- if (Advapi32Handle != NULL)
- {
- _CreateRestrictedToken = (__CreateRestrictedToken) (pg_funcptr_t) GetProcAddress(Advapi32Handle, "CreateRestrictedToken");
- }
-
- if (_CreateRestrictedToken == NULL)
- {
- /* Log error if we cannot get the function */
- write_stderr(_("%s: could not locate object function to create restricted token: error code %lu\n"),
- progname, (unsigned long) GetLastError());
- return 0;
- }
-
/* Open the current token to use as a base for the restricted one */
if (!OpenProcessToken(GetCurrentProcess(), TOKEN_ALL_ACCESS, &origToken))
{
@@ -1848,19 +1814,18 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo, bool as_ser
/* Error message already printed */
return 0;
- b = _CreateRestrictedToken(origToken,
- 0,
- sizeof(dropSids) / sizeof(dropSids[0]),
- dropSids,
- delPrivs->PrivilegeCount, delPrivs->Privileges,
- 0, NULL,
- &restrictedToken);
+ b = CreateRestrictedToken(origToken,
+ 0,
+ sizeof(dropSids) / sizeof(dropSids[0]),
+ dropSids,
+ delPrivs->PrivilegeCount, delPrivs->Privileges,
+ 0, NULL,
+ &restrictedToken);
free(delPrivs);
FreeSid(dropSids[1].Sid);
FreeSid(dropSids[0].Sid);
CloseHandle(origToken);
- FreeLibrary(Advapi32Handle);
if (!b)
{
@@ -1872,79 +1837,55 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo, bool as_ser
AddUserToTokenDacl(restrictedToken);
r = CreateProcessAsUser(restrictedToken, NULL, cmd, NULL, NULL, TRUE, CREATE_SUSPENDED, NULL, NULL, &si, processInfo);
- Kernel32Handle = LoadLibrary("KERNEL32.DLL");
- if (Kernel32Handle != NULL)
+ if (IsProcessInJob(processInfo->hProcess, NULL, &inJob))
{
- _IsProcessInJob = (__IsProcessInJob) (pg_funcptr_t) GetProcAddress(Kernel32Handle, "IsProcessInJob");
- _CreateJobObject = (__CreateJobObject) (pg_funcptr_t) GetProcAddress(Kernel32Handle, "CreateJobObjectA");
- _SetInformationJobObject = (__SetInformationJobObject) (pg_funcptr_t) GetProcAddress(Kernel32Handle, "SetInformationJobObject");
- _AssignProcessToJobObject = (__AssignProcessToJobObject) (pg_funcptr_t) GetProcAddress(Kernel32Handle, "AssignProcessToJobObject");
- _QueryInformationJobObject = (__QueryInformationJobObject) (pg_funcptr_t) GetProcAddress(Kernel32Handle, "QueryInformationJobObject");
- }
-
- /* Verify that we found all functions */
- if (_IsProcessInJob == NULL || _CreateJobObject == NULL || _SetInformationJobObject == NULL || _AssignProcessToJobObject == NULL || _QueryInformationJobObject == NULL)
- {
- /* Log error if we can't get version */
- write_stderr(_("%s: WARNING: could not locate all job object functions in system API\n"), progname);
- }
- else
- {
- BOOL inJob;
-
- if (_IsProcessInJob(processInfo->hProcess, NULL, &inJob))
+ if (!inJob)
{
- if (!inJob)
+ /*
+ * Job objects are working, and the new process isn't in one,
+ * so we can create one safely. If any problems show up when
+ * setting it, we're going to ignore them.
+ */
+ HANDLE job;
+ char jobname[128];
+
+ sprintf(jobname, "PostgreSQL_%lu",
+ (unsigned long) processInfo->dwProcessId);
+
+ job = CreateJobObject(NULL, jobname);
+ if (job)
{
- /*
- * Job objects are working, and the new process isn't in one,
- * so we can create one safely. If any problems show up when
- * setting it, we're going to ignore them.
- */
- HANDLE job;
- char jobname[128];
+ JOBOBJECT_BASIC_LIMIT_INFORMATION basicLimit;
+ JOBOBJECT_BASIC_UI_RESTRICTIONS uiRestrictions;
+ JOBOBJECT_SECURITY_LIMIT_INFORMATION securityLimit;
- sprintf(jobname, "PostgreSQL_%lu",
- (unsigned long) processInfo->dwProcessId);
+ ZeroMemory(&basicLimit, sizeof(basicLimit));
+ ZeroMemory(&uiRestrictions, sizeof(uiRestrictions));
+ ZeroMemory(&securityLimit, sizeof(securityLimit));
- job = _CreateJobObject(NULL, jobname);
- if (job)
- {
- JOBOBJECT_BASIC_LIMIT_INFORMATION basicLimit;
- JOBOBJECT_BASIC_UI_RESTRICTIONS uiRestrictions;
- JOBOBJECT_SECURITY_LIMIT_INFORMATION securityLimit;
+ basicLimit.LimitFlags = JOB_OBJECT_LIMIT_DIE_ON_UNHANDLED_EXCEPTION | JOB_OBJECT_LIMIT_PRIORITY_CLASS;
+ basicLimit.PriorityClass = NORMAL_PRIORITY_CLASS;
+ SetInformationJobObject(job, JobObjectBasicLimitInformation, &basicLimit, sizeof(basicLimit));
- ZeroMemory(&basicLimit, sizeof(basicLimit));
- ZeroMemory(&uiRestrictions, sizeof(uiRestrictions));
- ZeroMemory(&securityLimit, sizeof(securityLimit));
+ uiRestrictions.UIRestrictionsClass = JOB_OBJECT_UILIMIT_DESKTOP | JOB_OBJECT_UILIMIT_DISPLAYSETTINGS |
+ JOB_OBJECT_UILIMIT_EXITWINDOWS | JOB_OBJECT_UILIMIT_READCLIPBOARD |
+ JOB_OBJECT_UILIMIT_SYSTEMPARAMETERS | JOB_OBJECT_UILIMIT_WRITECLIPBOARD;
- basicLimit.LimitFlags = JOB_OBJECT_LIMIT_DIE_ON_UNHANDLED_EXCEPTION | JOB_OBJECT_LIMIT_PRIORITY_CLASS;
- basicLimit.PriorityClass = NORMAL_PRIORITY_CLASS;
- _SetInformationJobObject(job, JobObjectBasicLimitInformation, &basicLimit, sizeof(basicLimit));
+ SetInformationJobObject(job, JobObjectBasicUIRestrictions, &uiRestrictions, sizeof(uiRestrictions));
- uiRestrictions.UIRestrictionsClass = JOB_OBJECT_UILIMIT_DESKTOP | JOB_OBJECT_UILIMIT_DISPLAYSETTINGS |
- JOB_OBJECT_UILIMIT_EXITWINDOWS | JOB_OBJECT_UILIMIT_READCLIPBOARD |
- JOB_OBJECT_UILIMIT_SYSTEMPARAMETERS | JOB_OBJECT_UILIMIT_WRITECLIPBOARD;
+ securityLimit.SecurityLimitFlags = JOB_OBJECT_SECURITY_NO_ADMIN | JOB_OBJECT_SECURITY_ONLY_TOKEN;
+ securityLimit.JobToken = restrictedToken;
+ SetInformationJobObject(job, JobObjectSecurityLimitInformation, &securityLimit, sizeof(securityLimit));
- _SetInformationJobObject(job, JobObjectBasicUIRestrictions, &uiRestrictions, sizeof(uiRestrictions));
-
- securityLimit.SecurityLimitFlags = JOB_OBJECT_SECURITY_NO_ADMIN | JOB_OBJECT_SECURITY_ONLY_TOKEN;
- securityLimit.JobToken = restrictedToken;
- _SetInformationJobObject(job, JobObjectSecurityLimitInformation, &securityLimit, sizeof(securityLimit));
-
- _AssignProcessToJobObject(job, processInfo->hProcess);
- }
+ AssignProcessToJobObject(job, processInfo->hProcess);
}
}
}
-
CloseHandle(restrictedToken);
ResumeThread(processInfo->hThread);
- FreeLibrary(Kernel32Handle);
-
/*
* We intentionally don't close the job object handle, because we want the
* object to live on until pg_ctl shuts down.
On Thu, Sep 08, 2022 at 05:55:40PM +0900, Michael Paquier wrote:
On Tue, Aug 30, 2022 at 01:29:24PM +1200, Thomas Munro wrote:
This reminds me of 24c3ce8f1, which replaced a dlopen()-ish thing with
a direct function call. Can you just call all these functions
directly these days?Hmm. Some tests in the CI show that attempting to call directly
MiniDumpWriteDump() causes a linking failure at compilation. Anyway,
in the same fashion, I can get some simplifications done right for
pg_ctl.c, auth.c and restricted_token.c. And I am seeing all these
functions listed in the headers of MinGW, meaning that all these
should work out of the box in this case, no?The others are library-dependent, and I not really confident about
ldap_start_tls_sA(). So, at the end, I am finishing with the
attached, what do you think? This cuts some code, which is nice:
3 files changed, 48 insertions(+), 159 deletions(-)
+1
It seems silly to do it at runtime if it's possible to do it at link time.
--
Justin
On Thu, Sep 08, 2022 at 08:29:20AM -0500, Justin Pryzby wrote:
It seems silly to do it at runtime if it's possible to do it at link time.
Thanks. This set of simplifications is too good to let go, and I have
a window to look after the buildfarm today and tomorrow, which should
be enough to take action if need be. Hence, I have applied the
patch. Now, let's see what the buildfarm tells us ;)
--
Michael
On Fri, Sep 09, 2022 at 10:55:55AM +0900, Michael Paquier wrote:
Thanks. This set of simplifications is too good to let go, and I have
a window to look after the buildfarm today and tomorrow, which should
be enough to take action if need be. Hence, I have applied the
patch. Now, let's see what the buildfarm tells us ;)
Based on what I can see, the Windows animals seem to have digested
47bd0b3 (cygwin, MinGW and MSVC), so I think that we are good.
--
Michael
On Fri, Sep 09, 2022 at 08:11:09PM +0900, Michael Paquier wrote:
Based on what I can see, the Windows animals seem to have digested
47bd0b3 (cygwin, MinGW and MSVC), so I think that we are good.
The last part that's worth adjusting is ldap_start_tls_sA(), which
would lead to the attached simplification. The MinGW headers list
this routine, so like the previous change I think that it should be
safe for such builds.
Looking at the buildfarm animals, bowerbird, jacana, fairywren,
lorikeet and drongo disable ldap. hamerkop is the only member that
provides coverage for it, still that's a MSVC build.
The CI provides coverage for ldap as it is enabled by default and
windows_build_config.pl does not tell otherwise, but with the existing
animals we don't have ldap coverage under MinGW.
Anyway, I'd like to apply the attached, and I don't quite see why it
would not work after 47bd0b3 under MinGW. Any thoughts?
--
Michael
Attachments:
win32-ldap-funccall.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index a776bc3ed7..3a10baa9ce 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -135,14 +135,6 @@ static int CheckBSDAuth(Port *port, char *user);
#else
#include <winldap.h>
-/* Correct header from the Platform SDK */
-typedef
-ULONG (*__ldap_start_tls_sA) (IN PLDAP ExternalHandle,
- OUT PULONG ServerReturnValue,
- OUT LDAPMessage **result,
- IN PLDAPControlA * ServerControls,
- IN PLDAPControlA * ClientControls
-);
#endif
static int CheckLDAPAuth(Port *port);
@@ -2348,48 +2340,7 @@ InitializeLDAPConnection(Port *port, LDAP **ldap)
#ifndef WIN32
if ((r = ldap_start_tls_s(*ldap, NULL, NULL)) != LDAP_SUCCESS)
#else
- static __ldap_start_tls_sA _ldap_start_tls_sA = NULL;
-
- if (_ldap_start_tls_sA == NULL)
- {
- /*
- * Need to load this function dynamically because it may not exist
- * on Windows, and causes a load error for the whole exe if
- * referenced.
- */
- HANDLE ldaphandle;
-
- ldaphandle = LoadLibrary("WLDAP32.DLL");
- if (ldaphandle == NULL)
- {
- /*
- * should never happen since we import other files from
- * wldap32, but check anyway
- */
- ereport(LOG,
- (errmsg("could not load library \"%s\": error code %lu",
- "WLDAP32.DLL", GetLastError())));
- ldap_unbind(*ldap);
- return STATUS_ERROR;
- }
- _ldap_start_tls_sA = (__ldap_start_tls_sA) (pg_funcptr_t) GetProcAddress(ldaphandle, "ldap_start_tls_sA");
- if (_ldap_start_tls_sA == NULL)
- {
- ereport(LOG,
- (errmsg("could not load function _ldap_start_tls_sA in wldap32.dll"),
- errdetail("LDAP over SSL is not supported on this platform.")));
- ldap_unbind(*ldap);
- FreeLibrary(ldaphandle);
- return STATUS_ERROR;
- }
-
- /*
- * Leak LDAP handle on purpose, because we need the library to
- * stay open. This is ok because it will only ever be leaked once
- * per process and is automatically cleaned up on process exit.
- */
- }
- if ((r = _ldap_start_tls_sA(*ldap, NULL, NULL, NULL, NULL)) != LDAP_SUCCESS)
+ if ((r = ldap_start_tls_sA(*ldap, NULL, NULL, NULL, NULL)) != LDAP_SUCCESS)
#endif
{
ereport(LOG,
On Sun, Sep 11, 2022 at 09:28:54AM +0900, Michael Paquier wrote:
On Fri, Sep 09, 2022 at 08:11:09PM +0900, Michael Paquier wrote:
Based on what I can see, the Windows animals seem to have digested
47bd0b3 (cygwin, MinGW and MSVC), so I think that we are good.The last part that's worth adjusting is ldap_start_tls_sA(), which
would lead to the attached simplification. The MinGW headers list
this routine, so like the previous change I think that it should be
safe for such builds.Looking at the buildfarm animals, bowerbird, jacana, fairywren,
lorikeet and drongo disable ldap. hamerkop is the only member that
provides coverage for it, still that's a MSVC build.The CI provides coverage for ldap as it is enabled by default and
windows_build_config.pl does not tell otherwise, but with the existing
animals we don't have ldap coverage under MinGW.Anyway, I'd like to apply the attached, and I don't quite see why it
would not work after 47bd0b3 under MinGW. Any thoughts?
There's a CF entry to add it, and I launched it with your patch.
(This is in a branch which already has that, and also does a few other
things differently).
https://cirrus-ci.com/task/6302833585684480
[02:07:57.497] checking whether to build with LDAP support... yes
It compiles, which is probably all that matters, and eventually skips
the test anyway.
[02:23:18.209] [02:23:18] c:/cirrus/src/test/ldap/t/001_auth.pl .. skipped: ldap tests not supported on MSWin32 or dependencies not installed
--
Justin
On Sat, Sep 10, 2022 at 10:39:19PM -0500, Justin Pryzby wrote:
There's a CF entry to add it, and I launched it with your patch.
(This is in a branch which already has that, and also does a few other
things differently).
No need for a CF entry if you want to play with the tree. I have
Cirrus enabled on my own fork of Postgres on github, and I saw the
same result as you:
https://github.com/michaelpq/postgres/
--
Michael
On Sun, Sep 11, 2022 at 12:29 PM Michael Paquier <michael@paquier.xyz> wrote:
On Fri, Sep 09, 2022 at 08:11:09PM +0900, Michael Paquier wrote:
Based on what I can see, the Windows animals seem to have digested
47bd0b3 (cygwin, MinGW and MSVC), so I think that we are good.
Great, that's a lot of nice cleanup.
The last part that's worth adjusting is ldap_start_tls_sA(), which
would lead to the attached simplification.
- if ((r = _ldap_start_tls_sA(*ldap, NULL, NULL, NULL, NULL))
!= LDAP_SUCCESS)
+ if ((r = ldap_start_tls_sA(*ldap, NULL, NULL, NULL, NULL)) !=
LDAP_SUCCESS)When looking the function up it made sense to use the name ending in
'...A', but when calling directly I think we shouldn't use the A
suffix, we should let the <winldap.h> macros do that for us[1]https://docs.microsoft.com/en-us/windows/win32/api/winldap/nf-winldap-ldap_start_tls_sa. (I
wondered for a moment if that would even make Windows and Unix code
the same, but sadly not due to the extra NULL arguments.)
[1]: https://docs.microsoft.com/en-us/windows/win32/api/winldap/nf-winldap-ldap_start_tls_sa
On Mon, Sep 12, 2022 at 09:42:25AM +1200, Thomas Munro wrote:
When looking the function up it made sense to use the name ending in
'...A', but when calling directly I think we shouldn't use the A
suffix, we should let the <winldap.h> macros do that for us[1]. (I
wondered for a moment if that would even make Windows and Unix code
the same, but sadly not due to the extra NULL arguments.)
Good idea, I did not noticed this part. This should work equally, so
done this way and applied. I am keeping an eye on the buildfarm.
--
Michael
After 495ed0e, do these references to Windows SDK 8.1 still make sense?
src/sgml/install-windows.sgml: as well as standalone Windows SDK
releases 8.1a to 10.
src/sgml/install-windows.sgml: <productname>Microsoft Windows
SDK</productname> version 8.1a to 10 or
On Mon, May 15, 2023 at 02:08:32PM +1200, Thomas Munro wrote:
After 495ed0e, do these references to Windows SDK 8.1 still make sense?
src/sgml/install-windows.sgml: as well as standalone Windows SDK
releases 8.1a to 10.
src/sgml/install-windows.sgml: <productname>Microsoft Windows
SDK</productname> version 8.1a to 10 or
As listed on https://en.wikipedia.org/wiki/Microsoft_Windows_SDK,
likely not. What do you think about the attached?
--
Michael
Attachments:
doc-windows-sdk.patchtext/x-diff; charset=us-asciiDownload
diff --git a/doc/src/sgml/install-windows.sgml b/doc/src/sgml/install-windows.sgml
index cbc70a039c..379a2ea80b 100644
--- a/doc/src/sgml/install-windows.sgml
+++ b/doc/src/sgml/install-windows.sgml
@@ -79,9 +79,9 @@
32-bit PostgreSQL builds are possible with
<productname>Visual Studio 2015</productname> to
<productname>Visual Studio 2022</productname>,
- as well as standalone Windows SDK releases 8.1a to 10.
+ as well as standalone Windows SDK releases 10 and above.
64-bit PostgreSQL builds are supported with
- <productname>Microsoft Windows SDK</productname> version 8.1a to 10 or
+ <productname>Microsoft Windows SDK</productname> version 10 and above or
<productname>Visual Studio 2015</productname> and above.
<!--
For 2015 requirements:
On Mon, May 15, 2023 at 2:57 PM Michael Paquier <michael@paquier.xyz> wrote:
On Mon, May 15, 2023 at 02:08:32PM +1200, Thomas Munro wrote:
After 495ed0e, do these references to Windows SDK 8.1 still make sense?
As listed on https://en.wikipedia.org/wiki/Microsoft_Windows_SDK,
likely not. What do you think about the attached?
LGTM.