Fix MS-VC++2008 and Windows XP 32bit critical error problem #18856

dnobori · 2022-07-24T02:52:07Z

Problem

OpenSSL 3.0.0 to 3.0.5 binaries built with MS-VC++ 2008 always fails to run on Windows XP 32-bit Editions.

Solution

VC++ 2008 or earlier x86 compilers do not have an inline implementation of InterlockedOr64 for 32bit CPUs, then OpenSSL 3.0.0 to 3.0.5 binaries built with MS-VC++ 2008 will always fail to run on Windows XP 32bit.

See: https://docs.microsoft.com/en-us/cpp/intrinsics/interlockedor-intrinsic-functions#requirements

To work around this problem, we implement a manual locking mechanism for only VC++ 2008 or earlier x86 compilers.

Related issue

This patch fixes #18855

…on of InterlockedOr64 for 32bit and will fail to run on Windows XP 32bit. See: https://docs.microsoft.com/en-us/cpp/intrinsics/interlockedor-intrinsic-functions#requirements To work around this problem, we implement a manual locking mechanism for only VC++ 2008 or earlier x86 compilers.

paulidale

This submission will require a signed CLA. Refer to the contributor agreements page.

dnobori · 2022-07-25T01:23:29Z

@paulidale Thank you for information.

I signed and submit the Individual Contributor License Agreement (ICLA) to the OpenSSL Software Foundation email address just now.

hlandau · 2022-07-25T06:45:45Z

crypto/threads_win.c

+    // VC++ 2008 or earlier x86 compilers do not have an inline implementation of InterlockedOr64 for 32bit and will fail to run on Windows XP 32bit.
+    // See: https://docs.microsoft.com/en-us/cpp/intrinsics/interlockedor-intrinsic-functions#requirements
+    // To work around this problem, we implement a manual locking mechanism for only VC++ 2008 or earlier x86 compilers.
+	if (lock == NULL || !CRYPTO_THREAD_write_lock(lock))


Please use C98 block comments, not C99 line comments, and wrap lines to 80 characters.

What about doing this at the top of the file once...

/* * Comments.... */ #if (defined(_MSC_VER) && defined(_M_IX86) && _MSC_VER <= 1500) # define NO_INTERLOCKEDOR64 #endif

This patch is also required for VS 2015 which is _MSC_VER = 1900

I just changed the pragma check for _MSC_VER <= 1900 in crypto\thread_win.c and the build completed using VS2015.

crypto/threads_win.c

…haracters.

mattcaswell · 2022-08-04T09:41:46Z

There was an error in the CLA database which meant the "hold: cla required" flag hadn't been removed automatically. Fixed now.

openssl-machine · 2022-08-05T10:00:17Z

This pull request is ready to merge

mattcaswell · 2022-08-10T10:24:22Z

@slontis or @levitte - this PR has been modified since your approval. Please can one of you reconfirm?

slontis · 2022-08-10T21:31:11Z

reapproved

openssl-machine · 2022-08-11T22:00:15Z

This pull request is ready to merge

…on of InterlockedOr64 for 32bit and will fail to run on Windows XP 32bit. See: https://docs.microsoft.com/en-us/cpp/intrinsics/interlockedor-intrinsic-functions#requirements To work around this problem, we implement a manual locking mechanism for only VC++ 2008 or earlier x86 compilers. Reviewed-by: Shane Lontis <shane.lontis@oracle.com> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from #18856)

mattcaswell · 2022-08-12T09:49:01Z

Pushed to master and 3.0. Thanks. I squashed the commits before push.

…on of InterlockedOr64 for 32bit and will fail to run on Windows XP 32bit. See: https://docs.microsoft.com/en-us/cpp/intrinsics/interlockedor-intrinsic-functions#requirements To work around this problem, we implement a manual locking mechanism for only VC++ 2008 or earlier x86 compilers. Reviewed-by: Shane Lontis <shane.lontis@oracle.com> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from #18856) (cherry picked from commit 2d46a44)

…on of InterlockedOr64 for 32bit and will fail to run on Windows XP 32bit. See: https://docs.microsoft.com/en-us/cpp/intrinsics/interlockedor-intrinsic-functions#requirements To work around this problem, we implement a manual locking mechanism for only VC++ 2008 or earlier x86 compilers. Reviewed-by: Shane Lontis <shane.lontis@oracle.com> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from openssl#18856)

hlsantos

This patch is also required for VS 2015 which is _MSC_VER = 1900

I just changed the pragma check for _MSC_VER <= 1900 in crypto\thread_win.c and build completed.

hlsantos · 2023-02-13T12:33:11Z

There are many good reasons why I prefer to compile my products [with] vs2015 compiler version 19.00.24215.1, among them is security concerns with _MSC_VER > 1900 telemetry design. Back then, vs2015 was provided with a patch to remove the telemetry in perhaps its last update I am aware of. I still have a good bit of XP/2003 customers, sysops, operators, including myself who run ops on Wildcat! Server on private backend 2003 non-user boxes and/or as XP clients to play games or run a server There is good reason for a security component such as OpenSSL to continue to support XP/2003 boxes, at least for a while longer . Of course, I can continue to add my XP/2003 support build patch to the OpenSSL updates, such as this:


# 04/21/2019 08:03 am, SSI/HLS
# Adds XP/2003 support
# usage from within build folder:
# perl ..\configure VC-WIN32-XP --config=..\xp-support.conf
my %targets = (
    "VC-WIN32-XP" => {
        inherit_from => [ "VC-WIN32" ],
        lflags       => '/subsystem:console,"5.01" /opt:ref',
        bin_lflags   => '/subsystem:console,"5.01" /opt:ref',
    },
    "VC-WIN64A-XP" => {
        inherit_from => [ "VC-WIN64A" ],
        lflags       => '/subsystem:console,"5.02" /opt:ref',
        bin_lflags   => '/subsystem:console,"5.02" /opt:ref',
    },
);

I just built for openssl 1.1.1t, and with the crypto\thread_win.c patch, I just finished building for openssl-3.0.8.

I have to now recompile and test a few tools using v3.0.8.

openssl-machine added the hold: cla required The contributor needs to submit a license agreement label Jul 24, 2022

This was referenced Jul 24, 2022

InterlockedOr64() breaks compatibility with Windows versions older than Vista #18855

Open

InterlockedCompareExchange64 error on Windows XP 32-bit SoftEtherVPN/SoftEtherVPN#1631

Closed

paulidale approved these changes Jul 25, 2022

View reviewed changes

paulidale added branch: master Merge to master branch approval: review pending This pull request needs review by a committer triaged: bug The issue/pr is/fixes a bug branch: 3.0 Merge to openssl-3.0 branch labels Jul 25, 2022

github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Jul 25, 2022

slontis approved these changes Jul 25, 2022

View reviewed changes

levitte approved these changes Jul 25, 2022

View reviewed changes

hlandau requested changes Jul 25, 2022

View reviewed changes

Daiyuu Nobori added 2 commits August 4, 2022 17:52

Use C98 block comments, not C99 line comments, and wrap lines to 80 c…

51494a6

…haracters.

Use NO_INTERLOCKEDOR64 macro

4d1dac1

dnobori closed this Aug 4, 2022

dnobori reopened this Aug 4, 2022

mattcaswell closed this Aug 4, 2022

mattcaswell reopened this Aug 4, 2022

mattcaswell closed this Aug 4, 2022

mattcaswell reopened this Aug 4, 2022

openssl-machine removed the hold: cla required The contributor needs to submit a license agreement label Aug 4, 2022

hlandau approved these changes Aug 4, 2022

View reviewed changes

hlandau added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Aug 4, 2022

openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Aug 5, 2022

mattcaswell added approval: otc review pending This pull request needs review by an OTC member and removed approval: ready to merge The 24 hour grace period has passed, ready to merge labels Aug 10, 2022

slontis added approval: done This pull request has the required number of approvals and removed approval: otc review pending This pull request needs review by an OTC member labels Aug 10, 2022

openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Aug 11, 2022

mattcaswell closed this Aug 12, 2022

hlsantos reviewed Feb 13, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix MS-VC++2008 and Windows XP 32bit critical error problem #18856

Fix MS-VC++2008 and Windows XP 32bit critical error problem #18856

dnobori commented Jul 24, 2022 •

edited

paulidale left a comment

dnobori commented Jul 25, 2022

hlandau Jul 25, 2022

slontis Jul 25, 2022

hlsantos Feb 13, 2023

mattcaswell commented Aug 4, 2022

openssl-machine commented Aug 5, 2022

mattcaswell commented Aug 10, 2022

slontis commented Aug 10, 2022

openssl-machine commented Aug 11, 2022

mattcaswell commented Aug 12, 2022

hlsantos left a comment

hlsantos commented Feb 13, 2023 •

edited

Fix MS-VC++2008 and Windows XP 32bit critical error problem #18856

Fix MS-VC++2008 and Windows XP 32bit critical error problem #18856

Conversation

dnobori commented Jul 24, 2022 • edited

Problem

Solution

Related issue

paulidale left a comment

Choose a reason for hiding this comment

dnobori commented Jul 25, 2022

hlandau Jul 25, 2022

Choose a reason for hiding this comment

slontis Jul 25, 2022

Choose a reason for hiding this comment

hlsantos Feb 13, 2023

Choose a reason for hiding this comment

mattcaswell commented Aug 4, 2022

openssl-machine commented Aug 5, 2022

mattcaswell commented Aug 10, 2022

slontis commented Aug 10, 2022

openssl-machine commented Aug 11, 2022

mattcaswell commented Aug 12, 2022

hlsantos left a comment

Choose a reason for hiding this comment

hlsantos commented Feb 13, 2023 • edited

dnobori commented Jul 24, 2022 •

edited

hlsantos commented Feb 13, 2023 •

edited