set_processing_status() may crash + strerror_r print garbage

Description

While working with bro in a cluster I came across a crash (segfault) when bro is starting up. I found this happens when bro is run with the -U option (for instance -U .status) and an error occurs in the _set_processing_status() function. In my case the error was permission denied when opening the status file. The problem is that the error is reported with the reporter object but that object isn't created yet. Additionally when bro is shutting down the reporter object is deleted before the last call to set_processing_status. It is of course more unlikely that writing to the status file fails now but it can happen.

I have attached a patch which moves the creation of reporter up before calling set_processing_status setting the status to INITIALIZING and checking if the reporter object exist before calling it to handle the deletion of reporter when bro shuts down. Alternatively reporter should be deleted later (if at all) so it is guaranteed to always exist.

While working with this I also noticed that the printed error messages always contained random garbage. It appears that everywhere strerror_r is called it is assumed that it is the XSI-compliant version. On all machines where I compile bro I always get the GNU version which return the error message rather than putting it in the supplied buffer.

In the supplied patch I have stolen the strerror_r wrapper from aux/broker and put it (with small modifications) in util.cc. Then changed all calls to use the wrapper.

The two patches might overlap a bit in the _set_processing_status function in util.cc so please apply the processing_status patch before the strerror_r one.

Environment

linux

Assignee

Johanna Amann

Reporter

Thomas Petersen

Labels

None

External issue ID

None

Components

Affects versions

Priority

Normal
Configure