Fix invalid access of freed log-writer cfg#5445
Conversation
|
Can one of the admins verify this patch? |
1 similar comment
|
Can one of the admins verify this patch? |
|
@beni-atlnz could you please fix the commits-check issue (you can ignore the fedore-rawhide problems for now), thanks |
|
@beni-atlnz what would you say to this version instead? I think the responsibility belongs at a slightly higher level. Basically, it does the same as what you did, but at a more appropriate level. |
beni-atlnz
force-pushed
the
log-writer-cfg-bug
branch
from
b2dfce5 to
92cb686
Compare
|
@HofiOne the version you have supplied is a great fix for this issue. I think that that is a suitable place to be setting and resetting the cfg of the log writer. |
@beni-atlnz Could you please apply that version to this PR? You can use it freely. |
The cfg stored by a log-writer is freed in cfg deinitialization and then accessed in the cfg reinitialisation. This patch sets the stored cfg of the log writer to NULL and sets the cfg to the new cfg when restored. See here: syslog-ng#5444 Signed-off-by: Benjamin Ireland <[email protected]>
Signed-off-by: Benjamin Ireland <[email protected]>
beni-atlnz
force-pushed
the
log-writer-cfg-bug
branch
from
92cb686 to
722d652
Compare
|
@beni-atlnz Thank you, and thanks for your signal too! |
3 participants
Description
Reloading syslog-ng after a config change caused syslog-ng to crash. During the
main_loop_reload_config_applythe cfg is de-initialized and freed, however inafprogram_dd_deinitthe log writer is storedafprogram_dd_store_reload_store_itembefore the cfg is freed. The log writer is then restored and the cfg it points to no longer exists. Whenlog_pipe_initorlog_pipe_deinitcallscfg_tree_register_initialized_pipefor the log_writer this results in an invalid read, causing a seg fault crashing syslog-ng.See here: #5444
Implementation
To prevent the crash, when the log-writer is stored before the reload takes place the stored log-writer's pointer to the cfg is released. When the log-writer is restored, the cfg is set to the new cfg.
Alternative fixes were considered, such as always creating a new log-writer on reload. However, as the log-writer is being stored, it must be required that the log-writer be persistent after the reload.
Testing
This fix was tested by following the reproduction method outlined in #5444. Observing that valgrind did not detect an invalid read after using
syslog-ng-ctl reload. No syslog-ng crashes have been observed with this patch applied.Fixes: #5444