#1030519 check-missing-firmware: patch for files with space characters, mediamount and more (with code)

Package:
hw-detect
Source:
hw-detect
Submitter:
Alexander Dalm
Date:
2023-05-31 10:24:03 UTC
Severity:
normal
#1030519#5
Date:
2023-02-04 11:44:27 UTC
From:
To:
Dear Maintainer,

based on my previous bug report #1029843 (wrong package selected), so I tried to fix it myself.

First up, I had a really hard time setting things up for debugging. Building the debian-installer is quite easy but modifying an existing package is not mentioned in the wiki. Since packages are downloaded during build/rebuild, even if they are already existing in /debian-installer/installer/build/apt.udeb/cache/archives you can't replace these files. As it turns out you can simply put them here: /debian-installer/installer/build/localudebs
Please add this link: https://d-i.debian.org/doc/internals/ch04.html#idm692
to the Questions section of: https://wiki.debian.org/DebianInstaller/Build
And please mention that localudebs can replace the ones on the mirror here as well: /debian-installer/installer/build/README

Building the hw-detect udep was challenging as well. The package Makefile contains no useful information, it seems there are only options to install this package. This link is slightly helpful since it mentions that they are build like normal packages: https://wiki.debian.org/BuildingUDebs
btw: "XC-Package-Type: udeb" should be changed to "Package-Type: udeb"
So, I build it with dpkg-deb like just like my driver packages:

1. get the source:
git clone https://salsa.debian.org/installer-team/hw-detect.git hw-detect

2. extract and copy the missing templates file from hw-detect_1.154_amd64.udeb/control.tar.xz/./templates to /hw-detect/debian/

3. create folder structure, control file and copy files:
mkdir -p hw-detect_patched/DEBIAN && mkdir hw-detect_patched/bin && mkdir -p hw-detect_patched/etc/hotplug.d/net && mkdir -p hw-detect_patched/lib/udev/rules.d && mkdir -p hw-	detect_patched/usr/lib/finish-install.d && mkdir hw-detect_patched/usr/lib/post-base-installer.d && mkdir hw-detect_patched/usr/lib/pre-pkgsel.d

echo $'Package: hw-detect\nVersion: 1.154\nArchitecture: amd64\nMaintainer: Debian Install System Team <debian-boot@lists.debian.org>\nInstalled-Size: 326\nDepends: rootskel, archdetect, cdebconf-udeb, di-utils, pciutils-udeb, udpkg\nSection: debian-installer\nPriority: standard\nDescription: Detect hardware and load kernel drivers for it' > hw-detect_patched/DEBIAN/control

cp hw-detect/debian/templates hw-detect_patched/DEBIAN/templates
cp hw-detect/check-missing-firmware.sh hw-detect_patched/bin/check-missing-firmware
cp hw-detect/hotplug-pcmcia.sh hw-detect_patched/bin/hotplug-pcmcia
cp hw-detect/hw-detect.sh hw-detect_patched/bin/hw-detect
cp hw-detect/sysfs-update-devnames.sh hw-detect_patched/bin/sysfs-update-devnames
cp hw-detect/net-hotplug.sh hw-detect_patched/etc/hotplug.d/net/hw-detect.hotplug
cp hw-detect/net-hotplug.rules hw-detect_patched/lib/udev/rules.d/010_net-hotplug.rules
cp hw-detect/hw-detect.finish-install.d/08hw-detect hw-detect_patched/usr/lib/finish-install.d/08hw-detect
cp hw-detect/hw-detect.finish-install.d/30hw-detect hw-detect_patched/usr/lib/finish-install.d/30hw-detect
cp hw-detect/hw-detect.post-base-installer.d/50install-firmware hw-detect_patched/usr/lib/post-base-installer.d/50install-firmware
cp hw-detect/hw-detect.post-base-installer.d/60install-mouseemu hw-detect_patched/usr/lib/post-base-installer.d/60install-mouseemu
cp hw-detect/hw-detect.pre-pkgsel.d/20install-hwpackages hw-detect_patched/usr/lib/pre-pkgsel.d/20install-hwpackages
cp hw-detect/hw-detect.pre-pkgsel.d/50install-firmware hw-detect_patched/usr/lib/pre-pkgsel.d/50install-firmware

4. replace/edit source files

5. build package into localudebs/ directory:
dpkg-deb --build hw-detect_patched debian-installer/installer/build/localudebs/hw-detect_patched.udeb

dpkg-deb has no option to output udeb (apart from the filename), specifying "XC-Package-Type: udeb" or "Package-Type: udeb" doesn't work either. It seems like the udep on the mirror is also just a .dep file named .udep since "Package-Type: udeb" is not specified in the control file despite being specified in the source control file.

Can these steps be added to the Makefile or what is the proper way to build the hw-detect udep?
I also found this link but I didn't tried it since the manual way works fine:
https://wiki.debian.org/DebianInstaller/Modify/CD#Create_a_udeb_packages_file
---

check-missing-firmware(.sh) changes:
	nic_is_configured()
		added skip_modules variable to skip removing/loading of modules that cause problems
			(intel_sst_acpi -> modprobe: FATAL: Module snd_intel_acpi is in use.)
	check_missing()
		added support for firmware-filenames with spaces:
			files variable stored as files file in tmp/
			dmesg set regex and output modified as pipe separated
			"while IFS='|' read" for pipe separated list
		bug fixed in file listing:
			when modules aren't or can't be removed/loaded, files don't appear in get_fresh_dmesg again
				dmesg list is appended to fwlist instead of overwriting the file
				actual removed/loaded modules are removed from fwlist (in main loop)
		added filter for one file requested multiple times in dmesg
			(actually happened on my Z83 Mini PC / Beelink BT3)
		optimized modules listing -> no additional Uniquify needed
	try_copy()
		added source path parameter
			this allows to use plain files in /firmware and /cdrom/firmware mounts (PXE initrd / CD / USB drive)
			currently only .deb files can be used here
			-> main use case: simply flash iso with Rufus and drag and dop plain files to firmware folder
	check_for_firmware()
		grepfor file removed since the files file is already line-separated
		"for filename in $dir/*.deb; do" changed to "for filename in $(ls $dir/*.deb); do"
			otherwise only one item e.g. "/cdrom/firmware/*.deb" is checked instead of the actual files
			this is probably a compatibility issue with the used ash shell in the netboot mini-ISO
			(I generally had a hard time testing/debugging between installed debian sh and iso, >30x bulding netinst)
	main loop (while check_missing && ask_load_firmware; do)
		added modified try_copy to /firmware and /cdrom/firmware to allow plain files
			(see try_copy)
		mountmedia; / mountmedia driver;
			despite discussed in #1029543, no one actually tested this, I did:
				calling mountmedia; works fine
				calling mountmedia driver; causes long loading times and multiple errors (see Logs)
			crude disabling removed (if [ "$loop" -lt 1 ]; then)
			"check_for_firmware /media /media/firmware" moved from "mountmedia driver;" to "mountmedia;"
			"mountmedia driver;" removed
		remove files from fwlist after remove/load of module
			(see bug in check_missing)
	multiple locations
		various changes for line-separated files file instad of space-separated variable
			(use diff tool to spot the changes)

I will submit this file and a screenshot as attachment in a further information mail (instead of creating a Gitlab account)

suggested change to templates file (localisations):
	With my changes in check-missing-firmware files are displayed with line breaks in the graphical installer but the first file is still shown in the same line.
	This looks a bit confusing especially when dealing with multiple files.
	I tried adding a leading line break using $(cat $flist | sed '1s/^/\n/') or creating a new file without success.
	but the teplates file can be changed from:
		The missing firmware files are: ${FILES}
		.
		If you have such media available now, insert it, and continue.
	to:
		The missing firmware files are:
		.
		${FILES}
		.
		If you have such media available now, insert it, and continue.
	to solve this issue. There might be also a way without inserting an empty line on top but simply adding a line break doesn't work.


Logs:

mainloop iteration #1
DEBUG: firmware directory not found
DEBUG: cdrom-firmware directory not found

[point where mountmedia is called]
kernel: FAT-fs (mmcblk0p1): Volume was not properly unmounted. Some data may be corrupt. Please run fsck.
kernel: FAT-fs (mmcblk0p1): Volume was not properly unmounted. Some data may be corrupt. Please run fsck.
DEBUG: mountmedia ok
DEBUG: try copy (fw_sst_22a8.bin)
DEBUG: try copy (fw_sst_22a8.binbrcmfmac43340-sdio.To be filled by O.E.M.-Z83.bin)
DEBUG: try copy (brcmfmac43340-sdio.bin)
kernel: FAT-fs (mmcblk0p1): Volume was not properly unmounted. Some data may be corrupt. Please run fsck.
kernel: FAT-fs (mmcblk0p1): Volume was not properly unmounted. Some data may be corrupt. Please run fsck.

[point where mountmedia driver is called]
mount: mounting /dev/mmcblk0 on /media failed: Invalid argument
unmount: can't unmount /media: Invalid argument
mount: mounting /dev/mmcblk0 on /media failed: Invalid argument
mount: mounting /dev/mmcblk0boot0 on /media failed: Invalid argument
unmount: can't unmount /media: Invalid argument
mount: mounting /dev/mmcblk0boot0 on /media failed: Invalid argument
mount: mounting /dev/mmcblk0boot1 on /media failed: Invalid argument
unmount: can't unmount /media: Invalid argument
mount: mounting /dev/mmcblk0boot1 on /media failed: Invalid argument
mount: mounting /dev/sda on /media failed: Invalid argument
unmount: can't unmount /media: Invalid argument
mount: mounting /dev/sda on /media failed: Invalid argument
mount: mounting /dev/mmcblk0boot0 on /media failed: Invalid argument
unmount: can't unmount /media: Invalid argument
mount: mounting /dev/mmcblk0boot0 on /media failed: Invalid argument
mount: mounting /dev/mmcblk0boot1 on /media failed: Invalid argument
unmount: can't unmount /media: Invalid argument
mount: mounting /dev/mmcblk0boot1 on /media failed: Invalid argument
mount: mounting /dev/mmcblk0 on /media failed: Invalid argument
unmount: can't unmount /media: Invalid argument
mount: mounting /dev/mmcblk0 on /media failed: Invalid argument
mount: mounting /dev/mmcblk0boot0 on /media failed: Invalid argument
unmount: can't unmount /media: Invalid argument
mount: mounting /dev/mmcblk0boot0 on /media failed: Invalid argument
mount: mounting /dev/mmcblk0boot1 on /media failed: Invalid argument
unmount: can't unmount /media: Invalid argument
mount: mounting /dev/mmcblk0boot1 on /media failed: Invalid argument
mount: mounting /dev/sda on /media failed: Invalid argument
unmount: can't unmount /media: Invalid argument
mount: mounting /dev/sda on /media failed: Invalid argument
mount: mounting /dev/mmcblk0boot0 on /media failed: Invalid argument
unmount: can't unmount /media: Invalid argument
mount: mounting /dev/mmcblk0boot0 on /media failed: Invalid argument
mount: mounting /dev/mmcblk0boot1 on /media failed: Invalid argument
unmount: can't unmount /media: Invalid argument
mount: mounting /dev/mmcblk0boot1 on /media failed: Invalid argument
mount: mounting /dev/mmcblk0p2 on /media failed: Invalid argument
unmount: can't unmount /media: Invalid argument
mount: mounting /dev/mmcblk0p2 on /media failed: Invalid argument
mount: mounting /dev/mmcblk0p3 on /media failed: Invalid argument
unmount: can't unmount /media: Invalid argument
mount: mounting /dev/mmcblk0p3 on /media failed: Invalid argument
mount: mounting /dev/sda1 on /media failed: Invalid argument
unmount: can't unmount /media: Invalid argument
mount: mounting /dev/sda1 on /media failed: Invalid argument
mount: mounting /dev/mmcblk0 on /media failed: Invalid argument
unmount: can't unmount /media: Invalid argument
mount: mounting /dev/mmcblk0 on /media failed: Invalid argument
mount: mounting /dev/mmcblk0boot0 on /media failed: Invalid argument
unmount: can't unmount /media: Invalid argument
mount: mounting /dev/mmcblk0boot0 on /media failed: Invalid argument
mount: mounting /dev/mmcblk0boot1 on /media failed: Invalid argument
unmount: can't unmount /media: Invalid argument
mount: mounting /dev/mmcblk0boot1 on /media failed: Invalid argument
kernel: FAT-fs (mmcblk0p1): Volume was not properly unmounted. Some data may be corrupt. Please run fsck.
kernel: FAT-fs (mmcblk0p1): Volume was not properly unmounted. Some data may be corrupt. Please run fsck.
DEBUG: mountmedia driver not ok

[error at: remove and reload modules so they see the new firmware]
removing and loding kernel module intel_sst_acpi
modprobe: FATAL: Module intel_sst_acpi not found.
modprobe: FATAL: Module intel_sst_acpi not found in directory /lib/modules/6.1.0-3-amd64
modprobe: removing and loading kernel module snd_intel_sst_acpi as well (actual module for intel_sst_acpi)
modprobe: FATAL: Module snd_intel_acpi is in use.

-> intel/fw_sst_22a8.bin not requested in next loop
#1030519#10
Date:
2023-02-04 11:56:47 UTC
From:
To:
here are promised files: see attachment
#1030519#15
Date:
2023-02-04 15:15:40 UTC
From:
To:
Hi Alexander,

Alexander Dalm <a.dalm2807@googlemail.com> (2023-02-04):
[…]

That's a Debian package, it's built with dpkg-buildpackage, just like
any other Debian package. That's why there's no specific information in
any Makefile, README, or wiki.

Attaching patches is best done by using git diff / git format-patch.

Not a good idea, seeing iwlwifi requests 68 files…


Cheers,
#1030519#22
Date:
2023-02-05 14:23:55 UTC
From:
To:
Hi Cyril,
sure, but this is an inconvenience for me as outside contributor and might
deter other ones as well.
I'm sure you and other core maintainers can import one file with ease...
Oh, nevermind ... I changed the format back to be space separated -> $(cat
$files | tr '\n' ' ' | sed 's/.$//')

I did some further testing with different storage devices SD-Card /
USB-Stick etc. and also noticed the issues described in #1029962.
Calling mountmedia; (without driver) only mounts the first 'mountable'
device even when it's empty. Calling mountmedia driver; pics the first
device with a *.deb, *.udeb or *.ude file on it.
To get both loose and deb files working reliably the mountmedia package has
to be updated. There are also some other issues caused by this package like
unnecessary double tries with 5 second sleep (-> long loading times), devices
can appear in two lists (-> four mount tries) and unsuppressed error
messages.
I basically copied and modified the core functionality as two new functions
check_directory() and find_firmware() as drop in replacement for mountmedia.

tested and working fine and fast:
   deb on USB-stick
   loose files on USB-stick
   deb on SD-card
   loose files on SD-card
   deb on Firmware partion of flashed netboot mini-ISO on USB-stick
   loose files on Firmware partition of flashed netboot mini-ISO on
USB-stick

notes:
   "for filename in $(ls $dir/*.deb); do" in check_for_firmware() reverted
back to "for filename in $dir/*.deb; do",
   the one item "/cdrom/firmware/*.deb" returned is normal behaviour if
nothing is found.
  (e.g. due to wrong device mounted by mountmedia)

see attached file.


Am Sa., 4. Feb. 2023 um 16:15 Uhr schrieb Cyril Brulebois <kibi@debian.org>:
#1030519#27
Date:
2023-02-06 09:45:53 UTC
From:
To:
Hi,

Alexander Dalm <a.dalm2807@googlemail.com> (2023-02-05):

The burden goes both ways. Sending your file over the wall is easy for
you but that means we end up reviewing this huge diff, without any kind
of logical split between changes:

 check-missing-firmware.sh | 185 ++++++++++++++++++++++++++++++++++------------
 1 file changed, 139 insertions(+), 46 deletions(-)

Anyway, playing with IFS is a recipe for a disaster, and why are we even
talking about files with space characters in the first place?

This is a little sad because there might be interesting bits in there
(like side-stepping moutmedia which is apparently not quite doing what
some users might expect, see #1029543, #1029962).


Cheers,
#1030519#32
Date:
2023-02-06 18:33:01 UTC
From:
To:
 Hi Cyril,

no worries, I reverted back to the original 1.154 version and implemented
just the find_firmware() and check_directory() fucntions to repace
mountmedia in: check-missing-firmware-1
I also added a second version check-missing-firmware-2 including the
modified try_copy() to allow loose files for PXE PXE initrd / CD / USB
drive.
No other changes like changing regex, IFS, moule and file filtering were
made at all -> the diff should be reasonable now ;)

I also attached some screenshots, dmesg and syslog files that might help to
resolve the remaining issues.

Regarding the spaces (copied parts from #1030636 for completeness):
The needed files for my WiFi module BCM43340 / CYW43340 / AP6234 are:
brcmfmac43340-sdio.bin -> required firmware file
brcmfmac43340-sdio.txt -> required NVRAM file
brcmfmac43340-sdio.clm_blob -> optional and not available for 43340

Depending on the Kernel version the brcmfmac module requests each file
first as
brcmfmac43340-sdio.[ManufacturerName]-[ProductName].bin / .txt / .clm_blob
and then as fallback:
brcmfmac43340-sdio.bin / .txt / .clm_blob
-> Debian 11.6: only .txt in [ManufacturerName]-[ProductName] format
-> Debian 12: all three files in [ManufacturerName]-[ProductName] format

My manufacturer was kinda lazy and left the ManufacturerName in BIOS as "To
be filled by O.E.M"
-> brcmfmac43340-sdio.To be filled by O.E.M.-Z83.bin


Am Mo., 6. Feb. 2023 um 10:45 Uhr schrieb Cyril Brulebois <kibi@debian.org>:
#1030519#49
Date:
2023-05-03 20:37:14 UTC
From:
To:
#1030519#60
Date:
2023-05-31 10:20:42 UTC
From:
To:
Hi Alexander,

I've been reviewing your patch and would like to suggest extracting the
following changes from it to consider and apply individually:

  1. Supporting firmware filenames that contain spaces.

  2. Removing (or at least reducing) the 5s wait[1] for USB devices to settle.

  3. Refactoring the fwfile 'for' loop[2] to use less-complicated parameter
     expansion (your changes didn't modify this but did highlight it
     potentially more complicated than necessary).

Each of these changes would require some description and a small patch -
writing those may require more time, I admit; the reward is that it makes it
easier for the maintainer to accept the changes.


During review, I considered these as possible other changes:

  * Loading the 'vfat' kernel module before mountpoint search.

  * Consulting the 'maybe-usb-floppy' mountmedia device as an origin.

However, it seems that mountmedia already handles these?

https://sources.debian.org/src/mountmedia/0.26/mountmedia/?hl=20#L69
https://sources.debian.org/src/mountmedia/0.26/mountmedia/?hl=20#L20

Thank you!
James

[1] - https://sources.debian.org/src/mountmedia/0.26/mountmedia/?hl=20#L82

[2] - https://sources.debian.org/src/hw-detect/1.159/check-missing-firmware.sh/#L210