From d7c33ee024a4c044345e9d5b79ad039c8dafd8cd Mon Sep 17 00:00:00 2001 From: Tails developers Date: Fri, 7 Dec 2012 22:31:36 +0100 Subject: [PATCH] Using 'local' in a safe way. First of all, 'local' is non-POSIX, but it is a really good safeguard against hard-to-find bugs. However, doing a local + initializing combo like `local X=$Y` in dash is error prone. If `Y=1 2` will get an error since dash will expand $Y so we get `local X=1 2`, but it will treat the "2" as another variable to be made local, which isn't what we want. Hence, let's declare variables local and initialize them in separate commands, which is safe. --- scripts/boot/9990-misc-helpers.sh | 184 ++++++++++++++++++++++---------------- scripts/boot/9990-mount-http.sh | 7 +- scripts/boot/9990-mount-iscsi.sh | 13 +-- scripts/boot/9990-overlay.sh | 12 ++- 4 files changed, 126 insertions(+), 90 deletions(-) diff --git a/scripts/boot/9990-misc-helpers.sh b/scripts/boot/9990-misc-helpers.sh index 29f943c..356fe6d 100755 --- a/scripts/boot/9990-misc-helpers.sh +++ b/scripts/boot/9990-misc-helpers.sh @@ -65,7 +65,8 @@ get_backing_device () match_files_in_dir () { # Does any files match pattern ${1} ? - local pattern="${1}" + local pattern + pattern="${1}" if [ "$(echo ${pattern})" != "${pattern}" ] then @@ -369,24 +370,27 @@ really_export () is_in_list_separator_helper () { - local sep=${1} + local sep element list + sep=${1} shift - local element=${1} + element=${1} shift - local list=${*} + list=${*} echo ${list} | grep -qe "^\(.*${sep}\)\?${element}\(${sep}.*\)\?$" } is_in_space_sep_list () { - local element=${1} + local element + element=${1} shift is_in_list_separator_helper "[[:space:]]" "${element}" "${*}" } is_in_comma_sep_list () { - local element=${1} + local element + element=${1} shift is_in_list_separator_helper "," "${element}" "${*}" } @@ -503,25 +507,28 @@ trim_path () what_is_mounted_on () { - local dir="$(trim_path ${1})" + local dir + dir="$(trim_path ${1})" grep -m1 "^[^ ]\+ ${dir} " /proc/mounts | cut -d' ' -f1 } chown_ref () { - local reference="${1}" + local reference targets owner + reference="${1}" shift - local targets=${@} - local owner=$(stat -c %u:%g "${reference}") + targets=${@} + owner=$(stat -c %u:%g "${reference}") chown -h ${owner} ${targets} } chmod_ref () { - local reference="${1}" + local reference targets rights + reference="${1}" shift - local targets=${@} - local rights=$(stat -c %a "${reference}") + targets=${@} + rights=$(stat -c %a "${reference}") chmod ${rights} ${targets} } @@ -607,12 +614,13 @@ load_keymap () setup_loop () { - local fspath=${1} - local module=${2} - local pattern=${3} - local offset=${4} - local encryption=${5} - local readonly=${6} + local fspath module pattern offset encryption readonly + fspath=${1} + module=${2} + pattern=${3} + offset=${4} + encryption=${5} + readonly=${6} # the output of setup_loop is evaluated in other functions, # modprobe leaks kernel options like "libata.dma=0" @@ -721,17 +729,18 @@ try_mount () # success, print the mount point for $device. mount_persistence_media () { - local device=${1} - local probe=${2} + local device probe backing old_backing fstype mount_opts + device=${1} + probe=${2} - local backing="${rootmnt}/lib/live/mount/persistence/$(basename ${device})" + backing="${rootmnt}/lib/live/mount/persistence/$(basename ${device})" mkdir -p "${backing}" - local old_backing="$(where_is_mounted ${device})" + old_backing="$(where_is_mounted ${device})" if [ -z "${old_backing}" ] then - local fstype="$(get_fstype ${device})" - local mount_opts="rw,noatime" + fstype="$(get_fstype ${device})" + mount_opts="rw,noatime" if [ -n "${PERSISTENCE_READONLY}" ] then mount_opts="ro,noatime" @@ -766,8 +775,9 @@ mount_persistence_media () close_persistence_media () { - local device=${1} - local backing="$(where_is_mounted ${device})" + local device backing + device=${1} + backing="$(where_is_mounted ${device})" if [ -d "${backing}" ] then @@ -833,22 +843,25 @@ open_luks_device () get_gpt_name () { - local dev="${1}" + local dev + dev="${1}" /sbin/blkid -s PART_ENTRY_NAME -p -o value ${dev} 2>/dev/null } is_gpt_device () { - local dev="${1}" + local dev + dev="${1}" [ "$(/sbin/blkid -s PART_ENTRY_SCHEME -p -o value ${dev} 2>/dev/null)" = "gpt" ] } probe_for_gpt_name () { - local overlays="${1}" - local dev="${2}" + local overlays dev gpt_dev gpt_name + overlays="${1}" + dev="${2}" - local gpt_dev="${dev}" + gpt_dev="${dev}" if is_active_luks_mapping ${dev} then # if $dev is an opened luks device, we need to check @@ -861,7 +874,7 @@ probe_for_gpt_name () return fi - local gpt_name=$(get_gpt_name ${gpt_dev}) + gpt_name=$(get_gpt_name ${gpt_dev}) for label in ${overlays} do if [ "${gpt_name}" = "${label}" ] @@ -873,8 +886,9 @@ probe_for_gpt_name () probe_for_fs_label () { - local overlays="${1}" - local dev="${2}" + local overlays dev + overlays="${1}" + dev="${2}" for label in ${overlays} do @@ -887,11 +901,12 @@ probe_for_fs_label () probe_for_file_name () { - local overlays="${1}" - local dev="${2}" + local overlays dev ret backing + overlays="${1}" + dev="${2}" - local ret="" - local backing="$(mount_persistence_media ${dev} probe)" + ret="" + backing="$(mount_persistence_media ${dev} probe)" if [ -z "${backing}" ] then return @@ -902,7 +917,8 @@ probe_for_file_name () path=${backing}/${PERSISTENCE_PATH}${label} if [ -f "${path}" ] then - local loopdev=$(setup_loop "${path}" "loop" "/sys/block/loop*") + local loopdev + loopdev=$(setup_loop "${path}" "loop" "/sys/block/loop*") ret="${ret} ${label}=${loopdev}" fi done @@ -936,17 +952,19 @@ find_persistence_media () # ${white_list_devices} is non-empty, only devices in it will be # scanned. - local overlays="${1}" - local white_listed_devices="${2}" - local ret="" + local overlays white_listed_devices ret black_listed_devices + overlays="${1}" + white_listed_devices="${2}" + ret="" - local black_listed_devices="$(what_is_mounted_on ${rootmnt}/lib/live/medium)" + black_listed_devices="$(what_is_mounted_on ${rootmnt}/lib/live/medium)" for dev in $(storage_devices "${black_listed_devices}" "${white_listed_devices}") do - local result="" + local result luks_device + result="" - local luks_device="" + luks_device="" # Check if it's a luks device; we'll have to open the device # in order to probe any filesystem it contains, like we do # below. activate_custom_mounts() also depends on that any luks @@ -1122,11 +1140,12 @@ link_files () # is non-empty, remove mask from all source paths when # creating links (will be necessary if we change root, which # live-boot normally does (into $rootmnt)). + local src_dir dest_dir src_mask # remove multiple /:s and ensure ending on / - local src_dir="$(trim_path ${1})/" - local dest_dir="$(trim_path ${2})/" - local src_mask="${3}" + src_dir="$(trim_path ${1})/" + dest_dir="$(trim_path ${2})/" + src_mask="${3}" # This check can only trigger on the inital, non-recursive call since # we create the destination before recursive calls @@ -1139,7 +1158,8 @@ link_files () find "${src_dir}" -mindepth 1 -maxdepth 1 | \ while read src do - local dest="${dest_dir}$(basename "${src}")" + local dest final_src + dest="${dest_dir}$(basename "${src}")" if [ -d "${src}" ] then if [ -z "$(ls -A "${src}")" ] @@ -1154,7 +1174,7 @@ link_files () fi link_files "${src}" "${dest}" "${src_mask}" else - local final_src=${src} + final_src=${src} if [ -n "${src_mask}" ] then final_src="$(echo ${final_src} | sed "s|^${src_mask}||")" @@ -1168,10 +1188,11 @@ link_files () do_union () { - local unionmountpoint="${1}" # directory where the union is mounted - local unionrw="${2}" # branch where the union changes are stored - local unionro1="${3}" # first underlying read-only branch (optional) - local unionro2="${4}" # second underlying read-only branch (optional) + local unionmountpoint unionrw unionro1 unionro2 + unionmountpoint="${1}" # directory where the union is mounted + unionrw="${2}" # branch where the union changes are stored + unionro1="${3}" # first underlying read-only branch (optional) + unionro2="${4}" # second underlying read-only branch (optional) case "${UNIONTYPE}" in aufs) @@ -1236,12 +1257,13 @@ get_custom_mounts () # Side-effect: leaves $devices with persistence.conf mounted in ${rootmnt}/lib/live/mount/persistence # Side-effect: prints info to file $custom_mounts - local custom_mounts=${1} + local custom_mounts devices bindings links + custom_mounts=${1} shift - local devices=${@} + devices=${@} - local bindings="/tmp/bindings.list" - local links="/tmp/links.list" + bindings="/tmp/bindings.list" + links="/tmp/links.list" rm -rf ${bindings} ${links} 2> /dev/null for device in ${devices} @@ -1251,14 +1273,14 @@ get_custom_mounts () continue fi - local device_name="$(basename ${device})" - local backing=$(mount_persistence_media ${device}) + local device_name backing include_list + device_name="$(basename ${device})" + backing=$(mount_persistence_media ${device}) if [ -z "${backing}" ] then continue fi - local include_list if [ -r "${backing}/${persistence_list}" ] then include_list="${backing}/${persistence_list}" @@ -1288,8 +1310,9 @@ get_custom_mounts () continue fi - local opt_source="" - local opt_link="" + local opt_source opt_link source full_source full_dest + opt_source="" + opt_link="" for opt in $(echo ${options} | tr ',' ' '); do case "${opt}" in @@ -1308,7 +1331,7 @@ get_custom_mounts () esac done - local source="${dir}" + source="${dir}" if [ -n "${opt_source}" ] then if echo ${opt_source} | grep -q -e "^/" -e "^\(.*/\)\?\.\.\?\(/.*\)\?$" && [ "${opt_source}" != "." ] @@ -1320,8 +1343,8 @@ get_custom_mounts () fi fi - local full_source="$(trim_path ${backing}/${source})" - local full_dest="$(trim_path ${rootmnt}/${dir})" + full_source="$(trim_path ${backing}/${source})" + full_dest="$(trim_path ${rootmnt}/${dir})" if [ -n "${opt_link}" ] then echo "${device} ${full_source} ${full_dest} ${options}" >> ${links} @@ -1342,8 +1365,9 @@ get_custom_mounts () # We need to make sure that no two custom mounts have the same sources # or are nested; if that is the case, too much weird stuff can happen. - local prev_source="impossible source" # first iteration must not match - local prev_dest="" + local prev_source prev_dest + prev_source="impossible source" # first iteration must not match + prev_dest="" # This sort will ensure that a source /a comes right before a source # /a/b so we only need to look at the previous source sort -k2 -b ${custom_mounts} | @@ -1360,14 +1384,16 @@ get_custom_mounts () activate_custom_mounts () { - local custom_mounts="${1}" # the ouput from get_custom_mounts() - local used_devices="" + local custom_mounts used_devices + custom_mounts="${1}" # the ouput from get_custom_mounts() + used_devices="" while read device source dest options # < ${custom_mounts} do - local opt_bind="true" - local opt_link="" - local opt_union="" + local opt_bind opt_link opt_union + opt_bind="true" + opt_link="" + opt_union="" for opt in $(echo ${options} | tr ',' ' '); do case "${opt}" in @@ -1448,7 +1474,8 @@ activate_custom_mounts () # XXX: If CONFIG_AUFS_ROBR is added to the Debian kernel we can # ignore the loop below and set rootfs_dest_backing=$dest - local rootfs_dest_backing="" + local rootfs_dest_backing + rootfs_dest_backing="" if [ -n "${opt_link}"] then for d in ${rootmnt}/lib/live/mount/rootfs/* @@ -1468,13 +1495,14 @@ activate_custom_mounts () done fi + local cow_dir links_source if [ -n "${opt_link}" ] && [ -z "${PERSISTENCE_READONLY}" ] then link_files ${source} ${dest} ${rootmnt} elif [ -n "${opt_link}" ] && [ -n "${PERSISTENCE_READONLY}" ] then mkdir -p ${rootmnt}/lib/live/mount/persistence - local links_source=$(mktemp -d ${rootmnt}/lib/live/mount/persistence/links-source-XXXXXX) + links_source=$(mktemp -d ${rootmnt}/lib/live/mount/persistence/links-source-XXXXXX) chown_ref ${source} ${links_source} chmod_ref ${source} ${links_source} # We put the cow dir in the below strange place to @@ -1482,7 +1510,7 @@ activate_custom_mounts () # has its own directory and isn't nested with some # other custom mount (if so that mount's files would # be linked, causing breakage. - local cow_dir="${rootmnt}/lib/live/mount/overlay/lib/live/mount/persistence/$(basename ${links_source})" + cow_dir="${rootmnt}/lib/live/mount/overlay/lib/live/mount/persistence/$(basename ${links_source})" mkdir -p ${cow_dir} chown_ref "${source}" "${cow_dir}" chmod_ref "${source}" "${cow_dir}" @@ -1499,7 +1527,7 @@ activate_custom_mounts () # bind-mount and union mount are handled the same # in read-only mode, but note that rootfs_dest_backing # is non-empty (and necessary) only for unions - local cow_dir="${rootmnt}/lib/live/mount/overlay/${dest}" + cow_dir="${rootmnt}/lib/live/mount/overlay/${dest}" if [ -e "${cow_dir}" ] && [ -z "${opt_link}" ] then # If an earlier custom mount has files here diff --git a/scripts/boot/9990-mount-http.sh b/scripts/boot/9990-mount-http.sh index 1b718c0..b557404 100755 --- a/scripts/boot/9990-mount-http.sh +++ b/scripts/boot/9990-mount-http.sh @@ -8,8 +8,9 @@ do_httpmount () for webfile in HTTPFS FTPFS FETCH do - local url="$(eval echo \"\$\{${webfile}\}\")" - local extension="$(echo "${url}" | sed 's/\(.*\)\.\(.*\)/\2/')" + local url extension dest + url="$(eval echo \"\$\{${webfile}\}\")" + extension="$(echo "${url}" | sed 's/\(.*\)\.\(.*\)/\2/')" if [ -n "$url" ] then @@ -20,7 +21,7 @@ do_httpmount () mkdir -p "${alt_mountpoint}" dest="${alt_mountpoint}" else - local dest="${mountpoint}/${LIVE_MEDIA_PATH}" + dest="${mountpoint}/${LIVE_MEDIA_PATH}" mount -t ramfs ram "${mountpoint}" mkdir -p "${dest}" fi diff --git a/scripts/boot/9990-mount-iscsi.sh b/scripts/boot/9990-mount-iscsi.sh index fd29d91..6ce9851 100755 --- a/scripts/boot/9990-mount-iscsi.sh +++ b/scripts/boot/9990-mount-iscsi.sh @@ -7,7 +7,8 @@ do_iscsi() do_netsetup #modprobe ib_iser modprobe iscsi_tcp - local debugopt="" + local debugopt + debugopt="" [ "${DEBUG}" = "true" ] && debugopt="-d 8" #FIXME this name is supposed to be unique - some date + ifconfig hash? ISCSI_INITIATORNAME="iqn.1993-08.org.debian.live:01:$(echo "${HWADDR}" | sed -e s/://g)" @@ -21,12 +22,14 @@ do_iscsi() then panic "Failed to log into iscsi target" fi - local host="$(ls -d /sys/class/scsi_host/host*/device/iscsi_host:host* \ - /sys/class/scsi_host/host*/device/iscsi_host/host* | sed -e 's:/device.*::' -e 's:.*host::')" + local host + host="$(ls -d /sys/class/scsi_host/host*/device/iscsi_host:host* \ + /sys/class/scsi_host/host*/device/iscsi_host/host* | sed -e 's:/device.*::' -e 's:.*host::')" if [ -n "${host}" ] then - local devices="" - local i=0 + local devices i + devices="" + i=0 while [ -z "${devices}" -a $i -lt 60 ] do sleep 1 diff --git a/scripts/boot/9990-overlay.sh b/scripts/boot/9990-overlay.sh index 4282400..4739042 100755 --- a/scripts/boot/9990-overlay.sh +++ b/scripts/boot/9990-overlay.sh @@ -189,7 +189,8 @@ setup_unionfs () done fi - local whitelistdev="" + local whitelistdev + whitelistdev="" if [ -n "${PERSISTENCE_MEDIA}" ] then case "${PERSISTENCE_MEDIA}" in @@ -212,7 +213,8 @@ setup_unionfs () overlays="${custom_overlay_label}" fi - local overlay_devices="" + local overlay_devices + overlay_devices="" if [ "${whitelistdev}" != "ignore_all_devices" ] then for media in $(find_persistence_media "${overlays}" "${whitelistdev}") @@ -394,7 +396,8 @@ setup_unionfs () # Adding custom persistence if [ -n "${PERSISTENCE}" ] && [ -z "${NOPERSISTENCE}" ] then - local custom_mounts="/tmp/custom_mounts.list" + local custom_mounts + custom_mounts="/tmp/custom_mounts.list" rm -rf ${custom_mounts} 2> /dev/null # Gather information about custom mounts from devies detected as overlays @@ -403,7 +406,8 @@ setup_unionfs () [ -n "${DEBUG}" ] && cp ${custom_mounts} "/lib/live/mount/persistence" # Now we do the actual mounting (and symlinking) - local used_overlays="" + local used_overlays + used_overlays="" used_overlays=$(activate_custom_mounts ${custom_mounts}) rm ${custom_mounts} -- 2.1.4