commit 1c122bbd3c88d89695d3b60c1070d87073d56121 Author: Andrey Albershteyn Date: Wed Nov 11 16:11:08 2026 +0100 xfs_admin: remove eval use The eval is known to cause issues due to the fact that it parses and executes strings as code. Let's get rid of it. Remove use of eval and instead set a bunch of boolean and value flags. These are latter appended to the empty set of positional parameters $@. These parameters are not parsed. Reported-by: Hazley Samsudin Signed-off-by: Andrey Albershteyn Reviewed-by: Eric Sandeen diff --git a/db/xfs_admin.sh b/db/xfs_admin.sh index 13a876ad..6b210281 100755 --- a/db/xfs_admin.sh +++ b/db/xfs_admin.sh @@ -7,53 +7,74 @@ status=0 require_offline="" require_online="" -DB_OPTS="" -DB_DEV_OPTS="" -REPAIR_OPTS="" -IO_OPTS="" -SPACEMAN_OPTS="" -REPAIR_DEV_OPTS="" -LOG_OPTS="" +have_db="" +have_repair="" +opt_extflg="" # -e +opt_force="" # -f +opt_log2="" # -j +opt_projid32bit="" # -p +opt_get_label="" # -l +opt_get_uuid="" # -u +opt_cfgfile="" # -C +cfgfile="" +opt_set_label="" # -L +set_label="" +opt_set_uuid="" # -U +set_uuid="" +opt_rtdev="" # -r +rtdev="" +repair_tokens="" + USAGE="Usage: xfs_admin [-efjlpuV] [-C cfgfile] [-c 0|1] [-L label] [-O v5_feature] [-r rtdev] [-U uuid] device [logdev]" while getopts "C:c:efjlL:O:pr:uU:V" c do case $c in - c) REPAIR_OPTS=$REPAIR_OPTS" -c lazycount="$OPTARG + c) repair_tokens="$repair_tokens lazycount=$OPTARG" + require_offline=1 + have_repair=1 + ;; + C) opt_cfgfile=1 + cfgfile=$OPTARG + have_db=1 + ;; + e) opt_extflg=1 + have_db=1 require_offline=1 ;; - C) DB_OPTS=$DB_OPTS" -c 'makecfg "$OPTARG"'" - SPACEMAN_OPTS=$SPACEMAN_OPTS" -c 'makecfg "$OPTARG"'" - ;; - e) DB_OPTS=$DB_OPTS" -c 'version extflg'" + f) opt_force=1 + have_db=1 require_offline=1 ;; - f) DB_OPTS=$DB_OPTS" -f" + j) opt_log2=1 + have_db=1 require_offline=1 ;; - j) DB_OPTS=$DB_OPTS" -c 'version log2'" + l) opt_get_label=1 + have_db=1 + ;; + L) opt_set_label=1 + set_label=$OPTARG + have_db=1 + ;; + O) repair_tokens="$repair_tokens $OPTARG" + require_offline=1 + have_repair=1 + ;; + p) opt_projid32bit=1 + have_db=1 require_offline=1 ;; - l) DB_OPTS=$DB_OPTS" -r -c label" - IO_OPTS=$IO_OPTS" -r -c label" - ;; - L) DB_OPTS=$DB_OPTS" -c 'label "$OPTARG"'" - IO_OPTS=$IO_OPTS" -c 'label -s "$OPTARG"'" - ;; - O) REPAIR_OPTS=$REPAIR_OPTS" -c $OPTARG" + r) opt_rtdev=1 + rtdev=$OPTARG require_offline=1 ;; - p) DB_OPTS=$DB_OPTS" -c 'version projid32bit'" - require_offline=1 + u) opt_get_uuid=1 + have_db=1 ;; - r) REPAIR_DEV_OPTS=" -r '$OPTARG'" - DB_DEV_OPTS=" -R '$OPTARG'" - require_offline=1 - ;; - u) DB_OPTS=$DB_OPTS" -r -c uuid" - IO_OPTS=$IO_OPTS" -r -c fsuuid" - ;; - U) DB_OPTS=$DB_OPTS" -c 'uuid "$OPTARG"'" + U) opt_set_uuid=1 + set_uuid=$OPTARG + have_db=1 require_offline=1 ;; V) xfs_db -p xfs_admin -V @@ -65,50 +86,85 @@ do ;; esac done -set -- extra $@ +set -- extra "$@" shift $OPTIND +device=$1 +logdev=$2 + +# We use a bunch of opt_* flags here instead of crafting argument strings +# directly in the getopts loop above. This is because passing such strings +# to eval could cause command injection. +# +# As we cannot use arrays in POSIX, instead we resort to using the positional +# parameters ($@). Before calling any of the xfs_* utilities we reset $@ to an +# empty set with "set --". Then, based on the flags parsed above, we append all +# necessary arguments. This way each argument is passed verbatim, without being +# re-parsed or word-split by the shell. case $# in 1|2) - if mntpt="$(findmnt -t xfs -f -n -o TARGET "$1" 2>/dev/null)"; then + if mntpt="$(findmnt -t xfs -f -n -o TARGET "$device" 2>/dev/null)"; then # filesystem is mounted if [ -n "$require_offline" ]; then - echo "$1: filesystem is mounted." + echo "$device: filesystem is mounted." exit 2 fi - if [ -n "$SPACEMAN_OPTS" ]; then - eval xfs_spaceman -p xfs_admin $SPACEMAN_OPTS "$mntpt" - res=$? - test $res -ne 0 && exit $res - test -n "$IO_OPTS" || exit $res + have_io="" + if [ -n "$opt_get_label" ] || [ -n "$opt_set_label" ] || \ + [ -n "$opt_get_uuid" ]; then + have_io=1 fi - if [ -n "$IO_OPTS" ]; then - eval xfs_io -p xfs_admin $IO_OPTS "$mntpt" - exit $? + if [ -n "$opt_cfgfile" ]; then + xfs_spaceman -p xfs_admin -c "makecfg $cfgfile" "$mntpt" + res=$? + test $res -ne 0 && exit $res + test -n "$have_io" || exit $res + fi + + if [ -n "$have_io" ]; then + set -- + [ -n "$opt_get_label" ] && set -- "$@" -r -c label + [ -n "$opt_set_label" ] && set -- "$@" -c "label -s $set_label" + [ -n "$opt_get_uuid" ] && set -- "$@" -r -c fsuuid + exec xfs_io -p xfs_admin "$@" "$mntpt" fi fi # filesystem is not mounted if [ -n "$require_online" ]; then - echo "$1: filesystem is not mounted" + echo "$device: filesystem is not mounted" exit 2 fi - # Pick up the log device, if present - if [ -n "$2" ]; then - LOG_OPTS=" -l '$2'" - fi - - if [ -n "$DB_OPTS" ] + if [ -n "$have_db" ] then - eval xfs_db -x -p xfs_admin $LOG_OPTS $DB_DEV_OPTS $DB_OPTS "$1" + set -- + [ -n "$logdev" ] && set -- "$@" -l "$logdev" + [ -n "$opt_rtdev" ] && set -- "$@" -R "$rtdev" + [ -n "$opt_cfgfile" ] && set -- "$@" -c "makecfg $cfgfile" + [ -n "$opt_extflg" ] && set -- "$@" -c "version extflg" + [ -n "$opt_force" ] && set -- "$@" -f + [ -n "$opt_log2" ] && set -- "$@" -c "version log2" + [ -n "$opt_get_label" ] && set -- "$@" -r -c label + [ -n "$opt_set_label" ] && set -- "$@" -c "label $set_label" + [ -n "$opt_projid32bit" ] && set -- "$@" -c "version projid32bit" + [ -n "$opt_get_uuid" ] && set -- "$@" -r -c uuid + [ -n "$opt_set_uuid" ] && set -- "$@" -c "uuid $set_uuid" + xfs_db -x -p xfs_admin "$@" "$device" status=$? fi - if [ -n "$REPAIR_OPTS" ] + if [ -n "$have_repair" ] then echo "Running xfs_repair to upgrade filesystem." - eval xfs_repair $LOG_OPTS $REPAIR_DEV_OPTS $REPAIR_OPTS "$1" + set -- + [ -n "$logdev" ] && set -- "$@" -l "$logdev" + [ -n "$opt_rtdev" ] && set -- "$@" -r "$rtdev" + for token in $repair_tokens + do + set -- "$@" -c "$token" + done + xfs_repair "$@" "$device" status=`expr $? + $status` fi ;;