[Buildroot] [PATCH] openssl: fix race condition when symlink shared libs

[Buildroot] [PATCH] openssl: fix race condition when symlink shared libs

[Ryan Barnett]

The build-shared target depends on do_crypto and link-shared, which
will be executed in parallel. do_crypto calls
link_a.linux_shared -> link_a.gnu which does SYMLINK_SO; in parallel,
link-shared calls symlink.linux_shared which also does SYMLINK_SO.
Before the symlink is created, it is rm'ed, but there is a tiny chance
that the second one is created after the rm has been called.

Fix this by using 'ln -sf' instead of 'ln -s' so the build doesn't
error out.

Patch submitted upstream at:
  https://bugs.gentoo.org/show_bug.cgi?id=566260

Thanks to Arnout for explain the issue (wording used above).

Signed-off-by: Ryan Barnett <ryan.barnett@rockwellcollins.com>
CC: Arnout Vandecappelle <arnout@mind.be>
CC: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

---

Note this is a temporary fix until a gentoo maintainer weighs in and
updates their parallel build patch.

Signed-off-by: Ryan Barnett <ryan.barnett@rockwellcollins.com>
---
 ...ared-fix-race-condition-when-symlinking-s.patch | 45 ++++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 package/openssl/0003-makefile.shared-fix-race-condition-when-symlinking-s.patch

diff --git a/package/openssl/0003-makefile.shared-fix-race-condition-when-symlinking-s.patch b/package/openssl/0003-makefile.shared-fix-race-condition-when-symlinking-s.patch
new file mode 100644
index 0000000..fd39723
--- /dev/null
+++ b/package/openssl/0003-makefile.shared-fix-race-condition-when-symlinking-s.patch
@@ -0,0 +1,45 @@
+From bcea66251e8014c12db81ff54f419fe82b6f8527 Mon Sep 17 00:00:00 2001
+From: Ryan Barnett <ryan.barnett@rockwellcollins.com>
+Date: Thu, 19 Nov 2015 10:47:00 -0600
+Subject: [PATCH] makefile.shared: fix race condition when symlinking shared
+ libs
+
+The build-shared target depends on do_crypto and link-shared, which
+will be executed in parallel. do_crypto calls
+link_a.linux_shared -> link_a.gnu which does SYMLINK_SO; in parallel,
+link-shared calls symlink.linux_shared which also does SYMLINK_SO.
+Before the symlink is created, it is rm'ed, but there is a tiny chance
+that the second one is created after the rm has been called.
+
+Fix this race condition by just using ln -sf since it will be the same
+symlink regards and not cause the build to error out.
+
+Signed-off-by: Ryan Barnett <ryan.barnett@rockwellcollins.com>
+---
+ Makefile.shared | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/Makefile.shared b/Makefile.shared
+index 8d57163..ce66574 100644
+--- a/Makefile.shared
++++ b/Makefile.shared
+@@ -118,14 +118,14 @@ SYMLINK_SO=	\
+ 		if [ -n "$$SHLIB_COMPAT" ]; then \
+ 			for x in $$SHLIB_COMPAT; do \
+ 				( $(SET_X); rm -f $$SHLIB$$x$$SHLIB_SUFFIX; \
+-				  ln -s $$prev $$SHLIB$$x$$SHLIB_SUFFIX ); \
++				  ln -sf $$prev $$SHLIB$$x$$SHLIB_SUFFIX ); \
+ 				prev=$$SHLIB$$x$$SHLIB_SUFFIX; \
+ 			done; \
+ 		fi; \
+ 		if [ -n "$$SHLIB_SOVER" ]; then \
+ 			[ -e "$$SHLIB$$SHLIB_SUFFIX" ] || \
+ 			( $(SET_X); rm -f $$SHLIB$$SHLIB_SUFFIX; \
+-			  ln -s $$prev $$SHLIB$$SHLIB_SUFFIX ); \
++			  ln -sf $$prev $$SHLIB$$SHLIB_SUFFIX ); \
+ 		fi; \
+ 	fi
+ 
+-- 
+1.9.1
+
-- 
1.9.1

[Buildroot] [PATCH] openssl: fix race condition when symlink shared libs

[Peter Rosin]

Hi Ryan,

On 2015-11-19 20:50, Ryan Barnett wrote:
> The build-shared target depends on do_crypto and link-shared, which
> will be executed in parallel. do_crypto calls
> link_a.linux_shared -> link_a.gnu which does SYMLINK_SO; in parallel,
> link-shared calls symlink.linux_shared which also does SYMLINK_SO.
> Before the symlink is created, it is rm'ed, but there is a tiny chance
> that the second one is created after the rm has been called.
>
> Fix this by using 'ln -sf' instead of 'ln -s' so the build doesn't
> error out.
>
> Patch submitted upstream at:
>   https://bugs.gentoo.org/show_bug.cgi?id=566260
>
> Thanks to Arnout for explain the issue (wording used above).
>
> Signed-off-by: Ryan Barnett <ryan.barnett@rockwellcollins.com>
> CC: Arnout Vandecappelle <arnout@mind.be>
> CC: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>
> ---
>
> Note this is a temporary fix until a gentoo maintainer weighs in and
> updates their parallel build patch.
>
> Signed-off-by: Ryan Barnett <ryan.barnett@rockwellcollins.com>
> ---
>  ...ared-fix-race-condition-when-symlinking-s.patch | 45 ++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>  create mode 100644 package/openssl/0003-makefile.shared-fix-race-condition-when-symlinking-s.patch
>
> diff --git a/package/openssl/0003-makefile.shared-fix-race-condition-when-symlinking-s.patch b/package/openssl/0003-makefile.shared-fix-race-condition-when-symlinking-s.patch
> new file mode 100644
> index 0000000..fd39723
> --- /dev/null
> +++ b/package/openssl/0003-makefile.shared-fix-race-condition-when-symlinking-s.patch
> @@ -0,0 +1,45 @@
> +From bcea66251e8014c12db81ff54f419fe82b6f8527 Mon Sep 17 00:00:00 2001
> +From: Ryan Barnett <ryan.barnett@rockwellcollins.com>
> +Date: Thu, 19 Nov 2015 10:47:00 -0600
> +Subject: [PATCH] makefile.shared: fix race condition when symlinking shared
> + libs
> +
> +The build-shared target depends on do_crypto and link-shared, which
> +will be executed in parallel. do_crypto calls
> +link_a.linux_shared -> link_a.gnu which does SYMLINK_SO; in parallel,
> +link-shared calls symlink.linux_shared which also does SYMLINK_SO.
> +Before the symlink is created, it is rm'ed, but there is a tiny chance
> +that the second one is created after the rm has been called.
> +
> +Fix this race condition by just using ln -sf since it will be the same
> +symlink regards and not cause the build to error out.
> +
> +Signed-off-by: Ryan Barnett <ryan.barnett@rockwellcollins.com>
> +---
> + Makefile.shared | 4 ++--
> + 1 file changed, 2 insertions(+), 2 deletions(-)
> +
> +diff --git a/Makefile.shared b/Makefile.shared
> +index 8d57163..ce66574 100644
> +--- a/Makefile.shared
> ++++ b/Makefile.shared
> +@@ -118,14 +118,14 @@ SYMLINK_SO=	\
> + 		if [ -n "$$SHLIB_COMPAT" ]; then \
> + 			for x in $$SHLIB_COMPAT; do \
> + 				( $(SET_X); rm -f $$SHLIB$$x$$SHLIB_SUFFIX; \
> +-				  ln -s $$prev $$SHLIB$$x$$SHLIB_SUFFIX ); \
> ++				  ln -sf $$prev $$SHLIB$$x$$SHLIB_SUFFIX ); \

I thought the problem was that the two parties raced, not that the symlink couldn't be
created. Thus, the key part was to replace "rm" with the force flag of ln, so that race
loser does a no-operation instead of (temporarily) clobbering the state for the race
winner. So, you apparently only do half the job in this patch...

> + 				prev=$$SHLIB$$x$$SHLIB_SUFFIX; \
> + 			done; \
> + 		fi; \
> + 		if [ -n "$$SHLIB_SOVER" ]; then \
> + 			[ -e "$$SHLIB$$SHLIB_SUFFIX" ] || \
> + 			( $(SET_X); rm -f $$SHLIB$$SHLIB_SUFFIX; \
> +-			  ln -s $$prev $$SHLIB$$SHLIB_SUFFIX ); \
> ++			  ln -sf $$prev $$SHLIB$$SHLIB_SUFFIX ); \
> + 		fi; \
> + 	fi
> + 
> +-- 
> +1.9.1
> +
Cheers,
Peter

[Buildroot] [PATCH] openssl: fix race condition when symlink shared libs

[Ryan Barnett]

Hi Peter,

On Thu, Nov 19, 2015 at 11:37 PM, Peter Rosin <peda@lysator.liu.se> wrote:
>
> On 2015-11-19 20:50, Ryan Barnett wrote:
>
>> +
>> +diff --git a/Makefile.shared b/Makefile.shared
>> +index 8d57163..ce66574 100644
>> +--- a/Makefile.shared
>> ++++ b/Makefile.shared
>> +@@ -118,14 +118,14 @@ SYMLINK_SO=    \
>> +             if [ -n "$$SHLIB_COMPAT" ]; then \
>> +                     for x in $$SHLIB_COMPAT; do \
>> +                             ( $(SET_X); rm -f $$SHLIB$$x$$SHLIB_SUFFIX; \
>> +-                              ln -s $$prev $$SHLIB$$x$$SHLIB_SUFFIX ); \
>> ++                              ln -sf $$prev $$SHLIB$$x$$SHLIB_SUFFIX ); \
>
> I thought the problem was that the two parties raced, not that the symlink couldn't be
> created. Thus, the key part was to replace "rm" with the force flag of ln, so that race
> loser does a no-operation instead of (temporarily) clobbering the state for the race
> winner. So, you apparently only do half the job in this patch...

The errors are as follows:

  ln: failed to create symbolic link 'libssl.so': File exists
  ln: creating symbolic link `libcrypto.so': File exists

ln is failing to create a symbolic link because the link already
exists. As Arnout described, there are two parallel paths that are
calling SYMLINK_SO, so how I understand it is that there a tiny chance
that both parallel paths call the 'rm -f' and then one creates a
symbolic link before the other. I think this still fixes the issue,
however, I think that the 'rm -f' is pointless because the 'ln -sf' is
more or less perform what was intended by the 'rm -f'.

Saying that though, I will remove the 'rm -f' and resend the patch.

Thanks,
-Ryan

-- 
Ryan Barnett / Sr Software Engineer
Airborne Information Systems / Secure Platforms
MS 131-100, C Ave NE, Cedar Rapids, IA, 52498, USA
ryan.barnett at rockwellcollins.com
www.rockwellcollins.com