[Asterisk-code-review] sched: AST_SCHED_REPLACE_UNREF can lead to use after free of data (asterisk[16])

[George Joseph]

George Joseph has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/15038 )

Change subject: sched: AST_SCHED_REPLACE_UNREF can lead to use after free of data
......................................................................

sched: AST_SCHED_REPLACE_UNREF can lead to use after free of data

The data can be freed if the old object '_data' is the same object as
new 'data'. Because at first the object is unreferenced which can lead
to destroying it.

This could happened in res_pjsip_pubsub when the publication is updated
which could lead to segfault in function publish_expire.

Change-Id: I0164f57c387243510bdbd2f8dcf33377b6c202da
---
M include/asterisk/sched.h
1 file changed, 3 insertions(+), 2 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  Kevin Harwell: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved; Approved for Submit



diff --git a/include/asterisk/sched.h b/include/asterisk/sched.h
index 7ea6709..60a6605 100644
--- a/include/asterisk/sched.h
+++ b/include/asterisk/sched.h
@@ -136,11 +136,12 @@
 		while (id > -1 && (_res = ast_sched_del(sched, id) && _count++ < 10)) { \
 			usleep(1); \
 		} \
-		if (!_res && _data)							\
+		if (!_res && _data && _data != data)					\
 			unrefcall;	/* should ref _data! */		\
 		if (_count == 10) \
 			ast_log(LOG_WARNING, "Unable to cancel schedule ID %d.  This is probably a bug (%s: %s, line %d).\n", id, __FILE__, __PRETTY_FUNCTION__, __LINE__); \
-		refcall; \
+		if (_data != data) \
+			refcall; \
 		id = ast_sched_add_variable(sched, when, callback, data, variable); \
 		if (id == -1)  \
 			addfailcall;	\

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/15038
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I0164f57c387243510bdbd2f8dcf33377b6c202da
Gerrit-Change-Number: 15038
Gerrit-PatchSet: 5
Gerrit-Owner: Alexei Gradinari <alex2grad at gmail.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20201109/99e059b7/attachment-0001.html>