Bug 1476219 - Handle white spaces before variables in '/etc/kdump.conf' - add appropriate error handling and documentation
Handle white spaces before variables in '/etc/kdump.conf' - add appropriate e...
Status: ON_QA
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: kexec-tools (Show other bugs)
7.3
All Linux
low Severity low
: rc
: ---
Assigned To: Bhupesh Sharma
Ziqian SUN (Zamir)
Mark Flitter
: Reopened
Depends On:
Blocks: 1420851 1469549 1473055
  Show dependency treegraph
 
Reported: 2017-07-28 06:29 EDT by Kenneth D'souza
Modified: 2017-11-14 03:11 EST (History)
5 users (show)

See Also:
Fixed In Version: kexec-tools-2.0.15-8.el7
Doc Type: Release Note
Doc Text:
Story Points: ---
Clone Of:
: 1484945 (view as bug list)
Environment:
Last Closed: 2017-08-23 01:55:21 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
kdumpctl file with spacefunction to detect white space. (26.71 KB, application/x-shellscript)
2017-07-28 06:35 EDT, Kenneth D'souza
no flags Details

  None (edit)
Description Kenneth D'souza 2017-07-28 06:29:38 EDT
Description of problem:

When a white space is introduced before the path parameter.The vmocre is dumped on the default location /var/crash and not the path that was given.

The path is ignored and the service still restarts without any errors.

Version-Release number of selected component (if applicable):

On all RHEL 7 versions.

How reproducible:

Steps to Reproduce:

  mkdir /tmp/crash

1.Edit /etc/kdump.conf

     path /tmp/crash --------------->> Introduce space before the path.

2. systemctl restart kdump 

I analysed the mkdumprd and kdumpctl code, they would ignore the path and the default /var/kdump path would be set as there is no code to remove spaces.

3.Crash the kernel and check that the dump is stored in /var/kdump and not /tmp/kdump.


Expected results:

In RHEL 6 even if white space were mentioned the vmcore would be dumped at the path given and not default path.

This does not work in RHEL 7.

We can either say that there should be no space before the path by failing the service.


Or

We will have to change the code and remove the spaces in /etc/kdump.conf file.

The code of sed that would delete the spaces in RHEL 6 is not in RHEL 7.

sed 's/^\s\+//' ------------->> To remove the spaces.

The above change must be done in all code files of mkdumprd and kdumpctl.

The save_path and other code will have to be passed

Correct me if I am wrong.



===============================================================================

We can do the below and fail the kdump service by saying no white spaces are allowed before the config parameters. That would be an easy fix.

I changed the below code in /usr/bin/kdumpctl

#Performs a check on white spaces in a file.

space()
{
sed 's/^\s\+//' /etc/kdump.conf > /tmp/space.conf #This command will remove white spaces if there are any
exclude_space="/tmp/space.conf"
if cmp -s "$KDUMP_CONFIG_FILE" "$exclude_space"  
then
 return 0
else
    echo "There is space before config options"
   echo "The /etc/kdump.conf file contains space, please check if there are any white spaces before config options"
  return 1
      fi
}




Call the space function at sanity checking of invalid parameters in configuraion file. 


Call space in  ------------->> Line number 291

 space  || return 1 




======================================================================

Snippet /sbin/mkdumprd 


sed 's/^\s\+//' ------>> This is not impemented before grep ^path


SAVE_PATH=$(grep ^path $conf_file| cut -d' '  -f2)
[ -z "$SAVE_PATH" ] && SAVE_PATH=$DEFAULT_PATH
# strip the duplicated "/"
SAVE_PATH=$(echo $SAVE_PATH | tr -s /)


=====================================================================

The same is not implemented in Snippet /usr/bin/kdumpctl

get_save_path()
	local _save_path=$(grep "^path" /etc/kdump.conf|awk '{print $2}')
	if [ -z "$_save_path" ]; then
		_save_path="/var/crash"
	echo $_save_path
	_path=$(get_save_path)
	save_raw

====================================================================








Additional info:
Comment 2 Kenneth D'souza 2017-07-28 06:35 EDT
Created attachment 1305890 [details]
kdumpctl file with spacefunction to detect white space.
Comment 3 Kenneth D'souza 2017-07-28 09:25:52 EDT
sorry for the typo:

3.Crash the kernel and check that the dump is stored in /var/crash and not /tmp/crash.
Comment 5 Bhupesh Sharma 2017-08-21 01:56:55 EDT
This is reproducible with RHEL 7.4 kexec-tools:
# rpm -qa kexec-tools
kexec-tools-2.0.0-307.el6.x86_64


Initially it took me some time to reproduce this and I had to confirm this with the reporter as when I used a location inside '/tmp' the issue was NOT reproducible:
   path /tmp/crash  <--- space before path

however, when I used a crash location outside /tmp, I was able to reproduce this issue, for e.g. on beaker machine ibm-x3650m4-01-vm-06.lab.eng.bos.redhat.com with path variable set as:
   path /home/bhsharma  <--- space before path

Will add appropriate handling after observing the default behaviour on other utilities like dracut.
Comment 6 Dave Young 2017-08-23 01:05:17 EDT
To make clear about the problem, I have some questions about the bug:

1. where is the space? for example "path /tmp/crash", do you mean " patch /tmp/crash" or "path   /tmp/crash"?

2. where is the RHEL6 code to strip the space? I do not find it.


OTOH, in rhel6 code we have below code:
local _save_path=$(grep "^path" /etc/kdump.conf|awk '{print $2}')

So it is assumed that path should not prefixed with extra spaces.

Thanks
Dave
Comment 7 Dave Young 2017-08-23 01:05:46 EDT
> 1. where is the space? for example "path /tmp/crash", do you mean " patch
s/patch/path
Comment 8 Bhupesh Sharma 2017-08-23 01:25:58 EDT
(In reply to Dave Young from comment #7)
> > 1. where is the space? for example "path /tmp/crash", do you mean " patch
> s/patch/path

As I captured in Comment 5, the issue is seen when we have space before path.
Comment 9 Kenneth D'souza 2017-08-23 01:33:05 EDT

    1> space is before the path:

    Example:

    <whitespace>    path /home/crash

    2> RHEL 6 code to deal with white spaces:

    [root@localhost Desktop]# uname -r
    2.6.32-696.el6.x86_64
    [root@localhost Desktop]# rpm -qa | grep kexec
    kexec-tools-2.0.0-307.el6.x86_64
    [root@localhost Desktop]#

    The code is in /sbin/mkdumprd 

    Code Snippet
    ============================================================================
      # deal with indentation
        space_at_front=`echo "$msg" | grep -o "^[[:space:]]\+"`
        msg=`echo "$msg" | sed 's/^\s\+//'`

        msg_printed=0
        while [ $# -gt 0 ]; do
            trace_level=`echo "$1" | grep -o '^[0-9]\+'`
            trace_in_higher_levels=`echo "$1" | grep -o '+'`
            trace=`echo $1 | sed "s/^.*://"`

            if [ -z "$trace_level" ]; then
                trace_level=0
            fi

            insert_trace=0
            if [ -n "$trace_in_higher_levels" ]; then
                if [ "$log_level" -ge "$trace_level" ]; then
                    insert_trace=1
                fi
            else
                if [ "$log_level" -eq "$trace_level" ]; then
                    insert_trace=1
                fi
            fi

            if [ $insert_trace -eq 1 ]; then
                if [ $msg_printed -eq 0 ]; then
                    emit "${space_at_front}echo \"$prefix $msg\""
                    msg_printed=1
                fi
                emit "${space_at_front}$func $trace"
            fi
            shift
        done
    }

    ===========================================================================
      

Thanks 
Kenneth D'souza
Comment 10 Dave Young 2017-08-23 01:54:46 EDT
(In reply to Kenneth D'souza from comment #9)
> 
>     1> space is before the path:
> 
>     Example:
> 
>     <whitespace>    path /home/crash
> 
>     2> RHEL 6 code to deal with white spaces:
> 
>     [root@localhost Desktop]# uname -r
>     2.6.32-696.el6.x86_64
>     [root@localhost Desktop]# rpm -qa | grep kexec
>     kexec-tools-2.0.0-307.el6.x86_64
>     [root@localhost Desktop]#
> 
>     The code is in /sbin/mkdumprd 
> 
>     Code Snippet
>    
> ============================================================================
>       # deal with indentation
>         space_at_front=`echo "$msg" | grep -o "^[[:space:]]\+"`
>         msg=`echo "$msg" | sed 's/^\s\+//'`
> 
>         msg_printed=0
>         while [ $# -gt 0 ]; do
>             trace_level=`echo "$1" | grep -o '^[0-9]\+'`
>             trace_in_higher_levels=`echo "$1" | grep -o '+'`
>             trace=`echo $1 | sed "s/^.*://"`
> 
>             if [ -z "$trace_level" ]; then
>                 trace_level=0
>             fi
> 
>             insert_trace=0
>             if [ -n "$trace_in_higher_levels" ]; then
>                 if [ "$log_level" -ge "$trace_level" ]; then
>                     insert_trace=1
>                 fi
>             else
>                 if [ "$log_level" -eq "$trace_level" ]; then
>                     insert_trace=1
>                 fi
>             fi
> 
>             if [ $insert_trace -eq 1 ]; then
>                 if [ $msg_printed -eq 0 ]; then
>                     emit "${space_at_front}echo \"$prefix $msg\""
>                     msg_printed=1
>                 fi
>                 emit "${space_at_front}$func $trace"
>             fi
>             shift
>         done
>     }
> 

Thanks for the quick reply.

But above code is not for kdump.conf handling, it is for memory trace related. If so it is clear that it happens to work in RHEL6, but actually we do not support it, as I mentioned in last comment 6, RHEL6 also need "path" without extra spaces from the code.

Thus we'd better to enforce any options in kdump.conf should not start with spaces and close this bug.
Comment 11 Bhupesh Sharma 2017-08-23 02:28:21 EDT
Reopening this issue.

As discussed with Dave Young on irc, we need to add proper error handling in kdumpctl and also document this properly, as any user can add a space before a variable in kdump.conf and this should be handled properly.
Comment 12 Kenneth D'souza 2017-08-23 02:36:50 EDT
This makes us go with the option 
 
b> There should be no space before the path by failing the service.

Note You need to log in before you can comment on or make changes to this bug.