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 <prudo@redhat.com>
Reviewed-by: Pingfan Liu <piliu@redhat.com>

kdump-lib.sh

@@ -853,62 +853,125 @@ has_aarch64_smmu()
 	ls /sys/devices/platform/arm-smmu-* 1> /dev/null 2>&1
 }
 
-# $1 crashkernel=""
+is_memsize() { [[ "$1" =~ ^[+-]?[0-9]+[KkMmGg]?$ ]]; }
-# $2 delta in unit of MB
+
-_crashkernel_add()
+# range defined for crashkernel parameter
+# i.e. <start>-[<end>]
+is_memrange()
 {
-	local _ck _add _entry _ret
+	is_memsize "${1%-*}" || return 1
-	local _range _size _offset
+	[[ -n ${1#*-} ]] || return 0
+	is_memsize "${1#*-}"
+}
 
-	_ck="$1"
+to_bytes()
-	_add="$2"
+{
-	_ret=""
+	local _s
 
-	if [[ "$_ck" == *@* ]]; then
+	_s="$1"
-		_offset="@${_ck##*@}"
+	is_memsize "$_s" || return 1
-		_ck=${_ck%@*}
+
-	elif [[ "$_ck" == *,high ]] || [[ "$_ck" == *,low ]]; then
+	case "${_s: -1}" in
-		_offset=",${_ck##*,}"
+		K|k)
-		_ck=${_ck%,*}
+			_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
+	while read -d , -r entry; do
-		[[ -n "$_entry" ]] || continue
+		[[ -n "$entry" ]] || continue
-		if [[ "$_entry" == *:* ]]; then
+		if [[ "$entry" == *:* ]]; then
-			_range=${_entry%:*}
+			range=${entry%:*}
-			_size=${_entry#*:}
+			size=${entry#*:}
 		else
-			_range=""
+			range=""
-			_size=${_entry}
+			size=${entry}
 		fi
 
-		case "${_size: -1}" in
+		echo "$size;$range;"
-			K)
+	done <<< "$ck,"
-				_size=${_size::-1}
+	echo ";;$offset"
-				_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
 
-		[[ -n "$_range" ]] && _ret+="$_range:"
+# $1 crashkernel command line parameter
-		_ret+="$_size,"
+# $2 size to be added
-	done <<< "$_ck,"
+_crashkernel_add()
+{
+	local ck delta ret
+	local range size offset
 
-	_ret=${_ret%,}
+	ck="$1"
-	[[ -n "$_offset" ]] && _ret+=$_offset
+	delta="$2"
-	echo "$_ret"
+	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"

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'"
+		Context "For valid input values"
-			delta=100
 			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" "100M" "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: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" "1G-4G:1124M,4G-64G:2148M,64G-:3172M@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" "1G-4G:112400K,4G-64G:122400K,64G-:142400K@4G"
+				"1G-4G:10000K,4G-64G:20000K,64G-:40000K@4G" "100M" "1G-4G:112400K,4G-64G:122400K,64G-:142400K@4G"
-				"300M,high" "400M,high"
+				"1,high" "1" "2,high"
-				"300M,low" "400M,low"
+				"1K,low" "1" "1025,low"
-				"500M@1G" "600M@1G"
+				"1M@1G" "1k" "1025K@1G"
+				"500M@1G" "-100m" "400M@1G"
+				"1099511627776" "0" "1024G"
 			End
-			It "should add delta to the values after ':'"
+			It "should add delta to every value after ':'"
-
+				When call _crashkernel_add "$1" "$2"
-				When call _crashkernel_add "$1" "$delta"
+				The output should equal "$3"
-				The output should equal "$2"
+			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