From a8051cf9e21d231ce3c445f09631266157ffc2e0 Mon Sep 17 00:00:00 2001 From: Reid wahl Date: Fri, 10 Jul 2020 03:44:18 -0700 Subject: [PATCH 1/3] Filesystem: Support whitespace in device or directory name Whitespace in a device name (e.g., a CIFS share) or a directory name breaks resource operations. One issue is that many of the variable occurrences aren't quoted, so a string containing whitespace is split into multiple tokens. This is a problem when the string meant to be passed as a single argument to a function (e.g., `list_submounts()`). Another issue involves the parsing of `list_mounts()` output. `list_mounts()` can pull data from `/proc/mounts`, `/etc/mtab`, or the `mount` command. `/proc/mounts` and `/etc/mtab` represent spaces within a field as octal `\040` strings, while `mount` represents them as literal space characters. `list_mounts()` had to be modified to output the mount list as three distinct fields ((`device`, `mountpoint`, `fstype`), separated by tab characters) regardless of the data source. Parsers of `list_mounts()` were modified to use tabs as field delimiters. The for loop in `Filesystem_stop()` also had to become a while loop to read line-by-line irrespective of spaces. A for loop splits on spaces. Resolves: RHBZ#1624591 --- heartbeat/Filesystem | 106 +++++++++++++++++++++++++------------------ 1 file changed, 61 insertions(+), 45 deletions(-) diff --git a/heartbeat/Filesystem b/heartbeat/Filesystem index 2f07a90ad..9a52aa712 100755 --- a/heartbeat/Filesystem +++ b/heartbeat/Filesystem @@ -91,6 +91,7 @@ fi # Variables used by multiple methods HOSTOS=`uname` +TAB=' ' # The status file is going to an extra directory, by default # @@ -100,7 +101,7 @@ suffix="${OCF_RESOURCE_INSTANCE}" [ "$OCF_RESKEY_CRM_meta_clone" ] && suffix="${suffix}_$OCF_RESKEY_CRM_meta_clone" suffix="${suffix}_`uname -n`" -STATUSFILE=${OCF_RESKEY_directory}/$prefix$suffix +STATUSFILE="${OCF_RESKEY_directory}/$prefix$suffix" ####################################################################### @@ -283,6 +284,7 @@ flushbufs() { is_bind_mount() { echo "$options" | grep -w bind >/dev/null 2>&1 } + list_mounts() { local inpf="" local mount_list="" @@ -296,15 +298,23 @@ list_mounts() { # Make sure that the mount list has not been changed while reading. while [ "$mount_list" != "$check_list" ]; do - check_list=$mount_list + check_list="$mount_list" if [ "$inpf" ]; then - mount_list=$(cut -d' ' -f1,2,3 < $inpf) + # ... + # Spaces in device or mountpoint are octal \040 in $inpf + # Convert literal spaces (field separators) to tabs + mount_list=$(cut -d' ' -f1,2,3 < $inpf | tr ' ' "$TAB") else - mount_list=$($MOUNT | cut -d' ' -f1,3,5) + # on type ... + # Use tabs as field separators + match_string='\(.*\) on \(.*\) type \([^[:space:]]\)' + replace_string="\\1${TAB}\\3${TAB}\\5" + mount_list=$($MOUNT | sed "s/$match_string/$replace_string/g") fi done - echo "$mount_list" + # Convert octal \040 to space characters + printf "$mount_list" } determine_blockdevice() { @@ -318,7 +328,8 @@ determine_blockdevice() { nfs4|nfs|smbfs|cifs|glusterfs|ceph|tmpfs|overlay|overlayfs|rozofs|zfs|cvfs|none|lustre) : ;; *) - DEVICE=`list_mounts | grep " $CANONICALIZED_MOUNTPOINT " | cut -d' ' -f1` + match_string="${TAB}${CANONICALIZED_MOUNTPOINT}${TAB}" + DEVICE=`list_mounts | grep "$match_string" | cut -d"$TAB" -f1` if [ -b "$DEVICE" ]; then blockdevice=yes fi @@ -329,7 +340,7 @@ determine_blockdevice() { # Lists all filesystems potentially mounted under a given path, # excluding the path itself. list_submounts() { - list_mounts | grep " $1/" | cut -d' ' -f2 | sort -r + list_mounts | grep "${TAB}${1}/" | cut -d"$TAB" -f2 | sort -r } # kernels < 2.6.26 can't handle bind remounts @@ -358,15 +369,15 @@ bind_mount() { if is_bind_mount && [ "$options" != "-o bind" ] then bind_kernel_check - bind_opts=`echo $options | sed 's/bind/remount/'` - $MOUNT $bind_opts $MOUNTPOINT + bind_opts=`echo "$options" | sed 's/bind/remount/'` + $MOUNT $bind_opts "$MOUNTPOINT" else true # make sure to return OK fi } is_option() { - echo $OCF_RESKEY_options | grep -w "$1" >/dev/null 2>&1 + echo "$OCF_RESKEY_options" | grep -w "$1" >/dev/null 2>&1 } is_fsck_needed() { @@ -374,7 +385,7 @@ is_fsck_needed() { force) true;; no) false;; ""|auto) - case $FSTYPE in + case "$FSTYPE" in ext4|ext4dev|ext3|reiserfs|reiser4|nss|xfs|jfs|vfat|fat|nfs4|nfs|cifs|smbfs|ocfs2|gfs2|none|lustre|glusterfs|ceph|tmpfs|overlay|overlayfs|rozofs|zfs|cvfs) false;; *) @@ -403,7 +414,7 @@ fstype_supported() fi # support fuse-filesystems (e.g. GlusterFS) - case $FSTYPE in + case "$FSTYPE" in fuse.*|glusterfs|rozofs) support="fuse";; esac @@ -486,7 +497,8 @@ trigger_udev_rules_if_needed() Filesystem_start() { # Check if there are any mounts mounted under the mountpoint - if list_mounts | grep -q -E " $CANONICALIZED_MOUNTPOINT/\w+" >/dev/null 2>&1; then + match_string="${TAB}${CANONICALIZED_MOUNTPOINT}" + if list_mounts | grep -q -E "$match_string/\w+" >/dev/null 2>&1; then ocf_log err "There is one or more mounts mounted under $MOUNTPOINT." return $OCF_ERR_CONFIGURED fi @@ -514,9 +526,9 @@ Filesystem_start() if is_fsck_needed; then ocf_log info "Starting filesystem check on $DEVICE" if [ -z "$FSTYPE" ]; then - $FSCK -p $DEVICE + $FSCK -p "$DEVICE" else - $FSCK -t $FSTYPE -p $DEVICE + $FSCK -t "$FSTYPE" -p "$DEVICE" fi # NOTE: if any errors at all are detected, it returns non-zero @@ -529,20 +541,20 @@ Filesystem_start() fi [ -d "$MOUNTPOINT" ] || - ocf_run mkdir -p $MOUNTPOINT + ocf_run mkdir -p "$MOUNTPOINT" if [ ! -d "$MOUNTPOINT" ] ; then ocf_exit_reason "Couldn't find directory [$MOUNTPOINT] to use as a mount point" exit $OCF_ERR_INSTALLED fi - flushbufs $DEVICE + flushbufs "$DEVICE" # Mount the filesystem. case "$FSTYPE" in - none) $MOUNT $options $DEVICE $MOUNTPOINT && + none) $MOUNT $options "$DEVICE" "$MOUNTPOINT" && bind_mount ;; - "") $MOUNT $options $DEVICE $MOUNTPOINT ;; - *) $MOUNT -t $FSTYPE $options $DEVICE $MOUNTPOINT ;; + "") $MOUNT $options "$DEVICE" "$MOUNTPOINT" ;; + *) $MOUNT -t "$FSTYPE" $options "$DEVICE" "$MOUNTPOINT" ;; esac if [ $? -ne 0 ]; then @@ -595,23 +607,23 @@ signal_processes() { done } try_umount() { - local SUB=$1 - $UMOUNT $umount_force $SUB - list_mounts | grep -q " $SUB " >/dev/null 2>&1 || { + local SUB="$1" + $UMOUNT $umount_force "$SUB" + list_mounts | grep -q "${TAB}${SUB}${TAB}" >/dev/null 2>&1 || { ocf_log info "unmounted $SUB successfully" return $OCF_SUCCESS } return $OCF_ERR_GENERIC } fs_stop() { - local SUB=$1 timeout=$2 sig cnt + local SUB="$1" timeout=$2 sig cnt for sig in TERM KILL; do cnt=$((timeout/2)) # try half time with TERM while [ $cnt -gt 0 ]; do - try_umount $SUB && + try_umount "$SUB" && return $OCF_SUCCESS ocf_exit_reason "Couldn't unmount $SUB; trying cleanup with $sig" - signal_processes $SUB $sig + signal_processes "$SUB" $sig cnt=$((cnt-1)) sleep 1 done @@ -633,7 +645,7 @@ Filesystem_stop() # Wipe the status file, but continue with a warning if # removal fails -- the file system might be read only if [ $OCF_CHECK_LEVEL -eq 20 ]; then - rm -f ${STATUSFILE} + rm -f "${STATUSFILE}" if [ $? -ne 0 ]; then ocf_log warn "Failed to remove status file ${STATUSFILE}." fi @@ -650,7 +662,7 @@ Filesystem_stop() # Umount all sub-filesystems mounted under $MOUNTPOINT/ too. local timeout - for SUB in `list_submounts $MOUNTPOINT` $MOUNTPOINT; do + while read SUB; do ocf_log info "Trying to unmount $SUB" if ocf_is_true "$FAST_STOP"; then timeout=6 @@ -658,15 +670,18 @@ Filesystem_stop() timeout=${OCF_RESKEY_CRM_meta_timeout:="20000"} timeout=$((timeout/1000)) fi - fs_stop $SUB $timeout + fs_stop "$SUB" $timeout rc=$? if [ $rc -ne $OCF_SUCCESS ]; then ocf_exit_reason "Couldn't unmount $SUB, giving up!" fi - done + done <<-EOF + $(list_submounts "$CANONICALIZED_MOUNTPOINT"; \ + echo $CANONICALIZED_MOUNTPOINT) + EOF fi - flushbufs $DEVICE + flushbufs "$DEVICE" return $rc } @@ -677,7 +692,8 @@ Filesystem_stop() # Filesystem_status() { - if list_mounts | grep -q " $CANONICALIZED_MOUNTPOINT " >/dev/null 2>&1; then + match_string="${TAB}${CANONICALIZED_MOUNTPOINT}${TAB}" + if list_mounts | grep -q "$match_string" >/dev/null 2>&1; then rc=$OCF_SUCCESS msg="$MOUNTPOINT is mounted (running)" else @@ -712,7 +728,7 @@ Filesystem_monitor_10() return $OCF_SUCCESS fi dd_opts="iflag=direct bs=4k count=1" - err_output=`dd if=$DEVICE $dd_opts 2>&1 >/dev/null` + err_output=`dd if="$DEVICE" $dd_opts 2>&1 >/dev/null` if [ $? -ne 0 ]; then ocf_exit_reason "Failed to read device $DEVICE" ocf_log err "dd said: $err_output" @@ -733,20 +749,20 @@ Filesystem_monitor_20() # to bypass caches. dd_opts="oflag=direct,sync bs=4k conv=fsync,sync" fi - status_dir=`dirname $STATUSFILE` + status_dir=$(dirname "$STATUSFILE") [ -d "$status_dir" ] || mkdir -p "$status_dir" - err_output=`echo "${OCF_RESOURCE_INSTANCE}" | dd of=${STATUSFILE} $dd_opts 2>&1` + err_output=`echo "${OCF_RESOURCE_INSTANCE}" | dd of="${STATUSFILE}" $dd_opts 2>&1` if [ $? -ne 0 ]; then ocf_exit_reason "Failed to write status file ${STATUSFILE}" ocf_log err "dd said: $err_output" return $OCF_ERR_GENERIC fi - test -f ${STATUSFILE} + test -f "${STATUSFILE}" if [ $? -ne 0 ]; then ocf_exit_reason "Cannot stat the status file ${STATUSFILE}" return $OCF_ERR_GENERIC fi - cat ${STATUSFILE} > /dev/null + cat "${STATUSFILE}" > /dev/null if [ $? -ne 0 ]; then ocf_exit_reason "Cannot read the status file ${STATUSFILE}" return $OCF_ERR_GENERIC @@ -791,9 +807,9 @@ Filesystem_validate_all() # NOTE: Without inserting the $FSTYPE module, this step may be imprecise # TODO: This is Linux specific crap. if [ ! -z "$FSTYPE" -a "$FSTYPE" != none ]; then - cut -f2 /proc/filesystems |grep -q ^$FSTYPE$ + cut -f2 /proc/filesystems |grep -q "^${FSTYPE}$" if [ $? -ne 0 ]; then - modpath=/lib/modules/`uname -r` + modpath=/lib/modules/`uname -r` moddep=$modpath/modules.dep # Do we have $FSTYPE in modules.dep? cut -d' ' -f1 $moddep |grep -q "^$modpath.*$FSTYPE\.k\?o:$" @@ -826,7 +842,7 @@ set_blockdevice_var() { blockdevice=no # these are definitely not block devices - case $FSTYPE in + case "$FSTYPE" in nfs4|nfs|smbfs|cifs|none|glusterfs|ceph|tmpfs|overlay|overlayfs|rozofs|zfs|cvfs) return;; esac @@ -834,7 +850,7 @@ set_blockdevice_var() { return fi - case $DEVICE in + case "$DEVICE" in -*) # Oh... An option to mount instead... Typically -U or -L ;; /dev/null) # Special case for BSC @@ -863,7 +879,7 @@ if [ -n "${OCF_RESKEY_force_unmount}" ]; then FORCE_UNMOUNT=$OCF_RESKEY_force_unmount fi -DEVICE=$OCF_RESKEY_device +DEVICE="$OCF_RESKEY_device" FSTYPE=$OCF_RESKEY_fstype if [ ! -z "$OCF_RESKEY_options" ]; then options="-o $OCF_RESKEY_options" @@ -899,10 +915,10 @@ if [ -z "$OCF_RESKEY_directory" ]; then exit $OCF_ERR_CONFIGURED fi else - MOUNTPOINT=$(echo $OCF_RESKEY_directory | sed 's/\/*$//') + MOUNTPOINT="$(echo "$OCF_RESKEY_directory" | sed 's/\/*$//')" : ${MOUNTPOINT:=/} if [ -e "$MOUNTPOINT" ] ; then - CANONICALIZED_MOUNTPOINT=$(readlink -f "$MOUNTPOINT") + CANONICALIZED_MOUNTPOINT="$(readlink -f "$MOUNTPOINT")" if [ $? -ne 0 ]; then ocf_exit_reason "Could not canonicalize $MOUNTPOINT because readlink failed" exit $OCF_ERR_GENERIC @@ -947,7 +963,7 @@ CLUSTERSAFE=0 is_option "ro" && CLUSTERSAFE=2 -case $FSTYPE in +case "$FSTYPE" in nfs4|nfs|smbfs|cifs|none|gfs2|glusterfs|ceph|ocfs2|overlay|overlayfs|tmpfs|cvfs) CLUSTERSAFE=1 # this is kind of safe too ;; From eca9a96ad3356df3636bfa3187afe1b1954693b2 Mon Sep 17 00:00:00 2001 From: Reid wahl Date: Fri, 10 Jul 2020 16:38:04 -0700 Subject: [PATCH 2/3] Filesystem: POSIX-compliant syntax for portability Updated to use POSIX `$()` instead of Bourne-shell backticks, and to use `grep ... >/dev/null 2>&1` instead of `grep -q`. (Note: `grep -q` only suppresses `stdout` anyway. `grep -q -s` would be required to suppress both `stdout` and `stderr`.) --- heartbeat/Filesystem | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/heartbeat/Filesystem b/heartbeat/Filesystem index 9a52aa712..34ade20d7 100755 --- a/heartbeat/Filesystem +++ b/heartbeat/Filesystem @@ -90,7 +90,7 @@ fi : ${OCF_RESKEY_force_unmount=${OCF_RESKEY_force_unmount_default}} # Variables used by multiple methods -HOSTOS=`uname` +HOSTOS=$(uname) TAB=' ' # The status file is going to an extra directory, by default @@ -100,7 +100,7 @@ prefix=${OCF_RESKEY_statusfile_prefix} suffix="${OCF_RESOURCE_INSTANCE}" [ "$OCF_RESKEY_CRM_meta_clone" ] && suffix="${suffix}_$OCF_RESKEY_CRM_meta_clone" -suffix="${suffix}_`uname -n`" +suffix="${suffix}_$(uname -n)" STATUSFILE="${OCF_RESKEY_directory}/$prefix$suffix" ####################################################################### @@ -329,7 +329,7 @@ determine_blockdevice() { : ;; *) match_string="${TAB}${CANONICALIZED_MOUNTPOINT}${TAB}" - DEVICE=`list_mounts | grep "$match_string" | cut -d"$TAB" -f1` + DEVICE=$(list_mounts | grep "$match_string" | cut -d"$TAB" -f1) if [ -b "$DEVICE" ]; then blockdevice=yes fi @@ -354,7 +354,7 @@ bind_kernel_check() { exit(1); }' [ $? -ne 0 ] && - ocf_log warn "kernel `uname -r` cannot handle read only bind mounts" + ocf_log warn "kernel $(uname -r) cannot handle read only bind mounts" } bind_root_mount_check() { @@ -369,7 +369,7 @@ bind_mount() { if is_bind_mount && [ "$options" != "-o bind" ] then bind_kernel_check - bind_opts=`echo "$options" | sed 's/bind/remount/'` + bind_opts=$(echo "$options" | sed 's/bind/remount/') $MOUNT $bind_opts "$MOUNTPOINT" else true # make sure to return OK @@ -469,7 +469,7 @@ trigger_udev_rules_if_needed() refresh_flag="yes" fi else - tmp="`echo $DEVICE|awk '{$1=""; print substr($0,2)}'`" + tmp="$(echo $DEVICE|awk '{$1=""; print substr($0,2)}')" case "$DEVICE" in -U*|--uuid*) tmp="/dev/disk/by-uuid/$tmp" @@ -498,7 +498,7 @@ Filesystem_start() { # Check if there are any mounts mounted under the mountpoint match_string="${TAB}${CANONICALIZED_MOUNTPOINT}" - if list_mounts | grep -q -E "$match_string/\w+" >/dev/null 2>&1; then + if list_mounts | grep -E "$match_string/\w+" >/dev/null 2>&1; then ocf_log err "There is one or more mounts mounted under $MOUNTPOINT." return $OCF_ERR_CONFIGURED fi @@ -602,14 +602,14 @@ signal_processes() { return fi for pid in $pids; do - ocf_log info "sending signal $sig to: `ps -f $pid | tail -1`" + ocf_log info "sending signal $sig to: $(ps -f $pid | tail -1)" kill -s $sig $pid done } try_umount() { local SUB="$1" $UMOUNT $umount_force "$SUB" - list_mounts | grep -q "${TAB}${SUB}${TAB}" >/dev/null 2>&1 || { + list_mounts | grep "${TAB}${SUB}${TAB}" >/dev/null 2>&1 || { ocf_log info "unmounted $SUB successfully" return $OCF_SUCCESS } @@ -693,7 +693,7 @@ Filesystem_stop() Filesystem_status() { match_string="${TAB}${CANONICALIZED_MOUNTPOINT}${TAB}" - if list_mounts | grep -q "$match_string" >/dev/null 2>&1; then + if list_mounts | grep "$match_string" >/dev/null 2>&1; then rc=$OCF_SUCCESS msg="$MOUNTPOINT is mounted (running)" else @@ -728,7 +728,7 @@ Filesystem_monitor_10() return $OCF_SUCCESS fi dd_opts="iflag=direct bs=4k count=1" - err_output=`dd if="$DEVICE" $dd_opts 2>&1 >/dev/null` + err_output=$(dd if="$DEVICE" $dd_opts 2>&1 >/dev/null) if [ $? -ne 0 ]; then ocf_exit_reason "Failed to read device $DEVICE" ocf_log err "dd said: $err_output" @@ -751,7 +751,7 @@ Filesystem_monitor_20() fi status_dir=$(dirname "$STATUSFILE") [ -d "$status_dir" ] || mkdir -p "$status_dir" - err_output=`echo "${OCF_RESOURCE_INSTANCE}" | dd of="${STATUSFILE}" $dd_opts 2>&1` + err_output=$(echo "${OCF_RESOURCE_INSTANCE}" | dd of="${STATUSFILE}" $dd_opts 2>&1) if [ $? -ne 0 ]; then ocf_exit_reason "Failed to write status file ${STATUSFILE}" ocf_log err "dd said: $err_output" @@ -807,12 +807,13 @@ Filesystem_validate_all() # NOTE: Without inserting the $FSTYPE module, this step may be imprecise # TODO: This is Linux specific crap. if [ ! -z "$FSTYPE" -a "$FSTYPE" != none ]; then - cut -f2 /proc/filesystems |grep -q "^${FSTYPE}$" + cut -f2 /proc/filesystems | grep "^${FSTYPE}$" >/dev/null 2>&1 if [ $? -ne 0 ]; then - modpath=/lib/modules/`uname -r` + modpath=/lib/modules/$(uname -r) moddep=$modpath/modules.dep # Do we have $FSTYPE in modules.dep? - cut -d' ' -f1 $moddep |grep -q "^$modpath.*$FSTYPE\.k\?o:$" + cut -d' ' -f1 $moddep \ + | grep "^${modpath}.*${FSTYPE}\.k\?o:$" >/dev/null 2>&1 if [ $? -ne 0 ]; then ocf_log info "It seems we do not have $FSTYPE support" fi @@ -846,7 +847,7 @@ set_blockdevice_var() { nfs4|nfs|smbfs|cifs|none|glusterfs|ceph|tmpfs|overlay|overlayfs|rozofs|zfs|cvfs|lustre) return;; esac - if `is_option "loop"`; then + if $(is_option "loop"); then return fi From 5517712f4bb6e90b23cde6310c03509c9061cb36 Mon Sep 17 00:00:00 2001 From: Reid wahl Date: Fri, 10 Jul 2020 16:44:17 -0700 Subject: [PATCH 3/3] Filesystem: Convert leading space characters to tabs A few lines started with spaces instead of tabs. Tabs are the convention in this file. --- heartbeat/Filesystem | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/heartbeat/Filesystem b/heartbeat/Filesystem index 34ade20d7..501e5a0d0 100755 --- a/heartbeat/Filesystem +++ b/heartbeat/Filesystem @@ -359,10 +359,10 @@ bind_kernel_check() { bind_root_mount_check() { if [ "$(df -P "$1" | awk 'END{print $6}')" = "/" ]; then - return 1 - else - return 0 - fi + return 1 + else + return 0 + fi } bind_mount() { @@ -571,10 +571,10 @@ get_pids() local procs local mmap_procs - if is_bind_mount && ocf_is_true "$FORCE_UNMOUNT" && ! bind_root_mount_check "$DEVICE"; then - ocf_log debug "Change force_umount from '$FORCE_UNMOUNT' to 'safe'" - FORCE_UNMOUNT=safe - fi + if is_bind_mount && ocf_is_true "$FORCE_UNMOUNT" && ! bind_root_mount_check "$DEVICE"; then + ocf_log debug "Change force_umount from '$FORCE_UNMOUNT' to 'safe'" + FORCE_UNMOUNT=safe + fi if ocf_is_true "$FORCE_UNMOUNT"; then if [ "X${HOSTOS}" = "XOpenBSD" ];then