| Message ID | 20230620170726.3644-1-ckhardin@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/1] package/dhcp: fix dhcp symlink in target/var/lib/dhcp | expand |
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
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, 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
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 \
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(-)