From 1c8401be25e4749a2eee5ddc77ce7c6ac880c910 Mon Sep 17 00:00:00 2001
From: Leah Rowe <leah@libreboot.org>
Date: Thu, 24 Aug 2023 20:19:41 +0100
Subject: much, much stricter, more verbose error handling

lbmk is much more likely to crash now, in error conditions,
which is a boon for further auditing.

also: in "fetch", remove the downloaded program
if fail() was called.

this would also be done for gnulib, when downloading
grub, but done in such a way that gnulib goes first.

where calls to err write "ERROR" in the string, they
no longer say "ERROR" because the "err" function itself
now does that automatically.

also: listmodes/listoptions (in "lbmk") now reports an
error if no scripts and/or directories are found.

also: where a warning is given, but not an error, i've
gone through in some places and redirected the output
to stderr, not stdout

as part of error checks: running anything as root, except
for the "./build dependencies *" commands, is no longer
permitted and lbmk will throw an error

mrc downloads: debugfs output no longer redirected to /dev/null,
and stderr no longer redirected to stdout. everything is verbose.

certain non-error states are also more verbose. for example,
patch_rom in blobs/inject will now state when injection succeeds

certain actual errors(bugs) were fixed:
for example, build/release/roms now correctly prepares the blobs
hash files for a given target, containing only the files and
checksums in the list. Previously, a printf message was included.
Now, with this new code: blobutil/inject rightly verifies hashes.

doing all of this in one giant patch is cleaner
than 100 patches changing each file. even this is yet part
of a much larger audit going on in the Libreboot project.

Signed-off-by: Leah Rowe <leah@libreboot.org>
---
 resources/scripts/update/blobs/download | 205 ++++++++++++++++++--------------
 resources/scripts/update/blobs/extract  |  46 +++----
 resources/scripts/update/blobs/inject   |  98 +++++++++------
 resources/scripts/update/blobs/mrc      |  49 +++++---
 4 files changed, 232 insertions(+), 166 deletions(-)

(limited to 'resources/scripts/update')

diff --git a/resources/scripts/update/blobs/download b/resources/scripts/update/blobs/download
index c1babd13..422f0b3c 100755
--- a/resources/scripts/update/blobs/download
+++ b/resources/scripts/update/blobs/download
@@ -52,12 +52,15 @@ CONFIG_SMSC_SCH5545_EC_FW_FILE=""
 
 main()
 {
+	[ $# -gt 0 ] || \
+		err "No argument given"
+
 	board="${1}"
 	boarddir="${cbcfgsdir}/${board}"
 
-	[ ! -d "${boarddir}" ] && \
+	[ -d "${boarddir}" ] || \
 		err "Board target, ${board}, not defined"
-	[ ! -f "${boarddir}/target.cfg" ] && \
+	[ -f "${boarddir}/target.cfg" ] || \
 		err "Target missing target.cfg"
 
 	no_config="printf \"No config for target, %s\\n\" ${board} 1>&2; exit 0"
@@ -65,7 +68,7 @@ main()
 		[ -f "${x}" ] && no_config=""
 	done
 	eval "${no_config}"
-	
+
 	detect_firmware || exit 0
 	scan_sources_config
 
@@ -89,7 +92,7 @@ detect_firmware()
 	[ "${CONFIG_INCLUDE_SMSC_SCH5545_EC_FW}" = "y" ] && \
 	    needs="${needs} SCH5545EC"
 	[ -z ${needs+x} ] && \
-		printf 'No binary blobs needed for this board\n' && \
+		printf "No binary blobs needed for this board\n" && \
 		return 1
 	printf "Firmware needed for board '%s':\n%s\n" ${board} ${needs}
 }
@@ -155,16 +158,19 @@ build_dependencies()
 {
 	[ -d ${cbdir} ] || \
 		./fetch_trees coreboot ${cbdir##*/} || \
-		    err "can't download to ${cbdir}"
+		    err "build_dependencies: can't fetch ${cbdir}"
 	for d in uefitool biosutilities bios_extract me_cleaner; do
 		[ -d "${d}" ] && continue
-		./fetch ${d} || err "can't download ${d}"
+		./fetch "${d}" || \
+		    err "build_dependencies: can't fetch ${d}"
 	done
 	[ -f uefitool/uefiextract ] || \
-		./build src for -b uefitool || err "can't build uefitool"
-	[ -f ${cbdir}/util/kbc1126/kbc1126_ec_dump ] || \
+		./build src for -b uefitool || \
+		    err "build_dependencies: can't build uefitool"
+	if [ ! -f ${cbdir}/util/kbc1126/kbc1126_ec_dump ]; then
 		make -BC ${cbdir}/util/kbc1126 || \
-		    err "can't build kbc1126_ec_dump"
+		    err "build_dependencies: can't build kbc1126_ec_dump"
+	fi
 }
 
 download_blobs()
@@ -184,8 +190,9 @@ download_blobs()
 		esac
 	done
 	
-	[ -z ${_failed+x} ] || \
-		err "failed to obtain ${_failed}\nTry manual extraction?"
+	if [ ! -z ${_failed+x} ]; then
+		err "download_blobs: can't download blobs: ${_failed}\n"
+	fi
 }
 
 download_blob_intel_me()
@@ -203,11 +210,13 @@ extract_blob_intel_me()
 	_me_destination=${CONFIG_ME_BIN_PATH#../../}
 
 	[ -d "${_me_destination%/*}" ] || \
-		mkdir -p ${_me_destination%/*}
-	[ -d "${appdir}" ] && \
-		rm -Rf ${appdir}
+		mkdir -p ${_me_destination%/*} || \
+		    err "extract_blob_intel_me: mkdir ${_me_destination%/*}"
+	[ ! -d "${appdir}" ] || \
+		rm -Rf "${appdir}" || \
+		    err "extract_blob_intel_me: can't rm -Rf \"${appdir}\""
 	if [ -f "${_me_destination}" ]; then
-		printf 'me already downloaded\n'
+		printf "Intel ME firmware already downloaded\n" 1>&2
 		return 0
 	fi
 
@@ -216,12 +225,16 @@ extract_blob_intel_me()
 	innoextract ${dl_path} -d ${appdir} || \
 	    7z x ${dl_path} -o${appdir} || \
 	    unar "${dl_path}" -o "${appdir}" || \
-	    err "Could not extract vendor update"
+	    err "extract_blob_intel_me: could not extract vendor update"
 
 	bruteforce_extract_blob_intel_me "$(pwd)/${_me_destination}" \
-	    "$(pwd)/${appdir}" || err "Could not extract Intel ME firmware"
+	    "$(pwd)/${appdir}" || \
+	    err "extract_blob_intel_me: could not extract Intel ME firmware"
 
-	printf "Truncated and cleaned me output to ${_me_destination}\n"
+	[ -f "${_me_destination}" ] || \
+		err "extract_blob_intel_me, ${board}: me.bin missing"
+
+	printf "Truncated and cleaned me output to: %s\n" "${_me_destination}"
 }
 
 # cursed, carcinogenic code. TODO rewrite it better
@@ -237,7 +250,8 @@ bruteforce_extract_blob_intel_me()
 
 	(
 	printf "Entering %s\n" "${cdir}"
-	cd "${cdir}" || err "cannot enter directory, ${cdir}"
+	cd "${cdir}" || \
+	    err "bruteforce_extract_blob_intel_me: can't cd \"${cdir}\""
 	for i in *; do
 		if [ -f "${_me_destination}" ]; then
 			# me.bin found, so avoid needless further traversal
@@ -247,37 +261,33 @@ bruteforce_extract_blob_intel_me()
 			continue
 		elif [ -f "${i}" ]; then
 			"${mecleaner}" -r -t -O "${sdir}/vendorfile" \
-					-M "${_me_destination}" "${i}" \
-				&& break # (we found me.bin)	
+			    -M "${_me_destination}" "${i}" \
+			    && break # (we found me.bin)	
 			"${mecleaner}" -r -t -O "${_me_destination}" "${i}" \
-				&& break # (we found me.bin)
+			    && break # (we found me.bin)
 			"${me7updateparser}" -O ${_me_destination} "${i}" \
-				&& break # (we found me.bin)
+			    && break # (we found me.bin)
 			_7ztest="${_7ztest}a"
 			7z x "${i}" -o${_7ztest} \
 			    || innoextract "${i}" -d  "${_7ztest}" \
 			    || unar "${i}" -o "${_7ztest}" \
 			    || continue
 			bruteforce_extract_blob_intel_me "${_me_destination}" \
-					"${cdir}/${_7ztest}"
+			    "${cdir}/${_7ztest}"
 		elif [ -d "$i" ]; then
 			bruteforce_extract_blob_intel_me "${_me_destination}" \
-					"${cdir}/${i}"
+			    "${cdir}/${i}"
 		else
 			printf "SKIPPING: %s\n" "${i}"
 			continue
 		fi
 		cdir="${1}"
-		cd "${cdir}"
+		cd "${cdir}" # audit note: we already checked this (see above)
 	done
 	)
 
-	rm -Rf "${sdir}"
-
-	if [ ! -f "${_me_destination}" ]; then
-		printf "me.bin not found in vendor update for: %s\n" ${board}
-		return 1
-	fi
+	rm -Rf "${sdir}" || \
+	    err "bruteforce_extract_blob_intel_me: can't rm -Rf \"${sdir}\""
 }
 
 download_ec()
@@ -295,39 +305,48 @@ extract_blob_kbc1126_ec()
 	_ec_destination=${CONFIG_KBC1126_FW1#../../}
 
 	[ -d "${_ec_destination%/*}" ] || \
-		mkdir -p "${_ec_destination%/*}"
-	[ -d "${appdir}" ] && \
-		rm -Rf "${appdir}"
+		mkdir -p "${_ec_destination%/*}" || \
+		    err "extract_blob_kbc1126_ec: !mkdir ${_ec_destination%/*}"
+	[ ! -d "${appdir}" ] || \
+		rm -Rf "${appdir}" || \
+		    err "extract_blob_kbc1126_ec: !rm -Rf ${appdir}"
 	if [ -f "${_ec_destination}" ]; then
-		printf "ec already downloaded\n"
+		printf "KBC1126 EC firmware already downloaded\n" 1>&2
 		return 0
 	fi
 
-	unar "${dl_path}" -o "${appdir}"
+	unar "${dl_path}" -o "${appdir}" || \
+	    err "extract_blob_kbc1126_ec: !unar \"${dl_path}\" -o \"${appdir}\""
 
 	(
-	cd "${appdir}/${dl_path##*/}"
+	cd "${appdir}/${dl_path##*/}" || \
+	    err "extract_blob_kbc1126_ec: !cd \"${appdir}/${dl_path##*/}\""
 
-	mv Rompaq/68*.BIN ec.bin
+	mv Rompaq/68*.BIN ec.bin || :
 	if [ ! -f ec.bin ]; then
 		unar -D ROM.CAB Rom.bin || \
 		    unar -D Rom.CAB Rom.bin || \
-		    unar -D 68*.CAB Rom.bin
-		mv Rom.bin ec.bin
+		    unar -D 68*.CAB Rom.bin || \
+		    err "extract_blob_kbc1126_ec: can't extract ec.bin"
+		mv Rom.bin ec.bin || \
+		    err "extract_blob_kbc1126_ec: *didn't* extract ec.bin"
 	fi
+	[ -f ec.bin ] || \
+	    err "extract_blob_kbc1126_ec: ${board}: can't extract ec.bin"
 
-	[ -f ec.bin ] || err "could not extract ec.bin for board, ${board}"
-	"${kbc1126_ec_dump}" ec.bin
+	"${kbc1126_ec_dump}" ec.bin || \
+	    err "extract_blob_kbc1126_ec: ${board}: can't extract ecfw1/2.bin"
 	)
 
+	ec_ex="y"
 	for i in 1 2; do
-		[ -f "${appdir}/${dl_path##*/}/ec.bin.fw${i}" ] && continue
-		printf "Could not extract EC firmware for: %s\n" \
-		    ${board}
-		return 1
+		[ -f "${appdir}/${dl_path##*/}/ec.bin.fw${i}" ] || ec_ex="n"
 	done
+	[ "${ec_ex}" = "y" ] || \
+	    err "extract_blob_kbc1126_ec: ${board}: didn't extract ecfw1/2.bin"
 
-	cp "${appdir}/${dl_path##*/}"/ec.bin.fw* "${_ec_destination%/*}/"
+	cp "${appdir}/${dl_path##*/}"/ec.bin.fw* "${_ec_destination%/*}/" || \
+	    err "extract_blob_kbc1126_ec: cant mv ecfw1/2 ${_ec_destination%/*}"
 }
 
 download_e6400vga()
@@ -345,37 +364,41 @@ extract_e6400vga()
 	_vga_destination=${CONFIG_VGA_BIOS_FILE#../../}
 
 	if [ -f "${_vga_destination}" ]; then
-		printf 'vga rom already downloaded\n'
+		printf "extract_e6400vga: vga rom already downloaded\n" 1>&2
 		return 0
 	fi
 	[ -d "${_vga_destination%/*}" ] || \
-		mkdir -p ${_vga_destination%/*}
-	[ -d "${appdir}" ] && \
-		rm -Rf ${appdir}
-
-	mkdir -p "${appdir}"
-	cp "${dl_path}" "${appdir}"
-
-	if [ "${e6400_vga_offset}" = "" ]; then
-		printf "E6400 VGA offset not defined\n"
-		return 1
-	elif [ "${e6400_vga_romname}" = "" ]; then
-		printf "E6400 VGA ROM name not defined\n"
-		return 1
-	fi
+		mkdir -p ${_vga_destination%/*} || \
+		    err "extract_e6400vga: can't mkdir ${_vga_destination%/*}"
+	[ ! -d "${appdir}" ] || \
+		rm -Rf ${appdir} || \
+		    err "extract_e6400vga: can't rm -Rf ${appdir}"
+
+	mkdir -p "${appdir}" || \
+	    err "extract_e6400vga: can't mkdir ${appdir}"
+	cp "${dl_path}" "${appdir}" || \
+	    err "extract_e6400vga: can't copy vendor update"
+
+	[ "${e6400_vga_offset}" = "" ] && \
+		err "extract_e6400vga: E6400 VGA offset not defined"
+	[ "${e6400_vga_romname}" = "" ] && \
+		err "extract_e6400vga: E6400 VGA ROM name not defined"
 
 	(
-	cd "${appdir}"
-	tail -c +${e6400_vga_offset} "${dl_path##*/}" \
-			| gunzip > bios.bin
+	cd "${appdir}" || \
+	    err "extract_e6400vga: can't cd ${appdir}"
+	tail -c +${e6400_vga_offset} "${dl_path##*/}" | gunzip > bios.bin || \
+	    err "extract_e6400vga: can't gunzip > bios.bin"
+
 	[ -f "bios.bin" ] || \
-		err "Could not extract bios.bin from Dell E6400 update"
+		err "extract_e6400vga: can't extract bios.bin from update"
 	"${e6400_unpack}" bios.bin || printf "TODO: fix dell extract util\n"
 	[ -f "${e6400_vga_romname}" ] || \
-		err "Could not extract VGA ROM from Dell E6400 BIOS update"
+		err "extract_e6400vga: can't extract vga rom from bios.bin"
 	)
 
-	cp "${appdir}"/"${e6400_vga_romname}" "${_vga_destination}"
+	cp "${appdir}"/"${e6400_vga_romname}" "${_vga_destination}" || \
+	    err "extract_e6400vga: can't copy vga rom to ${_vga_destination}"
 
 	printf "E6400 Nvidia ROM saved to: %s\n" "${_vga_destination}"
 }
@@ -398,16 +421,18 @@ extract_sch5545ec()
 	_sch5545ec_destination=${CONFIG_SMSC_SCH5545_EC_FW_FILE#../../}
 
 	if [ -f "${_sch5545ec_destination}" ]; then
-		printf 'sch5545 firmware already downloaded\n'
+		printf "sch5545 firmware already downloaded\n" 1>&2
 		return 0
 	fi
 
-	[ -d "${appdir}" ] rm -Rf "${appdir}"
+	[ ! -d "${appdir}" ] || rm -Rf "${appdir}" || \
+	    err "extract_sch5545ec: can't remove ${appdir}"
 
-	mkdir -p "${appdir}/"
-	cp "${dl_path}" "${appdir}/"
+	mkdir -p "${appdir}/" || err "extract_sch5545ec: !mkdir ${appdir}"
+	cp "${dl_path}" "${appdir}/" || \
+	    err "extract_sch5545ec: can't copy vendor update file"
 	python "${pfs_extract}" "${appdir}/${dlsum}" -e || \
-	    err "cannot extract archive (dell, sch5545)"
+	    err "extract_sch5545ec: can't extract from vendor update"
 
 	# full system ROM (UEFI), to extract with UEFIExtract:
 	_bios="${appdir}/${dlsum}_extracted/Firmware"
@@ -419,15 +444,16 @@ extract_sch5545ec()
 	_sch5545ec_fw="${_sch5545ec_fw}/0 Raw section/body.bin" # <-- this!
 
 	# this makes the file defined by _sch5545ec_fw available to copy
-	"${uefiextract}" "${_bios}" || err "cannot extract dell uefi image"
+	"${uefiextract}" "${_bios}" || \
+	    err "extract_sch5545ec: cannot extract from uefi image"
 
 	cp "${_sch5545ec_fw}" "${_sch5545ec_destination}" || \
-	    err "cannot copy sch5545ec firmware file"
+	    err "extract_sch5545ec: cannot copy sch5545ec firmware file"
 }
 
 fetch_update()
 {
-	printf "Fetching vendor update for board: %s\n" ${board}
+	printf "Fetching vendor update for board: %s\n" "${board}"
 
 	fw_type="${1}"
 	dl=""
@@ -450,39 +476,38 @@ fetch_update()
 		dl_bkup="${sch5545ec_dl_url_bkup}"
 		dlsum="${sch5545ec_dl_hash}"
 	else
-		printf "Unsupported download type: %s\n" ${fw_type}
-		return 1
+		err "fetch_update: Unsupported download type: ${fw_type}"
 	fi
 
-	if [ -z "${dl_url+x}" ] && [ "${fw_type}" != "e6400vga" ]; then
-		printf "No vendor update specified for board: %s\n" ${board}
-		return 1
-	fi
+	[ -z "${dl_url+x}" ] && [ "${fw_type}" != "e6400vga" ] && \
+		err "fetch_update ${fw_type}: dl_url unspecified for: ${board}"
 
 	dl_path=${blobdir}/cache/${dlsum}
-	mkdir -p ${blobdir}/cache
+	mkdir -p ${blobdir}/cache || err "fetch_update: !mkdir ${blobdir}/cache"
 
 	dl_fail="y"
 	vendor_checksum ${dlsum} && dl_fail="n"
 	for x in "${dl}" "${dl_bkup}"; do
 		[ "${dl_fail}" = "n" ] && break
-		rm -f "${dl_path}"
-		wget -U "${agent}" ${x} -O ${dl_path}
+		[ -z "${x}" ] && continue
+		rm -f "${dl_path}" || \
+		    err "fetch_update ${fw_type}: !rm -f ${dl_path}"
+		wget -U "${agent}" ${x} -O ${dl_path} || continue
 		vendor_checksum ${dlsum} && dl_fail="n"
 	done
 	if [ "${dl_fail}" = "y" ]; then
-		printf "Could not download blob file\n" 1>&2
-		return 1
+		printf "ERROR: invalid vendor updates for: %s\n" "${board}" 1>&2
+		err "fetch_update ${fw_type}: matched vendor update unavailable"
 	fi
 }
 
 vendor_checksum()
 {
 	if [ ! -f "${dl_path}" ]; then
-		printf "Vendor update not found on disk for: %s\n" ${board}
+		printf "Vendor update not found on disk for: %s\n" ${board} 1>&2
 		return 1
 	elif [ "$(sha1sum ${dl_path} | awk '{print $1}')" != "${1}" ]; then
-		printf "Bad checksum on vendor update for: %s\n" ${board}
+		printf "Bad checksum on vendor update for: %s\n" ${board} 1>&2
 		return 1
 	fi
 }
diff --git a/resources/scripts/update/blobs/extract b/resources/scripts/update/blobs/extract
index d7a68bf3..b6b3af3b 100755
--- a/resources/scripts/update/blobs/extract
+++ b/resources/scripts/update/blobs/extract
@@ -44,22 +44,27 @@ main()
 
 check_board()
 {
-	[ -f "${vendor_rom}" ] || \
-		err "file does not exist: ${vendor_rom}"
-	[ -d "${boarddir}" ] || \
-		err "build/roms ${board}: target not defined"
-	[ -f "${boarddir}/target.cfg" ] || \
-		err "build/roms ${board}: missing target.cfg"
+	if [ ! -f "${vendor_rom}" ]; then
+		err "check_board: ${board}: file does not exist: ${vendor_rom}"
+	elif [ ! -d "${boarddir}" ]; then
+		err "check_board: ${board}: target not defined"
+	elif [ ! -f "${boarddir}/target.cfg" ]; then
+		err "check_board: ${board}: missing target.cfg"
+	fi
 }
 
 build_dependencies()
 {
-	[ -d me_cleaner ] || \
-		./fetch me_cleaner || err "can't fetch me_cleaner"
-	[ -d ${cbdir} ] || \
-		./fetch_trees coreboot default || err "can't fetch coreboot"
-	[ -f ${ifdtool} ] || \
-		make -C "${ifdtool%/ifdtool}" || err "can't build ifdtool"
+	if [ ! -d me_cleaner ]; then
+		./fetch me_cleaner || \
+		    err "build_dependencies: can't fetch me_cleaner"
+	elif [ ! -d "${cbdir}" ]; then
+		./fetch_trees coreboot default || \
+		    err "build_dependencies: can't fetch coreboot"
+	elif [ ! -f "${ifdtool}" ]; then
+		make -C "${ifdtool%/ifdtool}" || \
+		    err "build_dependencies: can't build ifdtool"
+	fi
 }
 
 extract_blobs()
@@ -67,11 +72,11 @@ extract_blobs()
 	printf "extracting blobs for %s from %s\n" ${board} ${vendor_rom}
 
 	set -- "${boarddir}/config/"*
-	. ${1} 2>/dev/null
+	. "${1}"
 	. "${boarddir}/target.cfg"
 
 	[ "$CONFIG_HAVE_MRC" != "y" ] || \
-		./update blobs mrc || err "could not download mrc"
+		./update blobs mrc || err "extract_blobs: can't fetch mrc"
 
 	_me_destination=${CONFIG_ME_BIN_PATH#../../}
 	_gbe_destination=${CONFIG_GBE_BIN_PATH#../../}
@@ -81,11 +86,11 @@ extract_blobs()
 	extract_blob_intel_gbe_nvm
 
 	# Cleans up other files extracted with ifdtool
-	rm -f flashregion*.bin 2> /dev/null
+	rm -f flashregion*.bin || err "extract_blobs: !rm -f flashregion*.bin"
 
-	[ -f ${_ifd_destination} ] || err "Could not extract IFD"
+	[ -f ${_ifd_destination} ] || err "extract_blobs: Could not extract IFD"
 	printf "gbe, ifd, and me extracted to %s\n" \
-	    ${_me_destination%/*}
+	    "${_me_destination%/*}"
 }
 
 extract_blob_intel_me()
@@ -96,15 +101,16 @@ extract_blob_intel_me()
 		-M ${_me_destination} ${vendor_rom} -t -r -S || \
 	    ${me7updateparser} \
 		-O ${_me_destination} ${vendor_rom} || \
-	    err "me_cleaner failed to extract blobs from rom"
+	    err "extract_blob_intel_me: cannot extract from vendor rom"
 }
 
 extract_blob_intel_gbe_nvm()
 {
 	printf "extracting gigabit ethernet firmware"
-	./${ifdtool} -x ${vendor_rom}
+	./${ifdtool} -x ${vendor_rom} || \
+	    err "extract_blob_intel_gbe_nvm: cannot extract gbe.bin from rom"
 	mv flashregion*gbe.bin ${_gbe_destination} || \
-	    err 'could not extract gbe'
+	    err "extract_blob_intel_gbe_nvm: cannot move gbe.bin"
 }
 
 print_help()
diff --git a/resources/scripts/update/blobs/inject b/resources/scripts/update/blobs/inject
index a6f6c007..4ab0c366 100755
--- a/resources/scripts/update/blobs/inject
+++ b/resources/scripts/update/blobs/inject
@@ -67,27 +67,28 @@ main()
 
 	printf "Friendly reminder (this is *not* an error message):\n"
 	printf "Please always ensure that the files were inserted correctly.\n"
-	printf "Read https://libreboot.org/docs/install/ivy_has_common.html\n"
+	printf "Read: https://libreboot.org/docs/install/ivy_has_common.html\n"
 }
 
 check_board()
 {
 	if ! check_release ${archive} ; then
 		[ -f "${rom}" ] || \
-			err "\"${rom}\" is not a valid path"
+			err "check_board: \"${rom}\" is not a valid path"
 		[ -z ${rom+x} ] && \
-			err "no rom specified"
+			err "check_board: no rom specified"
 		[ ! -z ${board+x} ] || \
-			board=$(detect_board ${rom}) || \
-			    err "no board specified"
+			board=$(detect_board "${rom}")
 	else
 		release=true
 		releasearchive="${archive}"
-		board=$(detect_board ${archive})
+		board=$(detect_board "${archive}")
 	fi
 
 	boarddir="${cbcfgsdir}/${board}"
-	[ -d "${boarddir}" ] || err "board ${board} not found"
+	if [ ! -d "${boarddir}" ]; then
+		err "check_board: board ${board} not found"
+	fi
 }
 
 check_release()
@@ -122,18 +123,20 @@ detect_board()
 build_dependencies()
 {
 	[ -d "${cbdir}" ] || ./fetch_trees coreboot default
-	./build coreboot utils default || err "could not build cbutils"
+	./build coreboot utils default || \
+	    err "build_dependencies: could not build cbutils"
 	./update blobs download ${board} || \
-	    err "Could not download blobs for ${board}"
+	    err "build_dependencies: Could not download blobs for ${board}"
 }
 
 inject_blobs()
 {
 	if [ "${release}" = "true" ]; then
-		echo 'patching release file'
+		printf "patching release file\n"
 		patch_release_roms
 	else
-		patch_rom ${rom}
+		patch_rom "${rom}" || \
+		    err "inject_blobs: could not patch ${x}"
 	fi
 }
 
@@ -141,17 +144,17 @@ patch_release_roms()
 {
 	_tmpdir=$(mktemp -d "/tmp/${board}_tmpXXXX")
 	tar xf "${releasearchive}" -C "${_tmpdir}" || \
-	err 'could not extract release archive'
+	    err "patch_release_roms: could not extract release archive"
 
 	for x in ${_tmpdir}/bin/*/*.rom ; do
 		echo "patching rom $x"
-		patch_rom ${x} || err "could not patch ${x}"
+		patch_rom ${x} || err "patch_release_roms: could not patch ${x}"
 	done
 
 	(
 	cd ${_tmpdir}/bin/*
 	sha1sum --status -c blobhashes || \
-	err 'ROMs did not match expected hashes'
+	    err "patch_release_roms: ROMs did not match expected hashes"
 	)
 
 	if [ "${modifygbe}" = "true" ]; then
@@ -160,19 +163,31 @@ patch_release_roms()
 		done
 	fi
 
-	[ -d bin/release ] || mkdir -p bin/release
-	mv ${_tmpdir}/bin/* bin/release/ && \
-	    printf '%s\n' 'Success! Your ROMs are in bin/release'
+	[ -d bin/release ] || mkdir -p bin/release || \
+	    err "patch_release_roms: !mkdir -p bin/release"
+	mv "${_tmpdir}"/bin/* bin/release/ || \
+	    err "patch_release_roms: !mv ${_tmpdir}/bin/* bin/release/"
+
+	printf "Success! Your ROMs are in bin/release\n"
 
-	rm -r "${_tmpdir}"
+	rm -Rf "${_tmpdir}" || err "patch_release_roms: !rm -Rf ${_tmpdir}"
 }
 
 patch_rom()
 {
 	rom="${1}"
 
+	no_config="printf \"No configs on target, %s\\n\" ${board} 1>&2; exit 1"
+	for x in "${boarddir}"/config/*; do
+		[ -f "${x}" ] && no_config=""
+	done
+	eval "${no_config}"
+
+	[ -f "${boarddir}/target.cfg" ] || \
+	    err "patch_rom: file missing: ${boarddir}/target.cfg"
+
 	set -- "${boarddir}/config/"*
-	. ${1} 2>/dev/null
+	. "${1}"
 	. "${boarddir}/target.cfg"
 
 	[ "$CONFIG_HAVE_MRC" = "y" ] && \
@@ -189,6 +204,8 @@ patch_rom()
 		inject_blob_smsc_sch5545_ec "${rom}"
 	[ "${modifygbe}" = "true" ] && ! [ "${release}" = "true" ] && \
 		modify_gbe ${rom}
+
+	printf "ROM image successfully patched: %s\n" "${rom}"
 }
 
 inject_blob_intel_mrc()
@@ -216,7 +233,7 @@ inject_blob_intel_mrc()
 	# TODO: this logic should be tweaked to handle more platforms
 
 	${cbfstool} ${rom} add -f mrc/haswell/mrc.bin -n mrc.bin -t mrc \
-		-b 0xfffa0000 || err "cannot insert mrc.bin"
+	    -b 0xfffa0000 || err "inject_blob_intel_mrc: cannot insert mrc.bin"
 }
 
 inject_blob_intel_me()
@@ -225,14 +242,14 @@ inject_blob_intel_me()
 
 	rom="${1}"
 	[ -z ${CONFIG_ME_BIN_PATH} ] && \
-		err "CONFIG_ME_BIN_PATH not set"
+		err "inject_blob_intel_me: CONFIG_ME_BIN_PATH not set"
 
 	_me_location=${CONFIG_ME_BIN_PATH#../../}
 	[ ! -f "${_me_location}" ] && \
-		err "CONFIG_ME_BIN_PATH points to missing file"
+		err "inject_blob_intel_me: per CONFIG_ME_BIN_PATH: file missing"
 
 	${ifdtool} -i me:${_me_location} ${rom} -O ${rom} || \
-	    err "cannot insert me.bin"
+	    err "inject_blob_intel_me: cannot insert me.bin"
 }
 
 inject_blob_hp_kbc1126_ec()
@@ -247,19 +264,21 @@ inject_blob_hp_kbc1126_ec()
 	printf "adding hp kbc1126 ec firmware\n"
 
 	if [ "${_ec1_offset}" = "" ] || [ "${_ec1_offset}" = "" ]; then
-		err "EC offsets not declared for board, ${board}"
+		err "inject_blob_hp_kbc1126_ec: ${board}: offset not declared"
 	fi
 	if [ "${_ec1_location}" = "" ] || [ "${_ec2_location}" = "" ]; then
-		err "EC firmware path not declared for board, ${board}"
+		err "inject_blob_hp_kbc1126_ec: ${board}: EC path not declared"
 	fi
 	if [ ! -f "${_ec1_location}" ] || [ ! -f "${_ec2_location}" ]; then
-		err "EC firmware not downloaded for board: ${board}"
+		err "inject_blob_hp_kbc1126_ec: ${board}: ecfw not downloaded"
 	fi
 
 	${cbfstool} "${rom}" add -f ${_ec1_location} -n ecfw1.bin \
-	    -b ${_ec1_offset} -t raw || err "cannot insert ecfw1.bin"
+	    -b ${_ec1_offset} -t raw || \
+	    err "inject_blob_hp_kbc1126_ec: cannot insert ecfw1.bin"
 	${cbfstool} "${rom}" add -f ${_ec2_location} -n ecfw2.bin \
-	    -b ${_ec2_offset} -t raw || err "cannot insert ecfw2.bin"
+	    -b ${_ec2_offset} -t raw || \
+	    err "inject_blob_hp_kbc1126_ec: cannot insert ecfw2.bin"
 }
 
 inject_blob_dell_e6400_vgarom_nvidia()
@@ -273,15 +292,15 @@ inject_blob_dell_e6400_vgarom_nvidia()
 	printf "adding pci option rom\n"
 
 	if [ "${_vga_dir}" != "${pciromsdir}" ]; then
-		err "Invalid PCI ROM directory, ${_vga_dir}"
+		err "inject_blob_dell_e6400vga: invalid pcirom dir: ${_vga_dir}"
 	fi
 	if [ ! -f "${_vga_location}" ]; then
-		err "No such file exists, ${_vga_location}"
+		err "inject_blob_dell_e6400vga: ${_vga_location} doesn't exist"
 	fi
 
 	${cbfstool} ${rom} add -f "${_vga_location}" \
 	    -n "pci${CONFIG_VGA_BIOS_ID}.rom" -t optionrom || \
-	    err "cannot insert e6400 nvidia rom"
+	    err "inject_blob_dell_e6400vga: cannot insert vga oprom"
 }
 
 inject_blob_smsc_sch5545_ec()
@@ -291,11 +310,12 @@ inject_blob_smsc_sch5545_ec()
 	_sch5545ec_location="${CONFIG_SMSC_SCH5545_EC_FW_FILE#../../}"
 
 	if [ ! -f "${_sch5545ec_location}" ]; then
-		err "SCH5545 firmware file missing"
+		err "inject_blob_smsc_sch5545_ec: SCH5545 fw missing"
 	fi
 
 	"${cbfstool}" "${rom}" add -f "${_sch5545ec_location}" \
-	    -n sch5545_ecfw.bin -t raw || err "cannot insert sch5545_ecfw.bin"
+	    -n sch5545_ecfw.bin -t raw || \
+	    err "inject_blob_smsc_sch5545_ec: can't insert sch5545_ecfw.bin"
 }
 
 modify_gbe()
@@ -305,22 +325,22 @@ modify_gbe()
 	rom=${1}
 
 	[ -z ${CONFIG_GBE_BIN_PATH} ] && \
-		err "CONFIG_GBE_BIN_PATH not set"
+		err "modify_gbe: ${board}: CONFIG_GBE_BIN_PATH not set"
 
 	_gbe_location=${CONFIG_GBE_BIN_PATH#../../}
 
 	[ -f "${_gbe_location}" ] || \
-		err "CONFIG_GBE_BIN_PATH points to missing file"
+		err "modify_gbe: CONFIG_GBE_BIN_PATH points to missing file"
 	[ -f ${nvmutil} ] || \
-		make -C util/nvmutil || err 'failed to build nvmutil'
+		make -C util/nvmutil || err "modify_gbe: couldn't build nvmutil"
 
 	_gbe_tmp=$(mktemp -t gbeXXXX.bin)
 	cp ${_gbe_location} ${_gbe_tmp}
-	${nvmutil} "${_gbe_tmp}" setmac ${new_mac} || \
-	    err 'failed to modify mac address'
+	${nvmutil} "${_gbe_tmp}" setmac "${new_mac}" || \
+	    err "modify_gbe: ${board}: failed to modify mac address"
 
 	${ifdtool} -i GbE:${_gbe_tmp} "${rom}" -O "${rom}" || \
-	    err "cannot insert modified gbe.bin"
+	    err "modify_gbe: ${board}: cannot insert modified gbe.bin"
 
 	rm -f ${_gbe_tmp}
 }
diff --git a/resources/scripts/update/blobs/mrc b/resources/scripts/update/blobs/mrc
index 57cbede6..74899990 100755
--- a/resources/scripts/update/blobs/mrc
+++ b/resources/scripts/update/blobs/mrc
@@ -72,23 +72,23 @@ check_existing()
 build_dependencies()
 {
 	[ -d "${cbdir}/" ] || ./fetch_trees coreboot default || \
-	    err "cannot fetch coreboot/default"
+	    err "build_dependencies: cannot fetch coreboot/default"
 	./build coreboot utils default || \
-	    err "cannot build cbutils/default"
+	    err "build_dependencies: cannot build cbutils/default"
 }
 
 fetch_mrc()
 {
-	mkdir -p mrc/haswell/ || err "cannot mkdir mrc/haswell"
+	mkdir -p mrc/haswell/ || err "fetch_mrc: !mkdir mrc/haswell"
 
 	(
-	cd mrc/haswell/
+	cd mrc/haswell/ || err "fetch_mrc: !cd mrc/haswell"
 
 	download_image ${_url} ${_file} ${_sha1sum}
 	[ -f ${_file} ] || \
 		download_image ${_url2} ${_file} ${_sha1sum}
 	[ -f $_file ] || \
-		err "%{_file} not downloaded / verification failed."
+		err "fetch_mrc: ${_file} not downloaded / verification failed."
 
 	extract_partition ROOT-A ${_file} root-a.ext2
 	extract_shellball root-a.ext2 chromeos-firmwareupdate-${_board}
@@ -96,9 +96,9 @@ fetch_mrc()
 	extract_coreboot chromeos-firmwareupdate-${_board}
 
 	../../${cbfstool} coreboot-*.bin extract -f mrc.bin -n mrc.bin \
-	    -r RO_SECTION || err "Could not fetch mrc.bin"
+	    -r RO_SECTION || err "fetch_mrc: could not fetch mrc.bin"
 	rm -f "chromeos-firmwareupdate-${_board}" coreboot-*.bin \
-	    "${_file}" "root-a.ext2"
+	    "${_file}" "root-a.ext2" || err "fetch_mrc: cannot remove files"
 
 	printf "\n\nmrc.bin saved to ${_mrc_complete}\n\n"
 	)
@@ -111,16 +111,16 @@ download_image()
 	_sha1sum=${3}
 
 	printf "Downloading recovery image\n"
-	curl "$url" > "$_file.zip"
+	curl "$url" > "$_file.zip" || err "download_image: curl failed"
 	printf "Verifying recovery image checksum\n"
 	if [ "$(sha1sum ${_file}.zip | awk '{print $1}')" = "${_sha1sum}" ]
 	then
-		unzip -q "${_file}.zip"
-		rm "${_file}.zip"
+		unzip -q "${_file}.zip" || err "download_image: cannot unzip"
+		rm -f "${_file}.zip" || err "download_image: can't rm zip {1}"
 		return 0
 	fi
-	rm "${_file}.zip"
-	err "Bad checksum. Recovery image deleted"
+	rm -f "${_file}.zip" || err "download_image: bad hash, and can't rm zip"
+	err "download_image: Bad checksum. Recovery image deleted"
 }
 
 extract_partition()
@@ -138,7 +138,8 @@ extract_partition()
 	SIZE=$(( $( echo ${ROOTP} | cut -f4 -d\ | tr -d "B" ) ))
 
 	dd if=${FILE} of=${ROOTFS} bs=${_bs} skip=$(( ${START} / ${_bs} )) \
-	    count=$(( ${SIZE} / ${_bs} )) > /dev/null
+	    count=$(( ${SIZE} / ${_bs} )) || \
+	    err "extract_partition: can't extract root file system"
 }
 
 extract_shellball()
@@ -148,7 +149,7 @@ extract_shellball()
 
 	printf "Extracting chromeos-firmwareupdate\n"
 	printf "cd /usr/sbin\ndump chromeos-firmwareupdate ${SHELLBALL}\nquit" \
-	    | debugfs ${ROOTFS} > /dev/null 2>&1
+	    | debugfs ${ROOTFS} || err "extract_shellball: debugfs"
 }
 
 extract_coreboot()
@@ -157,13 +158,27 @@ extract_coreboot()
 	_unpacked=$( mktemp -d )
 
 	printf "Extracting coreboot image\n"
-	sh ${_shellball} --unpack ${_unpacked} > /dev/null
+
+	[ -f "${_shellball}" ] || \
+	    err "extract_coreboot: shellball missing in google peppy image"
+
+	sh ${_shellball} --unpack ${_unpacked} || \
+	    err "extract_coreboot: shellball exits with non-zero status"
+
+	# TODO: audit the f* out of that shellball, for each mrc version.
+	# it has to be updated for each mrc update. we should ideally
+	# implement the functionality ourselves.
+
+	[ -f "${_unpacked}/VERSION" ] || \
+	    err "extract_coreboot: VERSION file missing on google coreboot rom"
 
 	_version=$( cat ${_unpacked}/VERSION | grep BIOS\ version: | \
 	    cut -f2 -d: | tr -d \  )
 
-	cp ${_unpacked}/bios.bin coreboot-${_version}.bin
-	rm -r "${_unpacked}"
+	cp ${_unpacked}/bios.bin coreboot-${_version}.bin || \
+	    err "extract_coreboot: cannot copy google peppy rom"
+	rm -Rf "${_unpacked}" || \
+	    err "extract_coreboot: cannot remove extracted google peppy archive"
 }
 
 main $@
-- 
cgit v1.2.1