Bug 1368728

I was able to replicate the issue and have resolved it with a patch that I will attach momentarily.

The issue was that having a configuration item without a value assigned creates a vector, but the token in slot 1 is not initialized/populated and therefore points to NULL.

The existing vector_ensure() function performs a check and, if the token isn't initialized, it is created and set to NULL. 

So, I call vector_ensure() and then verify that vector_slot() return a non-NULL value before using vector_slot().

In addition to resolving this issue with smtp_server, I found that the same problem is present with notification_email_from and smtp_connect_timeout, so the patch addresses these two, also.

-Rich Alloway (RogueWave)

Created attachment 1212600 [details]
Proposed patch for keepalived-1.2.13-7

(In reply to Richard D Alloway from comment #3)
> Created attachment 1212600 [details]
> Proposed patch for keepalived-1.2.13-7

(In reply to Richard D Alloway from comment #2)
> I was able to replicate the issue and have resolved it with a patch that I
> will attach momentarily.
> 
> The issue was that having a configuration item without a value assigned
> creates a vector, but the token in slot 1 is not initialized/populated and
> therefore points to NULL.
> 
> The existing vector_ensure() function performs a check and, if the token
> isn't initialized, it is created and set to NULL. 
> 
> So, I call vector_ensure() and then verify that vector_slot() return a
> non-NULL value before using vector_slot().
> 
> In addition to resolving this issue with smtp_server, I found that the same
> problem is present with notification_email_from and smtp_connect_timeout, so
> the patch addresses these two, also.
> 
> -Rich Alloway (RogueWave)

The problem exists for any config file keyword that expects a value where none is set. The parser does not detect this as it should. I've discussed this with upstream and I believe we have a simple patch that will address this problem, such that an error will be printed and keepalived will exit. That said, I will take a look at this patch.

(In reply to Richard D Alloway from comment #3)
> Created attachment 1212600 [details]
> Proposed patch for keepalived-1.2.13-7

This is only a diff of the spec file.

Hi Ryan!

There is a new file created as SOURCES/bz1368728-keepalived-config-segfault.patch towards the top of the patch file.

The spec file diff is below the patch.

Let me know if you can't see it and I'll upload the file again.

Thanks!

-Rich

(In reply to Richard D Alloway from comment #6)
> Hi Ryan!
> 
> There is a new file created as
> SOURCES/bz1368728-keepalived-config-segfault.patch towards the top of the
> patch file.
> 
> The spec file diff is below the patch.
> 
> Let me know if you can't see it and I'll upload the file again.
> 
> Thanks!
> 
> -Rich

OK. I see it now. For some reason if I click the diff link in the above comment I only see the spec file diff. The patch looks fine, but I would much prefer we fix the bigger issue with all keyword/value pairs. Between upstream and myself, if that doesn't happen soon I will use this patch as a temporary fix in RHEL. Thanks.

I tested this today and it is fixed in keepalived-1.3.5, which will released with RHEL7.4.

In the "global_defs" section of /etc/keepalived/keepalived.conf, simple add "smtp_server" without specifying the required name/address. Start keepalived, there will be no segfault. Instead, keepalived shoud fail to start and the following messages should appear in the logs:

*** Configuration line starting `smtp_server` is missing a parameter after keyword `smtp_server` at word position 2

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2017:2169