buginator <buginator@...> commented

(In Warzone2100/warzone2100@1a6fb8a) Fix another memory clobbering issue. (dereferenced pointer)

Patch Author: Safety0ff
fixes #2300

Original detective work done by Ai_Tak (#1656)

Signed-off-by: buginator <buginator@>

Buginator changed _comment0 which not transferred by tractive

buginator <buginator@...> changed status from new to closed

buginator <buginator@...> changed resolution from `` to fixed

Buginator uploaded file val_no_fe.txt (1.4 KiB)

Buginator uploaded file val_with_fe.txt (2.0 KiB)

Buginator uploaded file val_wPatch_fe.txt (1.2 KiB)

Buginator uploaded file val_wPatch_sa_fe.txt (1.2 KiB)

Buginator uploaded file warzone2100.gdmp-DXGWAl.txt (9.9 KiB)

Buginator changed _comment0 which not transferred by tractive

buginator <buginator@...> commented

In Warzone2100/warzone2100@1a6fb8a:

#CommitTicketReference repository="" revision="1a6fb8a7547f23ce2bd7af153ac839cba999e2b0"
Fix another memory clobbering issue. (dereferenced pointer)

Patch Author: Safety0ff
fixes #2300

Original detective work done by Ai_Tak (#1656)

Signed-off-by: buginator <buginator@>

Buginator commented

The deal is, when you run normal valgrind, then you get val_no_fe.txt.
Notice the invalid reads.

When you set valgrind to fill the free()ed memory with 0xfe, (--fill-free=0xfe then you get val_with_fe.txt, and also the crash dump.

The final results are in val_wPatch_sa_fe.txt, and we don't have invalid reads anymore.

cybersphinx <chr.ohm@...> commented

(In Warzone2100/warzone2100@34112c9) Fix another memory clobbering issue. (dereferenced pointer)

Patch Author: Safety0ff
fixes #2300

Original detective work done by Ai_Tak (#1656)

Signed-off-by: buginator buginator@gna.org
(cherry picked from commit 1a6fb8a7547f23ce2bd7af153ac839cba999e2b0)

cybersphinx <chr.ohm@...> commented

In Warzone2100/warzone2100@34112c9:

#CommitTicketReference repository="" revision="34112c9e2bb5da22eac9e4a5ecb71229b6df07d2"
Fix another memory clobbering issue. (dereferenced pointer)

Patch Author: Safety0ff
fixes #2300

Original detective work done by Ai_Tak (#1656)

Signed-off-by: buginator <buginator@gna.org>
(cherry picked from commit 1a6fb8a7547f23ce2bd7af153ac839cba999e2b0)

Safety0ff commented

I don't really consider this "fixed", I've been looking at that part of the code lately and understanding it more and more.

This type of error likely only occurs when a Callback event uses the setTrigger function, but there could potentialy be other ways for setTrigger calls to mess things up.

Anyways, I'm sure that deferred deletion would solve this and it will also handle the case where multiple events are triggered and one calls setTrigger for one of those other events.

I'd rather this not be "swept under the rug".

buginator <buginator@...> commented

(In Warzone2100/warzone2100@f8517a7) Fix another memory clobbering issue. (dereferenced pointer)

Patch Author: Safety0ff
fixes #2300

Original detective work done by Ai_Tak (#1656)

Signed-off-by: buginator buginator@gna.org
(cherry picked from commit 1a6fb8a7547f23ce2bd7af153ac839cba999e2b0)

buginator <buginator@...> commented

In Warzone2100/warzone2100@f8517a7:

#CommitTicketReference repository="" revision="f8517a7716e2fe11a78fc49ca73ac92b2f7d5211"
Fix another memory clobbering issue. (dereferenced pointer)

Patch Author: Safety0ff
fixes #2300

Original detective work done by Ai_Tak (#1656)

Signed-off-by: buginator <buginator@gna.org>
(cherry picked from commit 1a6fb8a7547f23ce2bd7af153ac839cba999e2b0)

Safety0ff uploaded file defdelMaster.patch (10.2 KiB)

Safety0ff changed status from closed to reopened

Safety0ff changed _comment0 which not transferred by tractive

Safety0ff changed _comment1 which not transferred by tractive

Safety0ff commented

In the new patch, I have reverted 1a6fb8a7547f23ce2bd7af153ac839cba999e2b0 by hand and added a field to mark trigger deletion, this is a much more robust solution imho.

I also changed a function to use pointers to pointers to remove code duplication.

This patch will likely apply to the other branches as well.

I ran it with valgrind and there were no warnings.

I believe it should be possible to construct a script that causes invalid pointer dereferences under the old patch (hence the re-open).

Safety0ff changed resolution from fixed to ``

cybersphinx uploaded file defdel-cleaned-up.patch (8.6 KiB)

Rebased on a tree with 1a6fb8a7547f23ce2bd7af153ac839cba999e2b0 already reverted.

Safety0ff uploaded file defdelMaster.2.patch (9.5 KiB)

cybersphinx uploaded file defdel2-cleaned-up.patch (8.6 KiB)

Patch without the revert.

Safety0ff uploaded file defdelMaster.3.patch (9.0 KiB)

Fixed a bug (notice the lack of fixme)

Safety0ff uploaded file defdel3-cleaned-up.patch (8.4 KiB)

Same as above without revert

safety0ff changed status from reopened to closed

safety0ff changed resolution from `` to fixed

safety0ff commented

Fix an invalid pointer dereferencing issue in the script events system.

Closes #2300.
Changeset: 8c9e7d5bd0bcc0029ba50422b5b77089ed4202fe

safety0ff commented

2.3: Fix an invalid pointer dereferencing issue in the script events system.

Closes #2300.
Changeset: ddfaeca6776aac765674bb0c2a5c58ecc63a978f

safety0ff commented

3.0: Fix an invalid pointer dereferencing issue in the script events system.

Closes #2300.
Changeset: dfbceff8647ecf3e488733ed7e18e4901e4ee774

buginator commented

Fix another memory clobbering issue. (dereferenced pointer)

Patch Author: Safety0ff
fixes #2300

Original detective work done by Ai_Tak (#1656)

Signed-off-by: buginator buginator@gna.org
(cherry picked from commit 1a6fb8a7547f23ce2bd7af153ac839cba999e2b0)
Changeset: f8517a7716e2fe11a78fc49ca73ac92b2f7d5211

safety0ff commented

Fix an invalid pointer dereferencing issue in the script events system.

Closes #2300.
Changeset: 8c9e7d5bd0bcc0029ba50422b5b77089ed4202fe

cybersphinx changed milestone from 3.0 to unspecified

cybersphinx commented

Milestone 3.0 deleted