[PATCH] Add sanity checks for the log level

[Lianbo Jiang]

Let's add sanity checks for the log level in order to avoid
passing illegal log level to the logger.

Signed-off-by: Lianbo Jiang 
---
 kdump-logger.sh | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/kdump-logger.sh b/kdump-logger.sh
index c7afa340acfa..a3d674cd07c2 100755
--- a/kdump-logger.sh
+++ b/kdump-logger.sh
@@ -34,15 +34,42 @@
 # First of all you have to start with dlog_init() function which initializes
 # required variables. Don't call any other logging function before that one!
 #
+
+# @brief Check the log level.
+# @retval 1 if something has gone wrong
+# @retval 0 on success.
+#
+check_loglvl()
+{
+    case "$1" in
+        0|1|2|3|4|5|6)
+            return 0
+           ;;
+        *)
+            return 1
+            ;;
+        esac
+}
+
 # @brief Initializes Logger.
 # @retval 1 if something has gone wrong
 # @retval 0 on success.
 #
 dlog_init() {
     local ret=0; local errmsg
+
     [ -z "$kdump_stdloglvl" ] && kdump_stdloglvl=4
     [ -z "$kdump_sysloglvl" ] && kdump_sysloglvl=4
     [ -z "$kdump_kmsgloglvl" ] && kdump_kmsgloglvl=0
+
+    for loglvl in $kdump_stdloglvl $kdump_kmsgloglvl $kdump_sysloglvl; do
+        check_loglvl $loglvl
+        if [ $? -ne 0 ]; then
+            echo "Illegal log level: $kdump_stdloglvl $kdump_kmsgloglvl $kdump_sysloglvl"
+            return 1
+        fi
+    done
+
     # Skip initialization if it's already done.
     [ -n "$kdump_maxloglvl" ] && return 0

[Kairui Song]

On Mon, Nov 2, 2020 at 6:59 PM Lianbo Jiang  wrote:
> Let's add sanity checks for the log level in order to avoid
> passing illegal log level to the logger.
> Signed-off-by: Lianbo Jiang
>
> kdump-logger.sh | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/kdump-logger.sh b/kdump-logger.sh
> index c7afa340acfa..a3d674cd07c2 100755
> --- a/kdump-logger.sh
> +++ b/kdump-logger.sh
> @@ -34,15 +34,42 @@
>  # First of all you have to start with dlog_init() function which initializes
>  # required variables. Don't call any other logging function before that one!
>  #
>
> +# @brief Check the log level.
> +# @retval 1 if something has gone wrong
> +# @retval 0 on success.
> +#
> +check_loglvl()
> +{
> case "$1" in
>    0|1|2|3|4|5|6)
>        return 0
>       ;;
>    *)
>        return 1
>        ;;
>    esac
>
> +}
>
> # @brief Initializes Logger.
>  # @retval 1 if something has gone wrong
>  # @retval 0 on success.
>  #
>  dlog_init() {
>      local ret=0; local errmsg
> [ -z "$kdump_stdloglvl" ] && kdump_stdloglvl=4
>    [ -z "$kdump_sysloglvl" ] && kdump_sysloglvl=4
>    [ -z "$kdump_kmsgloglvl" ] && kdump_kmsgloglvl=0
> for loglvl in $kdump_stdloglvl $kdump_kmsgloglvl $kdump_sysloglvl; do

Hi Lianbo, this seems will fail the test case like kdump_stdloglvl="3 4"
Should be:

for loglvl in "$kdump_stdloglvl" "$kdump_kmsgloglvl" "$kdump_sysloglvl"

I can fix this when merging the patch if you are OK with this.
>    check_loglvl $loglvl

Should be:
check_loglvl "$loglvl"
>    if [ $? -ne 0 ]; then
>        echo "Illegal log level: $kdump_stdloglvl $kdump_kmsgloglvl $kdump_sysloglvl"
>        return 1
>    fi
> done
> # Skip initialization if it's already done.
>    [ -n "$kdump_maxloglvl" ] && return 0
>
> --
> 2.17.1

[lijiang]

在 2020年11月12日 14:41, Kairui Song 写道:
> On Mon, Nov 2, 2020 at 6:59 PM Lianbo Jiang  wrote:
> > Let's add sanity checks for the log level in order to avoid
> > passing illegal log level to the logger.
> > Signed-off-by: Lianbo Jiang
> >
> > kdump-logger.sh | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git a/kdump-logger.sh b/kdump-logger.sh
> > index c7afa340acfa..a3d674cd07c2 100755
> > --- a/kdump-logger.sh
> > +++ b/kdump-logger.sh
> > @@ -34,15 +34,42 @@
> >  # First of all you have to start with dlog_init() function which initializes
> >  # required variables. Don't call any other logging function before that one!
> >  #
> >
> > +# @brief Check the log level.
> > +# @retval 1 if something has gone wrong
> > +# @retval 0 on success.
> > +#
> > +check_loglvl()
> > +{
> > case "$1" in
> >    0|1|2|3|4|5|6)
> >        return 0
> >       ;;
> >    *)
> >        return 1
> >        ;;
> >    esac
> >
> > +}
> >
> > # @brief Initializes Logger.
> >  # @retval 1 if something has gone wrong
> >  # @retval 0 on success.
> >  #
> >  dlog_init() {
> >      local ret=0; local errmsg
> > [ -z "$kdump_stdloglvl" ] && kdump_stdloglvl=4
> >    [ -z "$kdump_sysloglvl" ] && kdump_sysloglvl=4
> >    [ -z "$kdump_kmsgloglvl" ] && kdump_kmsgloglvl=0
> > for loglvl in $kdump_stdloglvl $kdump_kmsgloglvl $kdump_sysloglvl; do
>
> Hi Lianbo, this seems will fail the test case like kdump_stdloglvl="3 4"
> Should be:

Hmm, I noticed that previously no one checked the parameter, and used it directly
in code, which depends on the legal parameter values, seems not very good.

This should be a good improvement. I will update it and post it later,

Thanks.
Lianbo
> for loglvl in "$kdump_stdloglvl" "$kdump_kmsgloglvl" "$kdump_sysloglvl"
>
> I can fix this when merging the patch if you are OK with this.
> >    check_loglvl $loglvl
>
> Should be:
> check_loglvl "$loglvl"
> >    if [ $? -ne 0 ]; then
> >        echo "Illegal log level: $kdump_stdloglvl $kdump_kmsgloglvl $kdump_sysloglvl"
> >        return 1
> >    fi
> > done
> > # Skip initialization if it's already done.
> >    [ -n "$kdump_maxloglvl" ] && return 0
> >
> > --
> > 2.17.1