[Buildroot] [PATCH 1/1] package/dhcp: fix dhcp symlink in target/var/lib/dhcp

[Buildroot] [PATCH 1/1] package/dhcp: fix dhcp symlink in target/var/lib/dhcp

[Charles Hardin]

In a readonly fs configuration the skeleton-init-fs will move
the var directories to the usr/share/factory and the run a command
like this:

   mv $(TARGET_DIR)/var $(TARGET_DIR)/usr/share/factory/var
   mkdir -p $(TARGET_DIR)/var
   for i in $(TARGET_DIR)/usr/share/factory/var/* \
            $(TARGET_DIR)/usr/share/factory/var/lib/* \
            $(TARGET_DIR)/usr/share/factory/var/lib/systemd/*; do

If the symlink is to the /tmp directory the glob compare on the
dhcp link will evaluate and result in an incorrect entry being
created in the resulting tmpfs mount on var.

    L+! /var/lib/dhcp - - - - ../usr/share/factory//var/lib/dhcp

So, make the link relative and the result of the configuration
doesn't get impacted.

Signed-off-by: Charles Hardin <ckhardin@gmail.com>
---
 package/dhcp/dhcp.mk | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/package/dhcp/dhcp.mk b/package/dhcp/dhcp.mk
index 7be40191c8..edbc6a0d2a 100644
--- a/package/dhcp/dhcp.mk
+++ b/package/dhcp/dhcp.mk
@@ -103,7 +103,7 @@ define DHCP_INSTALL_CTL_LIBS
 endef
 define DHCP_INSTALL_SERVER
 	mkdir -p $(TARGET_DIR)/var/lib
-	(cd $(TARGET_DIR)/var/lib; ln -snf /tmp dhcp)
+	(cd $(TARGET_DIR)/var/lib; ln -snf ../../tmp dhcp)
 	$(MAKE) -C $(@D)/server DESTDIR=$(TARGET_DIR) install-sbinPROGRAMS
 	$(INSTALL) -m 0644 -D package/dhcp/dhcpd.conf \
 		$(TARGET_DIR)/etc/dhcp/dhcpd.conf
@@ -113,7 +113,7 @@ endif
 ifeq ($(BR2_PACKAGE_DHCP_RELAY),y)
 define DHCP_INSTALL_RELAY
 	mkdir -p $(TARGET_DIR)/var/lib
-	(cd $(TARGET_DIR)/var/lib; ln -snf /tmp dhcp)
+	(cd $(TARGET_DIR)/var/lib; ln -snf ../../tmp dhcp)
 	$(MAKE) -C $(@D)/relay DESTDIR=$(TARGET_DIR) install-sbinPROGRAMS
 endef
 endif
@@ -121,7 +121,7 @@ endif
 ifeq ($(BR2_PACKAGE_DHCP_CLIENT),y)
 define DHCP_INSTALL_CLIENT
 	mkdir -p $(TARGET_DIR)/var/lib
-	(cd $(TARGET_DIR)/var/lib; ln -snf /tmp dhcp)
+	(cd $(TARGET_DIR)/var/lib; ln -snf ../../tmp dhcp)
 	$(MAKE) -C $(@D)/client DESTDIR=$(TARGET_DIR) sbindir=/sbin \
 		install-sbinPROGRAMS
 	$(INSTALL) -m 0644 -D package/dhcp/dhclient.conf \
-- 
2.39.2 (Apple Git-143)

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/dhcp: fix dhcp symlink in target/var/lib/dhcp

[Thomas Petazzoni via buildroot]

Hello Charles,

+Yann in Cc. I will ask a few questions below, but honestly I'm not
super familiar with this part of the code base, so I might be asking
some stupid questions.

On Tue, 20 Jun 2023 10:07:26 -0700
Charles Hardin <ckhardin@gmail.com> wrote:

> In a readonly fs configuration the skeleton-init-fs will move

I supposed you meant skeleton-init-systemd. Indeed, there's nothing
named skeleton-init-fs in Buildroot as far as I know.

> the var directories to the usr/share/factory and the run a command
> like this:
> 
>    mv $(TARGET_DIR)/var $(TARGET_DIR)/usr/share/factory/var
>    mkdir -p $(TARGET_DIR)/var
>    for i in $(TARGET_DIR)/usr/share/factory/var/* \
>             $(TARGET_DIR)/usr/share/factory/var/lib/* \
>             $(TARGET_DIR)/usr/share/factory/var/lib/systemd/*; do
> 
> If the symlink is to the /tmp directory the glob compare on the
> dhcp link will evaluate and result in an incorrect entry being
> created in the resulting tmpfs mount on var.

I'm not able to parse your explanation here. Which glob compare? Which
evaluation?

> 
>     L+! /var/lib/dhcp - - - - ../usr/share/factory//var/lib/dhcp

Not obvious to me what is invalid in this entry, but admittedly I have
not done a test build/test run to investigate more.

Perhaps you can give a little bit more details to make the issue
obvious to the reviewer, and therefore help your patch to be merged?

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/dhcp: fix dhcp symlink in target/var/lib/dhcp

[Thomas Petazzoni via buildroot]

Hello Charles,

+Yann in Cc. I will ask a few questions below, but honestly I'm not
super familiar with this part of the code base, so I might be asking
some stupid questions.

On Tue, 20 Jun 2023 10:07:26 -0700
Charles Hardin <ckhardin@gmail.com> wrote:

> In a readonly fs configuration the skeleton-init-fs will move

I supposed you meant skeleton-init-systemd. Indeed, there's nothing
named skeleton-init-fs in Buildroot as far as I know.

> the var directories to the usr/share/factory and the run a command
> like this:
> 
>    mv $(TARGET_DIR)/var $(TARGET_DIR)/usr/share/factory/var
>    mkdir -p $(TARGET_DIR)/var
>    for i in $(TARGET_DIR)/usr/share/factory/var/* \
>             $(TARGET_DIR)/usr/share/factory/var/lib/* \
>             $(TARGET_DIR)/usr/share/factory/var/lib/systemd/*; do
> 
> If the symlink is to the /tmp directory the glob compare on the
> dhcp link will evaluate and result in an incorrect entry being
> created in the resulting tmpfs mount on var.

I'm not able to parse your explanation here. Which glob compare? Which
evaluation?

> 
>     L+! /var/lib/dhcp - - - - ../usr/share/factory//var/lib/dhcp

Not obvious to me what is invalid in this entry, but admittedly I have
not done a test build/test run to investigate more.

Perhaps you can give a little bit more details to make the issue
obvious to the reviewer, and therefore help your patch to be merged?

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/dhcp: fix dhcp symlink in target/var/lib/dhcp

[Yann E. MORIN]

Thomas, Chrles, Al,

On 2023-07-10 20:47 +0200, Thomas Petazzoni spake thusly:
> +Yann in Cc. I will ask a few questions below, but honestly I'm not
> super familiar with this part of the code base, so I might be asking
> some stupid questions.

There are no stupid questions, only stupid answers (which is customary
for me to provide).

> On Tue, 20 Jun 2023 10:07:26 -0700
> Charles Hardin <ckhardin@gmail.com> wrote:
> > the var directories to the usr/share/factory and the run a command
> > like this:
> > 
> >    mv $(TARGET_DIR)/var $(TARGET_DIR)/usr/share/factory/var
> >    mkdir -p $(TARGET_DIR)/var
> >    for i in $(TARGET_DIR)/usr/share/factory/var/* \
> >             $(TARGET_DIR)/usr/share/factory/var/lib/* \
> >             $(TARGET_DIR)/usr/share/factory/var/lib/systemd/*; do
> > 
> > If the symlink is to the /tmp directory the glob compare on the
> > dhcp link will evaluate and result in an incorrect entry being
> > created in the resulting tmpfs mount on var.
> I'm not able to parse your explanation here. Which glob compare? Which
> evaluation?
> > 
> >     L+! /var/lib/dhcp - - - - ../usr/share/factory//var/lib/dhcp
> Not obvious to me what is invalid in this entry, but admittedly I have
> not done a test build/test run to investigate more.

This line is indeed incorrect: it means to create /var/lib/dhcp as a
symlink to ../usr/share/factory//var/lib/dhcp, i.e. to
/var/lib/../usr/share/factory//var/lib/dhcp, which in turn is made
cannonical as /var/usr/share/factory//var/lib/dhcp, which indeed does
not exist.

> Perhaps you can give a little bit more details to make the issue
> obvious to the reviewer, and therefore help your patch to be merged?

I think a better solution is to actually fix the factory:

   41 »   for i in $(TARGET_DIR)/usr/share/factory/var/* \
   42 »   »    $(TARGET_DIR)/usr/share/factory/var/lib/* \
   43 »   »    $(TARGET_DIR)/usr/share/factory/var/lib/systemd/*; do \
   44 »   »   [ -e "$${i}" ] || continue; \
   45 »   »   j="$${i#$(TARGET_DIR)/usr/share/factory}"; \
   46 »   »   if [ -L "$${i}" ]; then \
   47 »   »   »   printf "L+! %s - - - - %s\n" \
   48 »   »   »   »   "$${j}" "../usr/share/factory/$${j}" \

The ../ here should be replaced with as-many ../ as the fiel is deep
relative to /usr/share/factory/var/

i.e. probably something (untested) like:

       for i in $(TARGET_DIR)/usr/share/factory/var/* \
            $(TARGET_DIR)/usr/share/factory/var/lib/* \
            $(TARGET_DIR)/usr/share/factory/var/lib/systemd/*; do \
           [ -e "$${i}" ] || continue; \
           j="$${i#$(TARGET_DIR)/usr/share/factory}"; \
           if [ -L "$${i}" ]; then \
               d="$$(sed -r -e 's,([^./]+/),../,g' <<<$${j%/*}/)" \
               printf "L+! %s - - - - %s\n" \
                   "$${j}" "${d}/usr/share/factory/$${j}" \

This way, that should fix all absolute synlinks that are in an arbitrary
depth in the factory.

Regards,
Yann E. MORIN.

> Thanks!
> 
> Thomas
> -- 
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering and training
> https://bootlin.com

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot