[Buildroot] [PATCH 1/1] package/swupdate: update service/init configs

Arnout Vandecappelle arnout at mind.be
Sat Aug 28 12:44:03 UTC 2021


 Hi James,

 "update" in the subject is not really helpful.

On 28/08/2021 06:46, James Hilliard wrote:
> Set SKIP_STRIP=y so that the swupdate binary isn't stripped.

 We can see that from the patch - instead you should explain why we want to skip
strip.

 Also, this has nothing to do with systemd configs, so should be a separate patch.

 In this case for sure, because I think it might be something for master.


> Introduce BR2_PACKAGE_SWUPDATE_WEBSERVER so that we can control
> the required webserver config installation when enabled.

 This also should be a separate patch (though less important in this case).


> Ensure swupdate systemd support is always set based on buildroot
> using systemd as this is required for the systemd services.
> 
> Use swupdate's make install so that we install swupdate tools needed
> for services.
> 
> Adapted from:
> https://github.com/sbabic/meta-swupdate/tree/abc520d97482232302046459715845a025aca7d6/recipes-support/swupdate/swupdate
> https://github.com/sbabic/meta-swupdate/blob/abc520d97482232302046459715845a025aca7d6/recipes-support/swupdate/swupdate.inc
> 
> Signed-off-by: James Hilliard <james.hilliard1 at gmail.com>
> ---
>  package/swupdate/10-mongoose-args          |  1 +
>  package/swupdate/90-start-progress         |  1 +
>  package/swupdate/Config.in                 |  7 ++++
>  package/swupdate/swupdate-progress.service |  9 +++++
>  package/swupdate/swupdate-usb.rules        |  2 +
>  package/swupdate/swupdate-usb at .service     |  8 ++++
>  package/swupdate/swupdate.mk               | 43 +++++++++++++++++++---
>  package/swupdate/swupdate.socket           | 11 ++++++
>  package/swupdate/tmpfiles-swupdate.conf    |  2 +
>  9 files changed, 79 insertions(+), 5 deletions(-)
>  create mode 100644 package/swupdate/10-mongoose-args
>  create mode 100644 package/swupdate/90-start-progress
>  create mode 100644 package/swupdate/swupdate-progress.service
>  create mode 100644 package/swupdate/swupdate-usb.rules
>  create mode 100644 package/swupdate/swupdate-usb at .service
>  create mode 100644 package/swupdate/swupdate.socket
>  create mode 100644 package/swupdate/tmpfiles-swupdate.conf
> 
> diff --git a/package/swupdate/10-mongoose-args b/package/swupdate/10-mongoose-args
> new file mode 100644
> index 0000000000..896e66ba86
> --- /dev/null
> +++ b/package/swupdate/10-mongoose-args
> @@ -0,0 +1 @@
> +SWUPDATE_WEBSERVER_ARGS="-r /www ${SWUPDATE_MONGOOSE_EXTRA_ARGS:--p 8080}"

 Is this correct? The webserver is installed in /var/www, not /www...

> diff --git a/package/swupdate/90-start-progress b/package/swupdate/90-start-progress
> new file mode 100644
> index 0000000000..3b0eb5e7d7
> --- /dev/null
> +++ b/package/swupdate/90-start-progress
> @@ -0,0 +1 @@
> +exec /usr/bin/swupdate-progress -w -r &
> diff --git a/package/swupdate/Config.in b/package/swupdate/Config.in
> index f601e45fcd..6b832cfcbf 100644
> --- a/package/swupdate/Config.in
> +++ b/package/swupdate/Config.in
> @@ -60,9 +60,16 @@ config BR2_PACKAGE_SWUPDATE_CONFIG
>  	  I you wish to use your own modified swupdate configuration
>  	  file specify the config file location with this option.
>  
> +config BR2_PACKAGE_SWUPDATE_WEBSERVER
> +	bool "swupdate webserver"
> +	default y
> +	help
> +	  Enable update from remote using a web server on the target.
> +
>  config BR2_PACKAGE_SWUPDATE_INSTALL_WEBSITE
>  	bool "install default website"
>  	default y
> +	depends on BR2_PACKAGE_SWUPDATE_WEBSERVER

 I could be mistaken, but isn't the website perfectly usable with an external
webserver as well? You just have to provide the upload endpoint IIRC.

>  	help
>  	  Install the provided website to /var/www/swupdate.
>  
> diff --git a/package/swupdate/swupdate-progress.service b/package/swupdate/swupdate-progress.service
> new file mode 100644
> index 0000000000..0d464bb420
> --- /dev/null
> +++ b/package/swupdate/swupdate-progress.service
> @@ -0,0 +1,9 @@
> +[Unit]
> +Description=swupdate progress service
> +After=swupdate.service
> +
> +[Service]
> +ExecStart=/usr/bin/swupdate-progress -r -w
> +
> +[Install]
> +WantedBy=swupdate.service

 I suspect this is copied verbatim from meta-swupdate... It's good form to then
add a comment in the beginning of the file that specifies the upstream copyright
and license (in SPDX) and an upstream link.

 Same for all other files you copied, of course.

> diff --git a/package/swupdate/swupdate-usb.rules b/package/swupdate/swupdate-usb.rules
> new file mode 100644
> index 0000000000..b4efd0b733
> --- /dev/null
> +++ b/package/swupdate/swupdate-usb.rules
> @@ -0,0 +1,2 @@
> +ACTION=="add", KERNEL=="sd*", SUBSYSTEM=="block", ENV{ID_BUS}=="usb", ENV{ID_FS_USAGE}=="filesystem", TAG+="systemd", ENV{SYSTEMD_WANTS}+="swupdate-usb@%k.service"

 Hm, I wonder if it's a good idea to install this unconditionally... It feels to
me that this is a possible attack vector that the user should be aware of - and
the best way to do that is to make the whole thing optional.

 But really, I kind of wonder if this is something that Buildroot should
provide. Ideally, this should be part of the swupdate documentation, and if you
actually want automatic updates when a USB storage device is plugged in, you can
put that stuff in an overlay.


> diff --git a/package/swupdate/swupdate-usb at .service b/package/swupdate/swupdate-usb at .service
> new file mode 100644
> index 0000000000..eda9d1539c
> --- /dev/null
> +++ b/package/swupdate/swupdate-usb at .service
> @@ -0,0 +1,8 @@
> +[Unit]
> +Description=usb media swupdate service
> +Requires=swupdate-progress.service
> +
> +[Service]
> +ExecStartPre=/bin/mount /dev/%I /mnt
> +ExecStart=/bin/sh -c "swupdate-client -v /mnt/*.swu"
> +ExecStopPost=/bin/umount /mnt
> diff --git a/package/swupdate/swupdate.mk b/package/swupdate/swupdate.mk
> index 402aa0a91f..3368b6ef6a 100644
> --- a/package/swupdate/swupdate.mk
> +++ b/package/swupdate/swupdate.mk
> @@ -22,7 +22,7 @@ SWUPDATE_LICENSE_FILES = LICENSES/BSD-1-Clause.txt \
>  # swupdate uses $CROSS-cc instead of $CROSS-gcc, which is not
>  # available in all external toolchains, and use CC for linking. Ensure
>  # TARGET_CC is used for both.
> -SWUPDATE_MAKE_ENV = CC="$(TARGET_CC)" LD="$(TARGET_CC)"
> +SWUPDATE_MAKE_ENV = CC="$(TARGET_CC)" LD="$(TARGET_CC)" SKIP_STRIP=y
>  
>  # swupdate bundles its own version of mongoose (version 6.16)
>  
> @@ -139,6 +139,13 @@ endif
>  
>  ifeq ($(BR2_PACKAGE_SYSTEMD),y)
>  SWUPDATE_DEPENDENCIES += systemd
> +define SWUPDATE_SET_SYSTEMD
> +	$(call KCONFIG_ENABLE_OPT,CONFIG_SYSTEMD)
> +endef
> +else
> +define SWUPDATE_SET_SYSTEMD
> +	$(call KCONFIG_DISABLE_OPT,CONFIG_SYSTEMD)
> +endef
>  endif
>  
>  ifeq ($(BR2_PACKAGE_LIBUBOOTENV),y)
> @@ -180,6 +187,16 @@ ifeq ($(BR2_PACKAGE_LIBRSYNC),y)
>  SWUPDATE_DEPENDENCIES += librsync
>  endif
>  
> +ifeq ($(BR2_PACKAGE_SWUPDATE_WEBSERVER),y)
> +define SWUPDATE_SET_WEBSERVER
> +	$(call KCONFIG_ENABLE_OPT,CONFIG_WEBSERVER)
> +endef
> +else
> +define SWUPDATE_SET_WEBSERVER
> +	$(call KCONFIG_DISABLE_OPT,CONFIG_WEBSERVER)
> +endef
> +endif
> +
>  SWUPDATE_BUILD_CONFIG = $(@D)/.config
>  
>  SWUPDATE_KCONFIG_FILE = $(call qstrip,$(BR2_PACKAGE_SWUPDATE_CONFIG))
> @@ -192,14 +209,17 @@ SWUPDATE_MAKE_OPTS = \
>  
>  define SWUPDATE_KCONFIG_FIXUP_CMDS
>  	$(SWUPDATE_SET_LUA_VERSION)
> +	$(SWUPDATE_SET_SYSTEMD)
> +	$(SWUPDATE_SET_WEBSERVER)
>  endef
>  
>  define SWUPDATE_BUILD_CMDS
> -	$(TARGET_MAKE_ENV) $(SWUPDATE_MAKE_ENV) $(MAKE) $(SWUPDATE_MAKE_OPTS) -C $(@D)
> +	$(TARGET_MAKE_ENV) $(SWUPDATE_MAKE_ENV) $(MAKE) -C $(@D) $(SWUPDATE_MAKE_OPTS)
>  endef
>  
>  define SWUPDATE_INSTALL_TARGET_CMDS
> -	$(INSTALL) -D -m 0755 $(@D)/swupdate $(TARGET_DIR)/usr/bin/swupdate
> +	$(TARGET_MAKE_ENV) $(SWUPDATE_MAKE_ENV) $(MAKE) -C $(@D) \
> +		$(SWUPDATE_MAKE_OPTS) DESTDIR=$(TARGET_DIR) install
>  	$(if $(BR2_PACKAGE_SWUPDATE_INSTALL_WEBSITE), \
>  		mkdir -p $(TARGET_DIR)/var/www/swupdate; \
>  		cp -dpfr $(@D)/examples/www/v2/* $(TARGET_DIR)/var/www/swupdate)
> @@ -213,23 +233,36 @@ $(error No Swupdate configuration file specified, check your BR2_PACKAGE_SWUPDAT
>  endif
>  endif
>  
> -ifeq ($(BR2_PACKAGE_SWUPDATE_INSTALL_WEBSITE),y)
>  define SWUPDATE_INSTALL_COMMON
>  	mkdir -p $(TARGET_DIR)/etc/swupdate/conf.d \
>  		$(TARGET_DIR)/usr/lib/swupdate/conf.d
>  	$(INSTALL) -D -m 755 package/swupdate/swupdate.sh \
>  		$(TARGET_DIR)/usr/lib/swupdate/swupdate.sh
> +	$(if $(BR2_PACKAGE_SWUPDATE_WEBSERVER), \

 Not saying that adding the Buildroot config option is bad, but another option
would be to simply check the setting of the webserver option in the swupdate config:

	if grep -q '^CONFIG_WEBSERVER=y' $(SWUPDATE_BUILD_CONFIG); then \
> +		$(INSTALL) -D -m 644 package/swupdate/10-mongoose-args \
> +			$(TARGET_DIR)/usr/lib/swupdate/conf.d/10-mongoose-args)
>  endef
>  define SWUPDATE_INSTALL_INIT_SYSTEMD
>  	$(SWUPDATE_INSTALL_COMMON)
>  	$(INSTALL) -D -m 644 package/swupdate/swupdate.service \
>  		$(TARGET_DIR)/usr/lib/systemd/system/swupdate.service
> +	$(INSTALL) -D -m 644 package/swupdate/swupdate.socket \
> +		$(TARGET_DIR)/usr/lib/systemd/system/swupdate.socket
> +	$(INSTALL) -D -m 644 package/swupdate/swupdate-usb at .service \
> +		$(TARGET_DIR)/usr/lib/systemd/system/swupdate-usb at .service
> +	$(INSTALL) -D -m 644 package/swupdate/swupdate-progress.service \
> +		$(TARGET_DIR)/usr/lib/systemd/system/swupdate-progress.service
> +	$(INSTALL) -D -m 644 package/swupdate/tmpfiles-swupdate.conf \
> +		$(TARGET_DIR)/usr/lib/tmpfiles.d/tmpfiles-swupdate.conf
> +	$(INSTALL) -D -m 644 package/swupdate/swupdate-usb.rules \
> +		$(TARGET_DIR)/lib/udev/rules.d/swupdate-usb.rules
>  endef
>  define SWUPDATE_INSTALL_INIT_SYSV
>  	$(SWUPDATE_INSTALL_COMMON)
>  	$(INSTALL) -D -m 755 package/swupdate/S80swupdate \
>  		$(TARGET_DIR)/etc/init.d/S80swupdate
> +	$(INSTALL) -D -m 644 package/swupdate/90-start-progress \
> +		$(TARGET_DIR)/usr/lib/swupdate/conf.d/90-start-progress
>  endef
> -endif
>  
>  $(eval $(kconfig-package))
> diff --git a/package/swupdate/swupdate.socket b/package/swupdate/swupdate.socket
> new file mode 100644
> index 0000000000..2b756714c5
> --- /dev/null
> +++ b/package/swupdate/swupdate.socket
> @@ -0,0 +1,11 @@
> +[Unit]
> +Description=SWUpdate socket listener
> +Documentation=https://github.com/sbabic/swupdate
> +Documentation=https://sbabic.github.io/swupdate
> +
> +[Socket]
> +ListenStream=/tmp/sockinstctrl
> +ListenStream=/tmp/swupdateprog
> +
> +[Install]
> +WantedBy=sockets.target
> diff --git a/package/swupdate/tmpfiles-swupdate.conf b/package/swupdate/tmpfiles-swupdate.conf
> new file mode 100644
> index 0000000000..4743672fcc
> --- /dev/null
> +++ b/package/swupdate/tmpfiles-swupdate.conf
> @@ -0,0 +1,2 @@
> +X /tmp/datadst
> +X /tmp/scripts

 Why are these needed? Are they only needed for systemd? If not, the S80swupdate
init script should also be updated to create those directories.

 Regards,
 Arnout


More information about the buildroot mailing list