Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Downtime::AddDowntime(): NULL-check pointer before deref not to crash #10127

Merged
merged 4 commits into from
Sep 18, 2024
Merged

Downtime::AddDowntime(): NULL-check pointer before deref not to crash #10127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5a15534
[Refactor] l_LegacyDowntimesCache: store Downtime objects, not just t…
Al2Klimov Apr 24, 2024
0bba441
[Refactor] Downtime::GetDowntimeIDFromLegacyID(): return the Downtime…
Al2Klimov Apr 24, 2024
d09d051
/v1/actions/schedule-downtime: reject request on invalid trigger_name
Al2Klimov Apr 24, 2024
7761d6f
l_LegacyDowntimesCache: delete removed objects not to leak memory
Al2Klimov Apr 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions lib/icinga/apiactions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,9 +354,16 @@ Dictionary::Ptr ApiActions::ScheduleDowntime(const ConfigObject::Ptr& object,
if (params->Contains("duration"))
duration = HttpUtility::GetLastParameter(params, "duration");

String triggerName;
if (params->Contains("trigger_name"))
triggerName = HttpUtility::GetLastParameter(params, "trigger_name");
Downtime::Ptr trigger;
String triggerName = HttpUtility::GetLastParameter(params, "trigger_name");

if (!triggerName.IsEmpty()) {
trigger = Downtime::GetByName(triggerName);

if (!trigger) {
return ApiActions::CreateResult(404, "Won't schedule downtime with non-existent trigger downtime.");
}
}

String author = HttpUtility::GetLastParameter(params, "author");
String comment = HttpUtility::GetLastParameter(params, "comment");
Expand All @@ -377,7 +384,7 @@ Dictionary::Ptr ApiActions::ScheduleDowntime(const ConfigObject::Ptr& object,
}

Downtime::Ptr downtime = Downtime::AddDowntime(checkable, author, comment, startTime, endTime,
fixed, triggerName, duration);
fixed, trigger, duration);
String downtimeName = downtime->GetName();

Dictionary::Ptr additional = new Dictionary({
Expand All @@ -399,7 +406,7 @@ Dictionary::Ptr ApiActions::ScheduleDowntime(const ConfigObject::Ptr& object,
<< "Creating downtime for service " << hostService->GetName() << " on host " << host->GetName();

Downtime::Ptr serviceDowntime = Downtime::AddDowntime(hostService, author, comment, startTime, endTime,
fixed, triggerName, duration, String(), String(), downtimeName);
fixed, trigger, duration, String(), String(), downtimeName);
String serviceDowntimeName = serviceDowntime->GetName();

serviceDowntimes.push_back(new Dictionary({
Expand All @@ -416,8 +423,9 @@ Dictionary::Ptr ApiActions::ScheduleDowntime(const ConfigObject::Ptr& object,
/* 'DowntimeTriggeredChildren' schedules child downtimes triggered by the parent downtime.
* 'DowntimeNonTriggeredChildren' schedules non-triggered downtimes for all children.
*/
if (childOptions == DowntimeTriggeredChildren)
triggerName = downtimeName;
if (childOptions == DowntimeTriggeredChildren) {
trigger = downtime;
}

Log(LogNotice, "ApiActions")
<< "Processing child options " << childOptions << " for downtime " << downtimeName;
Expand All @@ -443,7 +451,7 @@ Dictionary::Ptr ApiActions::ScheduleDowntime(const ConfigObject::Ptr& object,
<< "Scheduling downtime for child object " << child->GetName();

Downtime::Ptr childDowntime = Downtime::AddDowntime(child, author, comment, startTime, endTime,
fixed, triggerName, duration);
fixed, trigger, duration);
String childDowntimeName = childDowntime->GetName();

Log(LogNotice, "ApiActions")
Expand All @@ -463,7 +471,7 @@ Dictionary::Ptr ApiActions::ScheduleDowntime(const ConfigObject::Ptr& object,
<< "Creating downtime for service " << childService->GetName() << " on child host " << childHost->GetName();

Downtime::Ptr serviceDowntime = Downtime::AddDowntime(childService, author, comment, startTime, endTime,
fixed, triggerName, duration, String(), String(), childDowntimeName);
fixed, trigger, duration, String(), String(), childDowntimeName);
String serviceDowntimeName = serviceDowntime->GetName();

childServiceDowntimes.push_back(new Dictionary({
Expand Down
27 changes: 19 additions & 8 deletions lib/icinga/downtime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ using namespace icinga;

static int l_NextDowntimeID = 1;
static std::mutex l_DowntimeMutex;
static std::map<int, String> l_LegacyDowntimesCache;
static std::map<int, Downtime::Ptr> l_LegacyDowntimesCache;
static Timer::Ptr l_DowntimesExpireTimer;
static Timer::Ptr l_DowntimesStartTimer;

Expand Down Expand Up @@ -108,7 +108,7 @@ void Downtime::Start(bool runtimeCreated)
std::unique_lock<std::mutex> lock(l_DowntimeMutex);

SetLegacyId(l_NextDowntimeID);
l_LegacyDowntimesCache[l_NextDowntimeID] = GetName();
l_LegacyDowntimesCache[l_NextDowntimeID] = this;
l_NextDowntimeID++;
}

Expand Down Expand Up @@ -147,6 +147,12 @@ void Downtime::Start(bool runtimeCreated)

void Downtime::Stop(bool runtimeRemoved)
{
{
std::unique_lock<std::mutex> lock (l_DowntimeMutex);

l_LegacyDowntimesCache.erase(GetLegacyId());
}

GetCheckable()->UnregisterDowntime(this);

Downtime::Ptr parent = GetByName(GetParent());
Expand Down Expand Up @@ -229,17 +235,22 @@ int Downtime::GetNextDowntimeID()

Downtime::Ptr Downtime::AddDowntime(const Checkable::Ptr& checkable, const String& author,
const String& comment, double startTime, double endTime, bool fixed,
const String& triggeredBy, double duration,
const Downtime::Ptr& parentDowntime, double duration,
const String& scheduledDowntime, const String& scheduledBy, const String& parent,
const String& id, const MessageOrigin::Ptr& origin)
{
String fullName;
String triggeredBy;

if (id.IsEmpty())
fullName = checkable->GetName() + "!" + Utility::NewUniqueID();
else
fullName = id;

if (parentDowntime) {
triggeredBy = parentDowntime->GetName();
}

Dictionary::Ptr attrs = new Dictionary();

attrs->Set("author", author);
Expand Down Expand Up @@ -310,8 +321,7 @@ Downtime::Ptr Downtime::AddDowntime(const Checkable::Ptr& checkable, const Strin
BOOST_THROW_EXCEPTION(std::runtime_error("Could not create downtime."));
}

if (!triggeredBy.IsEmpty()) {
Downtime::Ptr parentDowntime = Downtime::GetByName(triggeredBy);
if (parentDowntime) {
Array::Ptr triggers = parentDowntime->GetTriggers();

ObjectLock olock(triggers);
Expand Down Expand Up @@ -470,14 +480,15 @@ void Downtime::SetRemovalInfo(const String& removedBy, double removeTime, const
OnRemovalInfoChanged(this, removedBy, removeTime, origin);
}

String Downtime::GetDowntimeIDFromLegacyID(int id)
Downtime::Ptr Downtime::GetDowntimeFromLegacyID(int id)
{
std::unique_lock<std::mutex> lock(l_DowntimeMutex);

auto it = l_LegacyDowntimesCache.find(id);

if (it == l_LegacyDowntimesCache.end())
return Empty;
if (it == l_LegacyDowntimesCache.end()) {
return nullptr;
}

return it->second;
}
Expand Down
4 changes: 2 additions & 2 deletions lib/icinga/downtime.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class Downtime final : public ObjectImpl<Downtime>

static Ptr AddDowntime(const intrusive_ptr<Checkable>& checkable, const String& author,
const String& comment, double startTime, double endTime, bool fixed,
const String& triggeredBy, double duration, const String& scheduledDowntime = String(),
const Ptr& parentDowntime, double duration, const String& scheduledDowntime = String(),
const String& scheduledBy = String(), const String& parent = String(), const String& id = String(),
const MessageOrigin::Ptr& origin = nullptr);

Expand All @@ -64,7 +64,7 @@ class Downtime final : public ObjectImpl<Downtime>

void OnAllConfigLoaded() override;

static String GetDowntimeIDFromLegacyID(int id);
static Downtime::Ptr GetDowntimeFromLegacyID(int id);

static DowntimeChildOptions ChildOptionsFromValue(const Value& options);

Expand Down
88 changes: 58 additions & 30 deletions lib/icinga/externalcommandprocessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -967,11 +967,13 @@ void ExternalCommandProcessor::ScheduleSvcDowntime(double, const std::vector<Str
if (!service)
BOOST_THROW_EXCEPTION(std::invalid_argument("Cannot schedule service downtime for non-existent service '" + arguments[1] + "' on host '" + arguments[0] + "'"));

String triggeredBy;
Downtime::Ptr triggeredBy;
int triggeredByLegacy = Convert::ToLong(arguments[5]);
int is_fixed = Convert::ToLong(arguments[4]);
if (triggeredByLegacy != 0)
triggeredBy = Downtime::GetDowntimeIDFromLegacyID(triggeredByLegacy);

if (triggeredByLegacy != 0) {
triggeredBy = Downtime::GetDowntimeFromLegacyID(triggeredByLegacy);
}

Log(LogNotice, "ExternalCommandProcessor")
<< "Creating downtime for service " << service->GetName();
Expand All @@ -983,7 +985,12 @@ void ExternalCommandProcessor::ScheduleSvcDowntime(double, const std::vector<Str
void ExternalCommandProcessor::DelSvcDowntime(double, const std::vector<String>& arguments)
{
int id = Convert::ToLong(arguments[0]);
String rid = Downtime::GetDowntimeIDFromLegacyID(id);
String rid;
auto dt (Downtime::GetDowntimeFromLegacyID(id));

if (dt) {
rid = dt->GetName();
}

try {
Downtime::RemoveDowntime(rid, false, true);
Expand All @@ -1002,11 +1009,13 @@ void ExternalCommandProcessor::ScheduleHostDowntime(double, const std::vector<St
if (!host)
BOOST_THROW_EXCEPTION(std::invalid_argument("Cannot schedule host downtime for non-existent host '" + arguments[0] + "'"));

String triggeredBy;
Downtime::Ptr triggeredBy;
int triggeredByLegacy = Convert::ToLong(arguments[4]);
int is_fixed = Convert::ToLong(arguments[3]);
if (triggeredByLegacy != 0)
triggeredBy = Downtime::GetDowntimeIDFromLegacyID(triggeredByLegacy);

if (triggeredByLegacy != 0) {
triggeredBy = Downtime::GetDowntimeFromLegacyID(triggeredByLegacy);
}

Log(LogNotice, "ExternalCommandProcessor")
<< "Creating downtime for host " << host->GetName();
Expand All @@ -1023,11 +1032,13 @@ void ExternalCommandProcessor::ScheduleAndPropagateHostDowntime(double, const st
if (!host)
BOOST_THROW_EXCEPTION(std::invalid_argument("Cannot schedule and propagate host downtime for non-existent host '" + arguments[0] + "'"));

String triggeredBy;
Downtime::Ptr triggeredBy;
int triggeredByLegacy = Convert::ToLong(arguments[4]);
int is_fixed = Convert::ToLong(arguments[3]);
if (triggeredByLegacy != 0)
triggeredBy = Downtime::GetDowntimeIDFromLegacyID(triggeredByLegacy);

if (triggeredByLegacy != 0) {
triggeredBy = Downtime::GetDowntimeFromLegacyID(triggeredByLegacy);
}

Log(LogNotice, "ExternalCommandProcessor")
<< "Creating downtime for host " << host->GetName();
Expand Down Expand Up @@ -1059,11 +1070,13 @@ void ExternalCommandProcessor::ScheduleAndPropagateTriggeredHostDowntime(double,
if (!host)
BOOST_THROW_EXCEPTION(std::invalid_argument("Cannot schedule and propagate triggered host downtime for non-existent host '" + arguments[0] + "'"));

String triggeredBy;
Downtime::Ptr triggeredBy;
int triggeredByLegacy = Convert::ToLong(arguments[4]);
int is_fixed = Convert::ToLong(arguments[3]);
if (triggeredByLegacy != 0)
triggeredBy = Downtime::GetDowntimeIDFromLegacyID(triggeredByLegacy);

if (triggeredByLegacy != 0) {
triggeredBy = Downtime::GetDowntimeFromLegacyID(triggeredByLegacy);
}

Log(LogNotice, "ExternalCommandProcessor")
<< "Creating downtime for host " << host->GetName();
Expand All @@ -1084,14 +1097,19 @@ void ExternalCommandProcessor::ScheduleAndPropagateTriggeredHostDowntime(double,

(void) Downtime::AddDowntime(child, arguments[6], arguments[7],
Convert::ToDouble(arguments[1]), Convert::ToDouble(arguments[2]),
Convert::ToBool(is_fixed), parentDowntime->GetName(), Convert::ToDouble(arguments[5]));
Convert::ToBool(is_fixed), parentDowntime, Convert::ToDouble(arguments[5]));
}
}

void ExternalCommandProcessor::DelHostDowntime(double, const std::vector<String>& arguments)
{
int id = Convert::ToLong(arguments[0]);
String rid = Downtime::GetDowntimeIDFromLegacyID(id);
String rid;
auto dt (Downtime::GetDowntimeFromLegacyID(id));

if (dt) {
rid = dt->GetName();
}

try {
Downtime::RemoveDowntime(rid, false, true);
Expand Down Expand Up @@ -1169,11 +1187,13 @@ void ExternalCommandProcessor::ScheduleHostSvcDowntime(double, const std::vector
if (!host)
BOOST_THROW_EXCEPTION(std::invalid_argument("Cannot schedule host services downtime for non-existent host '" + arguments[0] + "'"));

String triggeredBy;
Downtime::Ptr triggeredBy;
int triggeredByLegacy = Convert::ToLong(arguments[4]);
int is_fixed = Convert::ToLong(arguments[3]);
if (triggeredByLegacy != 0)
triggeredBy = Downtime::GetDowntimeIDFromLegacyID(triggeredByLegacy);

if (triggeredByLegacy != 0) {
triggeredBy = Downtime::GetDowntimeFromLegacyID(triggeredByLegacy);
}

Log(LogNotice, "ExternalCommandProcessor")
<< "Creating downtime for host " << host->GetName();
Expand All @@ -1198,11 +1218,13 @@ void ExternalCommandProcessor::ScheduleHostgroupHostDowntime(double, const std::
if (!hg)
BOOST_THROW_EXCEPTION(std::invalid_argument("Cannot schedule hostgroup host downtime for non-existent hostgroup '" + arguments[0] + "'"));

String triggeredBy;
Downtime::Ptr triggeredBy;
int triggeredByLegacy = Convert::ToLong(arguments[4]);
int is_fixed = Convert::ToLong(arguments[3]);
if (triggeredByLegacy != 0)
triggeredBy = Downtime::GetDowntimeIDFromLegacyID(triggeredByLegacy);

if (triggeredByLegacy != 0) {
triggeredBy = Downtime::GetDowntimeFromLegacyID(triggeredByLegacy);
}

for (const Host::Ptr& host : hg->GetMembers()) {
Log(LogNotice, "ExternalCommandProcessor")
Expand All @@ -1221,11 +1243,13 @@ void ExternalCommandProcessor::ScheduleHostgroupSvcDowntime(double, const std::v
if (!hg)
BOOST_THROW_EXCEPTION(std::invalid_argument("Cannot schedule hostgroup service downtime for non-existent hostgroup '" + arguments[0] + "'"));

String triggeredBy;
Downtime::Ptr triggeredBy;
int triggeredByLegacy = Convert::ToLong(arguments[4]);
int is_fixed = Convert::ToLong(arguments[3]);
if (triggeredByLegacy != 0)
triggeredBy = Downtime::GetDowntimeIDFromLegacyID(triggeredByLegacy);

if (triggeredByLegacy != 0) {
triggeredBy = Downtime::GetDowntimeFromLegacyID(triggeredByLegacy);
}

/* Note: we can't just directly create downtimes for all the services by iterating
* over all hosts in the host group - otherwise we might end up creating multiple
Expand Down Expand Up @@ -1255,11 +1279,13 @@ void ExternalCommandProcessor::ScheduleServicegroupHostDowntime(double, const st
if (!sg)
BOOST_THROW_EXCEPTION(std::invalid_argument("Cannot schedule servicegroup host downtime for non-existent servicegroup '" + arguments[0] + "'"));

String triggeredBy;
Downtime::Ptr triggeredBy;
int triggeredByLegacy = Convert::ToLong(arguments[4]);
int is_fixed = Convert::ToLong(arguments[3]);
if (triggeredByLegacy != 0)
triggeredBy = Downtime::GetDowntimeIDFromLegacyID(triggeredByLegacy);

if (triggeredByLegacy != 0) {
triggeredBy = Downtime::GetDowntimeFromLegacyID(triggeredByLegacy);
}

/* Note: we can't just directly create downtimes for all the hosts by iterating
* over all services in the service group - otherwise we might end up creating multiple
Expand Down Expand Up @@ -1288,11 +1314,13 @@ void ExternalCommandProcessor::ScheduleServicegroupSvcDowntime(double, const std
if (!sg)
BOOST_THROW_EXCEPTION(std::invalid_argument("Cannot schedule servicegroup service downtime for non-existent servicegroup '" + arguments[0] + "'"));

String triggeredBy;
Downtime::Ptr triggeredBy;
int triggeredByLegacy = Convert::ToLong(arguments[4]);
int is_fixed = Convert::ToLong(arguments[3]);
if (triggeredByLegacy != 0)
triggeredBy = Downtime::GetDowntimeIDFromLegacyID(triggeredByLegacy);

if (triggeredByLegacy != 0) {
triggeredBy = Downtime::GetDowntimeFromLegacyID(triggeredByLegacy);
}

for (const Service::Ptr& service : sg->GetMembers()) {
Log(LogNotice, "ExternalCommandProcessor")
Expand Down
8 changes: 4 additions & 4 deletions lib/icinga/scheduleddowntime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,17 +282,17 @@ void ScheduledDowntime::CreateNextDowntime()

Downtime::Ptr downtime = Downtime::AddDowntime(GetCheckable(), GetAuthor(), GetComment(),
segment.first, segment.second,
GetFixed(), String(), GetDuration(), GetName(), GetName());
GetFixed(), nullptr, GetDuration(), GetName(), GetName());
String downtimeName = downtime->GetName();

int childOptions = Downtime::ChildOptionsFromValue(GetChildOptions());
if (childOptions > 0) {
/* 'DowntimeTriggeredChildren' schedules child downtimes triggered by the parent downtime.
* 'DowntimeNonTriggeredChildren' schedules non-triggered downtimes for all children.
*/
String triggerName;
Downtime::Ptr trigger;
if (childOptions == 1)
triggerName = downtimeName;
trigger = downtime;

Log(LogNotice, "ScheduledDowntime")
<< "Processing child options " << childOptions << " for downtime " << downtimeName;
Expand All @@ -302,7 +302,7 @@ void ScheduledDowntime::CreateNextDowntime()
<< "Scheduling downtime for child object " << child->GetName();

Downtime::Ptr childDowntime = Downtime::AddDowntime(child, GetAuthor(), GetComment(),
segment.first, segment.second, GetFixed(), triggerName, GetDuration(), GetName(), GetName());
segment.first, segment.second, GetFixed(), trigger, GetDuration(), GetName(), GetName());

Log(LogNotice, "ScheduledDowntime")
<< "Add child downtime '" << childDowntime->GetName() << "'.";
Expand Down
Loading