[Buildroot] [PATCH 0/2] core/sdk: fix relative symlinks in generated tarball (branch yem/sdk)

[Buildroot] [PATCH 0/2] core/sdk: fix relative symlinks in generated tarball (branch yem/sdk)

[Yann E. MORIN]

Hello All!

This two-patch series is my counter-proposal to Joel's path to try and
fix the issue:
    https://patchwork.ozlabs.org/patch/1009168/

The first patch fixes the absolute symlinks, that were so far not
accounted for at all. A new helper scripts changes them into relative
symlinks.

The second patch is the actual fix to the issue Joel's patch was trying
to solve. I believe it is a better and simpler solution, although I have
to admit the patsubst trickery is not obvious.


Regards,
Yann E. MORIN.


The following changes since commit acb89c8d93952468fc9e24c7ddd9e361b8108e07

  .gitlab-ci.yml: regenerate after prosody tests addition (2018-12-06 23:15:20 +0100)


are available in the git repository at:

  git://git.buildroot.org/~ymorin/git/buildroot.git

for you to fetch changes up to 6ce3997e4824d41b50e13b8554e9f7e59e63a8af

  core/sdk: don't mangle symlinks with '.' or '..' at start (2018-12-07 18:59:41 +0100)


----------------------------------------------------------------
Joel Carlson (1):
      core/sdk: don't mangle symlinks with '.' or '..' at start

Yann E. MORIN (1):
      core: make symlinks relative when preparing the SDK

 Makefile                     |  5 +++--
 support/scripts/fix-symlinks | 53 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 2 deletions(-)
 create mode 100755 support/scripts/fix-symlinks

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/2] core: make symlinks relative when preparing the SDK

[Yann E. MORIN]

The SDK is supposed to be relocatable, so symlinks must not be
absolute.

Add a new helper script, that replaces all absolute symlinks with
relative ones.

The ideal solution would use the shortest relative path for the
destination, but it is non-trivial to come up with. We just ensure
the relative path does not cross a common base directory, and compute
all the paths relative to that anchor.

This can give non-optimum relative symlinks, like:
    /base-dir/bin/foo -> ../bin/bar

when the optimum would have been:
    /base-dir/bin/foo -> bar

Finally, as a sanity check, ensure there is not relative symlinl
pointing out of the base directory, because their targets would be
missing from the SDK.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Joel Carlson <JoelsonCarl@gmail.com>
---
 Makefile                     |  1 +
 support/scripts/fix-symlinks | 53 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)
 create mode 100755 support/scripts/fix-symlinks

diff --git a/Makefile b/Makefile
index 37df98520e..e8a1c7b366 100644
--- a/Makefile
+++ b/Makefile
@@ -586,6 +586,7 @@ prepare-sdk: world
 	@$(call MESSAGE,"Rendering the SDK relocatable")
 	$(TOPDIR)/support/scripts/fix-rpath host
 	$(TOPDIR)/support/scripts/fix-rpath staging
+	$(TOPDIR)/support/scripts/fix-symlinks "$(HOST_DIR)"
 	$(INSTALL) -m 755 $(TOPDIR)/support/misc/relocate-sdk.sh $(HOST_DIR)/relocate-sdk.sh
 	mkdir -p $(HOST_DIR)/share/buildroot
 	echo $(HOST_DIR) > $(HOST_DIR)/share/buildroot/sdk-location
diff --git a/support/scripts/fix-symlinks b/support/scripts/fix-symlinks
new file mode 100755
index 0000000000..b1190300e2
--- /dev/null
+++ b/support/scripts/fix-symlinks
@@ -0,0 +1,53 @@
+#!/usr/bin/env bash
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+# Transform all symlinks in ${1} from absolute to relative,
+# with the base of relativity anchored in ${1}
+#
+# Additionally, ensure that all existing symlinks point inside the
+# base directory.
+#
+# Note that the relativity of the symlink is not the shortest, e.g.
+# ${base}/bin/foo can end up as a symlink to ../bin/bar when it would
+# be shorter to have a symlink to ./bar, or even simply to bar. This
+# is not the nicest, but works and is easy to do.
+
+main() {
+    local base="${1}"
+    local link dest reldir
+    local -a links
+
+    links=( $(find "${base}" -type l -printf '%P\n') )
+    for link in "${links[@]}"; do
+        dest="$(readlink "${base}/${link}")"
+        case "${dest}" in
+          ("${base}"/*) ;; # We need to handle that one
+          (/*)
+            printf 'Absolute symlink outside of base "%s": "%s" -> "%s"\n' \
+                   "${base}" "${link}" "${dest}"
+            exit 1
+            ;;
+          (../*|*/../*|*/..)
+            # Not absolute, but does it cross the boundary?
+            dest="$(readlink -m "${base}/${link}")"
+            case "${dest}" in
+              ("${base}"/*) ;; # Anchored, OK
+              (*)
+                printf 'Relative symlink outside of base "%s": "%s" -> "%s"\n' \
+                       "${base}" "${link}" "${dest}"
+                exit 1
+                ;;
+            esac
+            continue # Not absolute, does not cross, OK
+            ;;
+          (*)
+            continue # Not absolute, does not cross, OK
+            ;;
+        esac
+        reldir="$(sed -r -e 's,([^/]+/),../,g; s,/[^/]+$,,' <<<"${link#${base}}")"
+        rm -f "${base}/${link}"
+        ln -sf "${reldir}${dest#${base}}" "${base}/${link}"
+    done
+}
+
+main "${@}"
-- 
2.14.1

[Buildroot] [PATCH 2/2] core/sdk: don't mangle symlinks with '.' or '..' at start

[Yann E. MORIN]

From: Joel Carlson <JoelsonCarl@gmail.com>

The current transform changes any '.' at the start of a filename to
$(BR2_SDK_PREFIX). This also applies to the target of a symlink, when
it is relative.

We thus might end up with something like:
    $(BR2_SDK_PREFIX)/bin/aarch64-linux-gnu-ar ->
    $(BR2_SDK_PREFIX)./opt/ext-toolchain/bin/aarch64-linux-gnu-ar

when it should be:
    $(BR2_SDK_PREFIX)/bin/aarch64-linux-gnu-ar ->
    ../opt/ext-toolchain/bin/aarch64-linux-gnu-ar

We fix that by making sure we always remove a known prefix, i.e. we
remove the path to host dir. The obvious solution would be to cd into
$(HOST_DIR)/.. , then tar ./host/ and finally use a --transfrom pattern
as 's,^\./$(notdir $(HOST_DIR)),$(BR2_SDK_PREFIX)'.

Since $(HOST_DIR) can point to a user-supplied location, we don't know
very well how the pattern may patch.

Instead, we cd into / and tar the full path to $(HOST_DIR).

Since tar removes any leading '/', it would spurr a warning message,
which is annoying. So we explicitly remove the leading '/' from
$(HOST_DIR) when we tar it.

Finally, we transform all filenames to replace a leading $(HOST_DIR)
(without a leading /) to the prefix to use.

Signed-off-by: Joel Carlson <JoelsonCarl@gmail.com>
[yann.morin.1998 at free.fr:
  - use a single transform pattern
  - use full HOST_DIR path as pattern to replace
  - update commit log accordingly
]
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

---
Changes v1 -> v2: (Yann)
  - changes in the commit log, above.
---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index e8a1c7b366..1a2783c477 100644
--- a/Makefile
+++ b/Makefile
@@ -599,8 +599,8 @@ sdk: prepare-sdk $(BR2_TAR_HOST_DEPENDENCY)
 	$(Q)mkdir -p $(BINARIES_DIR)
 	$(TAR) czf "$(BINARIES_DIR)/$(BR2_SDK_PREFIX).tar.gz" \
 		--owner=0 --group=0 --numeric-owner \
-		--transform='s#^\.#$(BR2_SDK_PREFIX)#' \
-		-C $(HOST_DIR) "."
+		--transform='s#^$(patsubst /%,%,$(HOST_DIR))#$(BR2_SDK_PREFIX)#' \
+		-C / $(patsubst /%,%,$(HOST_DIR))
 
 RSYNC_VCS_EXCLUSIONS = \
 	--exclude .svn --exclude .git --exclude .hg --exclude .bzr \
-- 
2.14.1

[Buildroot] [PATCH 1/2] core: make symlinks relative when preparing the SDK

[Joel Carlson]

On Fri, Dec 7, 2018 at 11:10 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>
> The SDK is supposed to be relocatable, so symlinks must not be
> absolute.
>
> Add a new helper script, that replaces all absolute symlinks with
> relative ones.
>
> The ideal solution would use the shortest relative path for the
> destination, but it is non-trivial to come up with. We just ensure
> the relative path does not cross a common base directory, and compute
> all the paths relative to that anchor.
>
> This can give non-optimum relative symlinks, like:
>     /base-dir/bin/foo -> ../bin/bar
>
> when the optimum would have been:
>     /base-dir/bin/foo -> bar
>
> Finally, as a sanity check, ensure there is not relative symlinl
> pointing out of the base directory, because their targets would be
> missing from the SDK.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Joel Carlson <JoelsonCarl@gmail.com>

Reviewed-by: Joel Carlson <JoelsonCarl@gmail.com>
Tested-by: Joel Carlson <JoelsonCarl@gmail.com>

Applied this patch and patch 2/2 to my branch and made sure the SDK
generated and the symlinks I had seen fail before looked to be
correct.
I also added a few of my own symlinks into host/ with absolute paths
pointing both inside and outside of host/ to test that the cases
caught them.
And I added some relative symlinks that crossed the boundary to make
sure that case was caught as well.

[Buildroot] [PATCH 2/2] core/sdk: don't mangle symlinks with '.' or '..' at start

[Joel Carlson]

On Fri, Dec 7, 2018 at 11:10 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>
> From: Joel Carlson <JoelsonCarl@gmail.com>
>
> The current transform changes any '.' at the start of a filename to
> $(BR2_SDK_PREFIX). This also applies to the target of a symlink, when
> it is relative.
>
> We thus might end up with something like:
>     $(BR2_SDK_PREFIX)/bin/aarch64-linux-gnu-ar ->
>     $(BR2_SDK_PREFIX)./opt/ext-toolchain/bin/aarch64-linux-gnu-ar
>
> when it should be:
>     $(BR2_SDK_PREFIX)/bin/aarch64-linux-gnu-ar ->
>     ../opt/ext-toolchain/bin/aarch64-linux-gnu-ar
>
> We fix that by making sure we always remove a known prefix, i.e. we
> remove the path to host dir. The obvious solution would be to cd into
> $(HOST_DIR)/.. , then tar ./host/ and finally use a --transfrom pattern
> as 's,^\./$(notdir $(HOST_DIR)),$(BR2_SDK_PREFIX)'.
>
> Since $(HOST_DIR) can point to a user-supplied location, we don't know
> very well how the pattern may patch.
>
> Instead, we cd into / and tar the full path to $(HOST_DIR).
>
> Since tar removes any leading '/', it would spurr a warning message,
> which is annoying. So we explicitly remove the leading '/' from
> $(HOST_DIR) when we tar it.
>
> Finally, we transform all filenames to replace a leading $(HOST_DIR)
> (without a leading /) to the prefix to use.
>
> Signed-off-by: Joel Carlson <JoelsonCarl@gmail.com>
> [yann.morin.1998 at free.fr:
>   - use a single transform pattern
>   - use full HOST_DIR path as pattern to replace
>   - update commit log accordingly
> ]
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>

So not sure what protocol is on tags when this is essentially v2 of my
own patch, but...
Reviewed-by: Joel Carlson <JoelsonCarl@gmail.com>
Tested-by: Joel Carlson <JoelsonCarl@gmail.com>

Applied this and the first patch to my branch and made sure my SDKs
seemed to have the correct links.

[Buildroot] [PATCH 2/2] core/sdk: don't mangle symlinks with '.' or '..' at start

[Yann E. MORIN]

Joel, All,

On 2018-12-07 12:39 -0700, Joel Carlson spake thusly:
> On Fri, Dec 7, 2018 at 11:10 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> > From: Joel Carlson <JoelsonCarl@gmail.com>
> > The current transform changes any '.' at the start of a filename to
> > $(BR2_SDK_PREFIX). This also applies to the target of a symlink, when
> > it is relative.
> >
> > We thus might end up with something like:
> >     $(BR2_SDK_PREFIX)/bin/aarch64-linux-gnu-ar ->
> >     $(BR2_SDK_PREFIX)./opt/ext-toolchain/bin/aarch64-linux-gnu-ar
> >
> > when it should be:
> >     $(BR2_SDK_PREFIX)/bin/aarch64-linux-gnu-ar ->
> >     ../opt/ext-toolchain/bin/aarch64-linux-gnu-ar
> >
> > We fix that by making sure we always remove a known prefix, i.e. we
> > remove the path to host dir. The obvious solution would be to cd into
> > $(HOST_DIR)/.. , then tar ./host/ and finally use a --transfrom pattern
> > as 's,^\./$(notdir $(HOST_DIR)),$(BR2_SDK_PREFIX)'.
> >
> > Since $(HOST_DIR) can point to a user-supplied location, we don't know
> > very well how the pattern may patch.
> >
> > Instead, we cd into / and tar the full path to $(HOST_DIR).
> >
> > Since tar removes any leading '/', it would spurr a warning message,
> > which is annoying. So we explicitly remove the leading '/' from
> > $(HOST_DIR) when we tar it.
> >
> > Finally, we transform all filenames to replace a leading $(HOST_DIR)
> > (without a leading /) to the prefix to use.
> >
> > Signed-off-by: Joel Carlson <JoelsonCarl@gmail.com>
> > [yann.morin.1998 at free.fr:
> >   - use a single transform pattern
> >   - use full HOST_DIR path as pattern to replace
> >   - update commit log accordingly
> > ]
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> >
> 
> So not sure what protocol is on tags when this is essentially v2 of my
> own patch, but...
> Reviewed-by: Joel Carlson <JoelsonCarl@gmail.com>
> Tested-by: Joel Carlson <JoelsonCarl@gmail.com>

Since my proposal is more than trivial tweaks of your proposal, your
tags are welcome.

Thanks for reporting the issue, for your initial patch which got me
kickstarted, and thanks for your review and tests! :-)

Regards,
Yann E. MORIN.

> Applied this and the first patch to my branch and made sure my SDKs
> seemed to have the correct links.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 0/2] core/sdk: fix relative symlinks in generated tarball (branch yem/sdk)

[Andreas Naumann]

Hello Yann, Joel,


Am 07.12.18 um 19:10 schrieb Yann E. MORIN:
> Hello All!
> 
> This two-patch series is my counter-proposal to Joel's path to try and
> fix the issue:
>      https://patchwork.ozlabs.org/patch/1009168/
> 
> The first patch fixes the absolute symlinks, that were so far not
> accounted for at all. A new helper scripts changes them into relative
> symlinks.
> 
> The second patch is the actual fix to the issue Joel's patch was trying
> to solve. I believe it is a better and simpler solution, although I have
> to admit the patsubst trickery is not obvious.

Thank you for these patches as they adress an SDK issue I have just found.
I have applied them on top of 2018.11 and verified the link ./usr -> ./ 
remains as is and is not changed into a dead link to 
./arm-buildroot-linux-gnueabihf_sdk-buildroot/.

However, the check for symlinks pointing out of host/ showed to two 
issues originating in install steps of iptables and eudev. Both create 
links that point to system pathes.

I have locally made POST_STAGING_HOOKS to fix that but that's probably a 
non acceptable hack.


Regards,
Andreas


> 
> 
> Regards,
> Yann E. MORIN.
> 
> 
> The following changes since commit acb89c8d93952468fc9e24c7ddd9e361b8108e07
> 
>    .gitlab-ci.yml: regenerate after prosody tests addition (2018-12-06 23:15:20 +0100)
> 
> 
> are available in the git repository at:
> 
>    git://git.buildroot.org/~ymorin/git/buildroot.git
> 
> for you to fetch changes up to 6ce3997e4824d41b50e13b8554e9f7e59e63a8af
> 
>    core/sdk: don't mangle symlinks with '.' or '..' at start (2018-12-07 18:59:41 +0100)
> 
> 
> ----------------------------------------------------------------
> Joel Carlson (1):
>        core/sdk: don't mangle symlinks with '.' or '..' at start
> 
> Yann E. MORIN (1):
>        core: make symlinks relative when preparing the SDK
> 
>   Makefile                     |  5 +++--
>   support/scripts/fix-symlinks | 53 ++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 56 insertions(+), 2 deletions(-)
>   create mode 100755 support/scripts/fix-symlinks
> 

[Buildroot] [PATCH 0/2] core/sdk: fix relative symlinks in generated tarball (branch yem/sdk)

[Joel Carlson]

Andreas,

On Fri, Dec 21, 2018 at 2:46 AM Andreas Naumann <dev@andin.de> wrote:

<snip>

> However, the check for symlinks pointing out of host/ showed to two
> issues originating in install steps of iptables and eudev. Both create
> links that point to system pathes.
>
> I have locally made POST_STAGING_HOOKS to fix that but that's probably a
> non acceptable hack.

Would be hard to tell without seeing the actual change. ;-)
That being said, this is probably something that would need to be
fixed in iptables and eudev (patches submitted upstream), and then
those patches added to buildroot to fix the issue.

So far I've submitted a patch to iptables here:
http://patchwork.ozlabs.org/patch/1017739/

And a patch to buildroot to pull in a version of that here:
https://patchwork.ozlabs.org/patch/1017740/

I've also submitted a pull request for eudev (and while typing this
email it was already merged):
https://github.com/gentoo/eudev/pull/165

I'll submit a patch for eudev in buildroot to pull in that change once
I see feedback on the iptables one. For it to work for me I needed to
set AUTORECONF=YES on eudev, but I don't really understand
autotools/automake stuff very well and I don't know if that is the
right way to be doing things.
-Joel

[Buildroot] [PATCH 0/2] core/sdk: fix relative symlinks in generated tarball (branch yem/sdk)

[Yann E. MORIN]

Joel, Andreas, All,

On 2018-12-21 17:13 -0700, Joel Carlson spake thusly:
> On Fri, Dec 21, 2018 at 2:46 AM Andreas Naumann <dev@andin.de> wrote:
> > However, the check for symlinks pointing out of host/ showed to two
> > issues originating in install steps of iptables and eudev. Both create
> > links that point to system pathes.
> >
> > I have locally made POST_STAGING_HOOKS to fix that but that's probably a
> > non acceptable hack.
> 
> Would be hard to tell without seeing the actual change. ;-)
> That being said, this is probably something that would need to be
> fixed in iptables and eudev (patches submitted upstream), and then
> those patches added to buildroot to fix the issue.
> 
> So far I've submitted a patch to iptables here:
> http://patchwork.ozlabs.org/patch/1017739/
> 
> And a patch to buildroot to pull in a version of that here:
> https://patchwork.ozlabs.org/patch/1017740/
> 
> I've also submitted a pull request for eudev (and while typing this
> email it was already merged):
> https://github.com/gentoo/eudev/pull/165

I did nt see Thomas' reply to the iptables patch before I replied to
your eudev one. It turns out we basically said exactly the same thing:
this is not the correct solution.

If you do what you propose, then you can see that at runtime, on the
_target_, the symlinks will be wrong, bnecause they would be prefixed
with the DESTDIR, which is clearly not appropriate.

A better solution, in that case, wuld be to make relative symlinks
instead, but that is a little bit trickier.  A simple way to do that is
to use 'ln -r -s FILE LINK' but the -r option is not old enough to be
widely available on all distors, especially entreprise-grade distros.

An other approach, would be that we whietelist certain PATHs from our
SDK. Afterall, there are things we do not care about in the SDK. For
example, most executables in staging/{,usr/}{,s}bin are not needed. We
only really need libs and headers, so we could just ignore anything
that is not in staging/{lib,usr/lib,usr/include}/

I.e. the heuristic would become:

  if it is a symlink:
      if is not below staging/:
          check and fix it
      else if it is below staging/{lib,usr/lib,usr/include}:
          check and fix it
      else
          ignore it

I can try to look into fixing that in my series. Is enabling eudev
and/or iptables enough to echibit the problem?

Regards,
Yann E. MORIN.

> I'll submit a patch for eudev in buildroot to pull in that change once
> I see feedback on the iptables one. For it to work for me I needed to
> set AUTORECONF=YES on eudev, but I don't really understand
> autotools/automake stuff very well and I don't know if that is the
> right way to be doing things.
> -Joel

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 0/2] core/sdk: fix relative symlinks in generated tarball (branch yem/sdk)

[Yann E. MORIN]

Andreas, All,

On 2018-12-21 10:46 +0100, Andreas Naumann spake thusly:
> Am 07.12.18 um 19:10 schrieb Yann E. MORIN:
> >This two-patch series is my counter-proposal to Joel's path to try and
> >fix the issue:
> >     https://patchwork.ozlabs.org/patch/1009168/
> >
> >The first patch fixes the absolute symlinks, that were so far not
> >accounted for at all. A new helper scripts changes them into relative
> >symlinks.
> >
> >The second patch is the actual fix to the issue Joel's patch was trying
> >to solve. I believe it is a better and simpler solution, although I have
> >to admit the patsubst trickery is not obvious.
> 
> Thank you for these patches as they adress an SDK issue I have just found.
> I have applied them on top of 2018.11 and verified the link ./usr -> ./
> remains as is and is not changed into a dead link to
> ./arm-buildroot-linux-gnueabihf_sdk-buildroot/.
> 
> However, the check for symlinks pointing out of host/ showed to two issues
> originating in install steps of iptables and eudev. Both create links that
> point to system pathes.

Ok, thanks for the report! :-)

> I have locally made POST_STAGING_HOOKS to fix that but that's probably a non
> acceptable hack.

Indeed no, please see my reply to Joel's suggestion instead.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 0/2] core/sdk: fix relative symlinks in generated tarball (branch yem/sdk)

[Joel Carlson]

On Sat, Dec 22, 2018 at 1:31 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>
> Joel, Andreas, All,
>
> On 2018-12-21 17:13 -0700, Joel Carlson spake thusly:
> > On Fri, Dec 21, 2018 at 2:46 AM Andreas Naumann <dev@andin.de> wrote:
> > > However, the check for symlinks pointing out of host/ showed to two
> > > issues originating in install steps of iptables and eudev. Both create
> > > links that point to system pathes.
> > >
> > > I have locally made POST_STAGING_HOOKS to fix that but that's probably a
> > > non acceptable hack.
> >
> > Would be hard to tell without seeing the actual change. ;-)
> > That being said, this is probably something that would need to be
> > fixed in iptables and eudev (patches submitted upstream), and then
> > those patches added to buildroot to fix the issue.
> >
> > So far I've submitted a patch to iptables here:
> > http://patchwork.ozlabs.org/patch/1017739/
> >
> > And a patch to buildroot to pull in a version of that here:
> > https://patchwork.ozlabs.org/patch/1017740/
> >
> > I've also submitted a pull request for eudev (and while typing this
> > email it was already merged):
> > https://github.com/gentoo/eudev/pull/165
>
> I did nt see Thomas' reply to the iptables patch before I replied to
> your eudev one. It turns out we basically said exactly the same thing:
> this is not the correct solution.

My apologies on submitting a bad patch.

> If you do what you propose, then you can see that at runtime, on the
> _target_, the symlinks will be wrong, bnecause they would be prefixed
> with the DESTDIR, which is clearly not appropriate.
>
> A better solution, in that case, wuld be to make relative symlinks
> instead, but that is a little bit trickier.  A simple way to do that is
> to use 'ln -r -s FILE LINK' but the -r option is not old enough to be
> widely available on all distors, especially entreprise-grade distros.
>
> An other approach, would be that we whietelist certain PATHs from our
> SDK. Afterall, there are things we do not care about in the SDK. For
> example, most executables in staging/{,usr/}{,s}bin are not needed. We
> only really need libs and headers, so we could just ignore anything
> that is not in staging/{lib,usr/lib,usr/include}/
>
> I.e. the heuristic would become:
>
>   if it is a symlink:
>       if is not below staging/:
>           check and fix it
>       else if it is below staging/{lib,usr/lib,usr/include}:
>           check and fix it
>       else
>           ignore it
>
> I can try to look into fixing that in my series. Is enabling eudev
> and/or iptables enough to echibit the problem?

That is a better solution. And yes, with the previous sdk fixes
applied, just enabling eudev and/or iptables and then trying to 'make
sdk' would error out because of the symlink path being outside of
staging.
> Regards,
> Yann E. MORIN.
>
> > I'll submit a patch for eudev in buildroot to pull in that change once
> > I see feedback on the iptables one. For it to work for me I needed to
> > set AUTORECONF=YES on eudev, but I don't really understand
> > autotools/automake stuff very well and I don't know if that is the
> > right way to be doing things.
> > -Joel
>
> --
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 0/2] core/sdk: fix relative symlinks in generated tarball (branch yem/sdk)

[Yann E. MORIN]

Joel, All,

On 2018-12-22 11:21 -0700, Joel Carlson spake thusly:
> On Sat, Dec 22, 2018 at 1:31 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
[--SNIP--]
> > I did nt see Thomas' reply to the iptables patch before I replied to
> > your eudev one. It turns out we basically said exactly the same thing:
> > this is not the correct solution.
> My apologies on submitting a bad patch.

Hey, no worries! :-)  That's what reviews are for.

> >   if it is a symlink:
> >       if is not below staging/:
> >           check and fix it
> >       else if it is below staging/{lib,usr/lib,usr/include}:
> >           check and fix it
> >       else
> >           ignore it
> >
> > I can try to look into fixing that in my series. Is enabling eudev
> > and/or iptables enough to echibit the problem?
> 
> That is a better solution. And yes, with the previous sdk fixes
> applied, just enabling eudev and/or iptables and then trying to 'make
> sdk' would error out because of the symlink path being outside of
> staging.

Yeah, I eventually managed to reproduce the issue, indeed. so I
submitted v3 of the series. Thanks! :-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 0/2] core/sdk: fix relative symlinks in generated tarball (branch yem/sdk)

[Trent Piepho]

On Sat, 2018-12-22 at 11:21 -0700, Joel Carlson wrote:
> On Sat, Dec 22, 2018 at 1:31 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> > 
> > 
> > An other approach, would be that we whietelist certain PATHs from our
> > SDK. Afterall, there are things we do not care about in the SDK. For
> > example, most executables in staging/{,usr/}{,s}bin are not needed. We
> > only really need libs and headers, so we could just ignore anything
> > that is not in staging/{lib,usr/lib,usr/include}/

With per-package staging, could this be done before the per-package
staging areas are combined?  The idea would be to reduce differences
between the stage in buildroot and the stage when using an SDK.  Every
difference is a possible source of problems, even if it is something
that we are not supposed to care about.

Could this be taken further, and everything not in the stage whitelist
be deleted?  It would be nice if the SDK were smaller.