[PATCH] kdumpctl: Error out in case there are white spaces before an option name

[Bhupesh Sharma]

Resolves: BZ1484945
https://bugzilla.redhat.com/show_bug.cgi?id=1484945

Currently the kdumpctl script doesn't handle
whitespaces (including TABs) which might be there before
an option name in the kdump.conf

This patch addresses this issue, by ensuring that the
kdumpctl errors out in case it finds any stray whitespaces
before a option name.

While at it, we also add a relevant note in the
kdump.conf man page to explain this error case.

Reported-by: Kenneth Dsouzak 
Signed-off-by: Bhupesh Sharma 
---
 kdump.conf.5 | 4 ++++
 kdumpctl     | 9 ++++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/kdump.conf.5 b/kdump.conf.5
index 11b1fad559b4..a68e6e8f426a 100644
--- a/kdump.conf.5
+++ b/kdump.conf.5
@@ -339,6 +339,10 @@ scp /proc/vmcore :path/vmcore
 examples for other options please see
 .I /etc/kdump.conf

+.PP
+.B
+NOTE: No whitespaces are allowed before a kdump OPTION name.
+
 .SH SEE ALSO

kexec(8) mkdumprd(8) dracut.cmdline(7)
diff --git a/kdumpctl b/kdumpctl
index b7a3105026eb..b4340840453b 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -406,6 +406,13 @@ check_config()
    	return 1
    }

+	# Check if we have any leading spaces before the 
+        # variable name in the kdump conf file  
+        if grep '^[[:space:]]' $KDUMP_CONFIG_FILE; then
+                echo "No whitespaces are allowed before a kdump OPTION name. Refer to kdump.conf manpage for details"
+                return 1
+        fi
+	
    while read config_opt config_val; do
    	case "$config_opt" in
    	#* | "")
@@ -432,7 +439,7 @@ check_config()
    check_default_config || return 1

check_fence_kdump_config || return 1
-
+	
    return 0
 }

[Dave Young]

Hi,

On 08/24/17 at 09:09pm, Bhupesh Sharma wrote:
> Resolves: BZ1484945
> https://bugzilla.redhat.com/show_bug.cgi?id=1484945
>
> Currently the kdumpctl script doesn't handle
> whitespaces (including TABs) which might be there before
> an option name in the kdump.conf
>
> This patch addresses this issue, by ensuring that the
> kdumpctl errors out in case it finds any stray whitespaces
> before a option name.
>
> While at it, we also add a relevant note in the
> kdump.conf man page to explain this error case.
>
> Reported-by: Kenneth Dsouzak 
> Signed-off-by: Bhupesh Sharma
>
> kdump.conf.5 | 4 ++++
>  kdumpctl     | 9 ++++++++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/kdump.conf.5 b/kdump.conf.5
> index 11b1fad559b4..a68e6e8f426a 100644
> --- a/kdump.conf.5
> +++ b/kdump.conf.5
> @@ -339,6 +339,10 @@ scp /proc/vmcore :path/vmcore
>  examples for other options please see
>  .I /etc/kdump.conf
>
> +.PP
> +.B
> +NOTE: No whitespaces are allowed before a kdump OPTION name.
>
> .SH SEE ALSO
>
> kexec(8) mkdumprd(8) dracut.cmdline(7)
> diff --git a/kdumpctl b/kdumpctl
> index b7a3105026eb..b4340840453b 100755
> --- a/kdumpctl
> +++ b/kdumpctl
> @@ -406,6 +406,13 @@ check_config()
>    	return 1
>    }
> # Check if we have any leading spaces before the
>    # variable name in the kdump conf file  
>    if grep '^[[:space:]]' $KDUMP_CONFIG_FILE; then

We should allow white space if it is not a kdump option at all like those in a
blank line or before a comment line.
>            echo "No whitespaces are allowed before a kdump OPTION name. Refer to kdump.conf manpage for details"

use lower case for "option" please. In kdump.conf manpage it is same one
sentence description, no need to let user to refer to manpage anymore..
>            return 1
>    fi
> while read config_opt config_val; do
> case "$config_opt" in
> #* | "")
>
> @@ -432,7 +439,7 @@ check_config()
>    check_default_config || return 1
>
> check_fence_kdump_config || return 1
> return 0
>
> }
>
> -- 
> 2.7.4

Thanks
Dave

[Dave Young]

On 08/24/17 at 09:09pm, Bhupesh Sharma wrote:
> Resolves: BZ1484945
> https://bugzilla.redhat.com/show_bug.cgi?id=1484945
>
> Currently the kdumpctl script doesn't handle
> whitespaces (including TABs) which might be there before
> an option name in the kdump.conf
>
> This patch addresses this issue, by ensuring that the
> kdumpctl errors out in case it finds any stray whitespaces
> before a option name.
>
> While at it, we also add a relevant note in the
> kdump.conf man page to explain this error case.
>
> Reported-by: Kenneth Dsouzak 
> Signed-off-by: Bhupesh Sharma
>
> kdump.conf.5 | 4 ++++
>  kdumpctl     | 9 ++++++++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/kdump.conf.5 b/kdump.conf.5
> index 11b1fad559b4..a68e6e8f426a 100644
> --- a/kdump.conf.5
> +++ b/kdump.conf.5
> @@ -339,6 +339,10 @@ scp /proc/vmcore :path/vmcore
>  examples for other options please see
>  .I /etc/kdump.conf
>
> +.PP
> +.B
> +NOTE: No whitespaces are allowed before a kdump OPTION name.
>
> .SH SEE ALSO
>
> kexec(8) mkdumprd(8) dracut.cmdline(7)
> diff --git a/kdumpctl b/kdumpctl
> index b7a3105026eb..b4340840453b 100755
> --- a/kdumpctl
> +++ b/kdumpctl
> @@ -406,6 +406,13 @@ check_config()
>    	return 1
>    }
> # Check if we have any leading spaces before the
>    # variable name in the kdump conf file  
>    if grep '^[[:space:]]' $KDUMP_CONFIG_FILE; then

We only care about space and tab, so [[:blank:]] should be better here.
>            echo "No whitespaces are allowed before a kdump OPTION name. Refer to kdump.conf manpage for details"
>            return 1
>    fi
> while read config_opt config_val; do
> case "$config_opt" in
> #* | "")
>
> @@ -432,7 +439,7 @@ check_config()
>    check_default_config || return 1
>
> check_fence_kdump_config || return 1
> return 0
>
> }
>
> -- 
> 2.7.4

[Bhupesh Sharma]

Hi Dave,

Thanks for the review,

On Fri, Aug 25, 2017 at 12:23 PM, Dave Young  wrote:
> On 08/24/17 at 09:09pm, Bhupesh Sharma wrote:
> > Resolves: BZ1484945
> > https://bugzilla.redhat.com/show_bug.cgi?id=1484945
> >
> > Currently the kdumpctl script doesn't handle
> > whitespaces (including TABs) which might be there before
> > an option name in the kdump.conf
> >
> > This patch addresses this issue, by ensuring that the
> > kdumpctl errors out in case it finds any stray whitespaces
> > before a option name.
> >
> > While at it, we also add a relevant note in the
> > kdump.conf man page to explain this error case.
> >
> > Reported-by: Kenneth Dsouzak 
> > Signed-off-by: Bhupesh Sharma
> >
> > kdump.conf.5 | 4 ++++
> >  kdumpctl     | 9 ++++++++-
> >  2 files changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/kdump.conf.5 b/kdump.conf.5
> > index 11b1fad559b4..a68e6e8f426a 100644
> > --- a/kdump.conf.5
> > +++ b/kdump.conf.5
> > @@ -339,6 +339,10 @@ scp /proc/vmcore :path/vmcore
> >  examples for other options please see
> >  .I /etc/kdump.conf
> >
> > +.PP
> > +.B
> > +NOTE: No whitespaces are allowed before a kdump OPTION name.
> >
> > .SH SEE ALSO
> >
> > kexec(8) mkdumprd(8) dracut.cmdline(7)
> > diff --git a/kdumpctl b/kdumpctl
> > index b7a3105026eb..b4340840453b 100755
> > --- a/kdumpctl
> > +++ b/kdumpctl
> > @@ -406,6 +406,13 @@ check_config()
> >               return 1
> >       }
> > # Check if we have any leading spaces before the
> >    # variable name in the kdump conf file
> >    if grep '^[[:space:]]' $KDUMP_CONFIG_FILE; then
>
> We only care about space and tab, so [[:blank:]] should be better here.

Yes. I agree. Initially I had thought of including all white-space
characters, but may be we just need to cater to spaces and horizontal
tabs.

Regards,
Bhupesh
> >            echo "No whitespaces are allowed before a kdump OPTION name. Refer to kdump.conf manpage for details"
> >            return 1
> >    fi
> > while read config_opt config_val; do
> >         case "$config_opt" in
> >         \#* | "")
> >
> > @@ -432,7 +439,7 @@ check_config()
> >       check_default_config || return 1
> >   check_fence_kdump_config || return 1
> > return 0
> >
> > }
> >
> > --
> > 2.7.4