From 64f2827a4bfa3f26974972f2f5a4477eec58f221 Mon Sep 17 00:00:00 2001 From: Philipp Rudo Date: Wed, 6 Sep 2023 10:49:47 +0200 Subject: [PATCH] kdump-lib: Harden _crashkernel_add _crashkernel_add currently always assumes the good case, i.e. that the value of the crashkernel parameter has the correct syntax and that the delta added is a number. Both doesn't have to be true when the values are provided by users. Thus add some additional checks. Furthermore require the delta to have a explicit unit, i.e. no longer assume that is in megabytes, i.e. 100 -> 100M. Signed-off-by: Philipp Rudo Reviewed-by: Pingfan Liu --- kdump-lib.sh | 155 +++++++++++++++++++++++++++++------------ spec/kdump-lib_spec.sh | 38 ++++++---- 2 files changed, 134 insertions(+), 59 deletions(-) diff --git a/kdump-lib.sh b/kdump-lib.sh index 55c29bd..042ac87 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -853,62 +853,125 @@ has_aarch64_smmu() ls /sys/devices/platform/arm-smmu-* 1> /dev/null 2>&1 } -# $1 crashkernel="" -# $2 delta in unit of MB -_crashkernel_add() +is_memsize() { [[ "$1" =~ ^[+-]?[0-9]+[KkMmGg]?$ ]]; } + +# range defined for crashkernel parameter +# i.e. -[] +is_memrange() { - local _ck _add _entry _ret - local _range _size _offset + is_memsize "${1%-*}" || return 1 + [[ -n ${1#*-} ]] || return 0 + is_memsize "${1#*-}" +} - _ck="$1" - _add="$2" - _ret="" +to_bytes() +{ + local _s - if [[ "$_ck" == *@* ]]; then - _offset="@${_ck##*@}" - _ck=${_ck%@*} - elif [[ "$_ck" == *,high ]] || [[ "$_ck" == *,low ]]; then - _offset=",${_ck##*,}" - _ck=${_ck%,*} + _s="$1" + is_memsize "$_s" || return 1 + + case "${_s: -1}" in + K|k) + _s=${_s::-1} + _s="$((_s * 1024))" + ;; + M|m) + _s=${_s::-1} + _s="$((_s * 1024 * 1024))" + ;; + G|g) + _s=${_s::-1} + _s="$((_s * 1024 * 1024 * 1024))" + ;; + *) + ;; + esac + echo "$_s" +} + +memsize_add() +{ + local -a units=("" "K" "M" "G") + local i a b + + a=$(to_bytes "$1") || return 1 + b=$(to_bytes "$2") || return 1 + i=0 + + (( a += b )) + while :; do + [[ $(( a / 1024 )) -eq 0 ]] && break + [[ $(( a % 1024 )) -ne 0 ]] && break + [[ $(( ${#units[@]} - 1 )) -eq $i ]] && break + + (( a /= 1024 )) + (( i += 1 )) + done + + echo "${a}${units[$i]}" +} + +_crashkernel_parse() +{ + local ck entry + local range size offset + + ck="$1" + + if [[ "$ck" == *@* ]]; then + offset="@${ck##*@}" + ck=${ck%@*} + elif [[ "$ck" == *,high ]] || [[ "$ck" == *,low ]]; then + offset=",${ck##*,}" + ck=${ck%,*} else - _offset='' + offset='' fi - while read -d , -r _entry; do - [[ -n "$_entry" ]] || continue - if [[ "$_entry" == *:* ]]; then - _range=${_entry%:*} - _size=${_entry#*:} + while read -d , -r entry; do + [[ -n "$entry" ]] || continue + if [[ "$entry" == *:* ]]; then + range=${entry%:*} + size=${entry#*:} else - _range="" - _size=${_entry} + range="" + size=${entry} fi - case "${_size: -1}" in - K) - _size=${_size::-1} - _size="$((_size + (_add * 1024)))K" - ;; - M) - _size=${_size::-1} - _size="$((_size + _add))M" - ;; - G) - _size=${_size::-1} - _size="$((_size * 1024 + _add))M" - ;; - *) - _size="$((_size + (_add * 1024 * 1024)))" - ;; - esac + echo "$size;$range;" + done <<< "$ck," + echo ";;$offset" +} - [[ -n "$_range" ]] && _ret+="$_range:" - _ret+="$_size," - done <<< "$_ck," +# $1 crashkernel command line parameter +# $2 size to be added +_crashkernel_add() +{ + local ck delta ret + local range size offset - _ret=${_ret%,} - [[ -n "$_offset" ]] && _ret+=$_offset - echo "$_ret" + ck="$1" + delta="$2" + ret="" + + while IFS=';' read -r size range offset; do + if [[ -n "$offset" ]]; then + ret="${ret%,}$offset" + break + fi + + [[ -n "$size" ]] || continue + if [[ -n "$range" ]]; then + is_memrange "$range" || return 1 + ret+="$range:" + fi + + size=$(memsize_add "$size" "$delta") || return 1 + ret+="$size," + done < <( _crashkernel_parse "$ck") + + echo "${ret%,}" } # get default crashkernel @@ -958,7 +1021,7 @@ kdump_get_arch_recommend_crashkernel() #4k kernel, mlx5 consumes extra 124M memory, and choose 150M has_mlx5 && ((_delta += 150)) fi - _ck_cmdline=$(_crashkernel_add "$_ck_cmdline" "$_delta") + _ck_cmdline=$(_crashkernel_add "$_ck_cmdline" "${_delta}M") elif [[ $_arch == "ppc64le" ]]; then if [[ $_dump_mode == "fadump" ]]; then _ck_cmdline="4G-16G:768M,16G-64G:1G,64G-128G:2G,128G-1T:4G,1T-2T:6G,2T-4T:12G,4T-8T:20G,8T-16T:36G,16T-32T:64G,32T-64T:128G,64T-:180G" diff --git a/spec/kdump-lib_spec.sh b/spec/kdump-lib_spec.sh index 81f0f86..814f961 100644 --- a/spec/kdump-lib_spec.sh +++ b/spec/kdump-lib_spec.sh @@ -49,21 +49,33 @@ Describe 'kdump-lib' End Describe "_crashkernel_add()" - Context "when the input parameter is '1G-4G:256M,4G-64G:320M,64G-:576M'" - delta=100 + Context "For valid input values" Parameters - "1G-4G:256M,4G-64G:320M,64G-:576M" "1G-4G:356M,4G-64G:420M,64G-:676M" - "1G-4G:256M,4G-64G:320M,64G-:576M@4G" "1G-4G:356M,4G-64G:420M,64G-:676M@4G" - "1G-4G:1G,4G-64G:2G,64G-:3G@4G" "1G-4G:1124M,4G-64G:2148M,64G-:3172M@4G" - "1G-4G:10000K,4G-64G:20000K,64G-:40000K@4G" "1G-4G:112400K,4G-64G:122400K,64G-:142400K@4G" - "300M,high" "400M,high" - "300M,low" "400M,low" - "500M@1G" "600M@1G" + "1G-4G:256M,4G-64G:320M,64G-:576M" "100M" "1G-4G:356M,4G-64G:420M,64G-:676M" + "1G-4G:256M,4G-64G:320M,64G-:576M@4G" "100M" "1G-4G:356M,4G-64G:420M,64G-:676M@4G" + "1G-4G:1G,4G-64G:2G,64G-:3G@4G" "100M" "1G-4G:1124M,4G-64G:2148M,64G-:3172M@4G" + "1G-4G:10000K,4G-64G:20000K,64G-:40000K@4G" "100M" "1G-4G:112400K,4G-64G:122400K,64G-:142400K@4G" + "1,high" "1" "2,high" + "1K,low" "1" "1025,low" + "1M@1G" "1k" "1025K@1G" + "500M@1G" "-100m" "400M@1G" + "1099511627776" "0" "1024G" End - It "should add delta to the values after ':'" - - When call _crashkernel_add "$1" "$delta" - The output should equal "$2" + It "should add delta to every value after ':'" + When call _crashkernel_add "$1" "$2" + The output should equal "$3" + End + End + Context "For invalid input values" + Parameters + "1G-4G:256M.4G-64G:320M" "100M" + "foo" "1" + "1" "bar" + End + It "shall return an error" + When call _crashkernel_add "$1" "$2" + The output should equal "" + The status should be failure End End End