[OpenWrt-Devel] [PATCH] package/base-files: caldata: work around dd's limitation

[mail at adrianschmutzler.de]

Hi,

> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces at lists.openwrt.org]
> On Behalf Of Thibaut VARÈNE
> Sent: Sonntag, 17. Mai 2020 20:29
> To: openwrt-devel at lists.openwrt.org
> Cc: Thibaut VARÈNE <hacks at slashdirt.org>
> Subject: [OpenWrt-Devel] [PATCH] package/base-files: caldata: work around
> dd's limitation
> 
> tl;dr: dd will silently truncate the output if reading from special files (e.g. sysfs
> attributes) with a too large bs parameter.
> 
> This problem was exposed on some RouterBOARD ipq40xx devices which use
> a caldata payload which is larger than PAGE_SIZE, contrary to all other
> currently supported RouterBOARD devices: the caldata would fail to properly
> load with the current scripts.
> 
> Background: dd doesn't seem to correctly handle read() results that return
> less than requested data. sysfs attributes have a kernel exchange buffer
> which is at most PAGE_SIZE big, so only 1 page can be read() at a time. In this
> case, if bs is larger than PAGE_SIZE, dd will silently truncate blocks to
> PAGE_SIZE. With the current scripts using bs=<size> count=1, the data is
> truncated to PAGE_SIZE as soon as the requested <size> exceeds this value.
> 
> This commit works around this problem by using `cat` in the caldata routines
> that can read from a file (routines that read from mtd devices are
> untouched). cat correctly handles partial read requests. The output is then
> piped to dd with the same parameters as before, to ensure that the resulting
> file remains exactly the same.
> 
> This is a simple workaround, the downside is that it uses a pipe and one more
> executable, and therefore has a larger memory footprint and is slower. This
> is deemed acceptable considering these routines are only used at boot time.
> 
> Tested-by: Robert Marko <robimarko at gmail.com>
> Signed-off-by: Thibaut VARÈNE <hacks at slashdirt.org>
> ---
>  package/base-files/Makefile                       | 2 +-
>  package/base-files/files/lib/functions/caldata.sh | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/package/base-files/Makefile b/package/base-files/Makefile index
> d8e7c31878..5fb275533d 100644
> --- a/package/base-files/Makefile
> +++ b/package/base-files/Makefile
> @@ -12,7 +12,7 @@ include $(INCLUDE_DIR)/version.mk  include
> $(INCLUDE_DIR)/feeds.mk
> 
>  PKG_NAME:=base-files
> -PKG_RELEASE:=220
> +PKG_RELEASE:=221
>  PKG_FLAGS:=nonshared
> 
>  PKG_FILE_DEPENDS:=$(PLATFORM_DIR)/
> $(GENERIC_PLATFORM_DIR)/base-files/
> diff --git a/package/base-files/files/lib/functions/caldata.sh b/package/base-
> files/files/lib/functions/caldata.sh
> index 6862da7164..8b031e29cd 100644
> --- a/package/base-files/files/lib/functions/caldata.sh
> +++ b/package/base-files/files/lib/functions/caldata.sh
> @@ -64,7 +64,7 @@ caldata_from_file() {
> 
>  	[ -n "$target" ] || target=/lib/firmware/$FIRMWARE
> 
> -	dd if=$source of=$target iflag=skip_bytes bs=$count skip=$offset
> count=1 2>/dev/null || \
> +	cat $source | dd of=$target iflag=skip_bytes bs=$count skip=$offset

These lines should receive a short comment directly next to the code, as otherwise somebody will "improve" this by removing the cat again when nobody remembers your patch anymore. I already see a "remove useless cat" commit title in front of my eye :-)

Best

Adrian

> +count=1 2>/dev/null || \
>  		caldata_die "failed to extract calibration data from $source"
>  }
> 
> @@ -74,12 +74,12 @@ caldata_sysfsload_from_file() {
>  	local count=$(($3))
> 
>  	# test extract to /dev/null first
> -	dd if=$source of=/dev/null iflag=skip_bytes bs=$count skip=$offset
> count=1 2>/dev/null || \
> +	cat $source | dd of=/dev/null iflag=skip_bytes bs=$count
> skip=$offset
> +count=1 2>/dev/null || \
>  		caldata_die "failed to extract calibration data from $source"
> 
>  	# can't fail now
>  	echo 1 > /sys/$DEVPATH/loading
> -	dd if=$source of=/sys/$DEVPATH/data iflag=skip_bytes bs=$count
> skip=$offset count=1 2>/dev/null
> +	cat $source | dd of=/sys/$DEVPATH/data iflag=skip_bytes bs=$count
> +skip=$offset count=1 2>/dev/null
>  	echo 0 > /sys/$DEVPATH/loading
>  }
> 
> --
> 2.11.0
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel at lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: openpgp-digital-signature.asc
Type: application/pgp-signature
Size: 834 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/openwrt-devel/attachments/20200517/79143c28/attachment.sig>
-------------- next part --------------
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel