COVESA / dlt-daemon

Diagnostic Log and Trace.
https://covesa.github.io/dlt-daemon/
Mozilla Public License 2.0
381 stars 293 forks source link

dlt_user.dlt_log_handle used without any form of syncronization #253

Open martin-ejdestig opened 4 years ago

martin-ejdestig commented 4 years ago

It looks to me like there are several places in https://github.com/GENIVI/dlt-daemon/blob/master/src/lib/dlt_user.c where dlt_user.dlt_log_handle is used without any thread safe synchronization mechanism.

E.g. it is closed and set to -1 in dlt_user_log_send_log() at: https://github.com/GENIVI/dlt-daemon/blob/602ed3ef81101421822ecc2d44f0884ff981c0e6/src/lib/dlt_user.c#L3753 but I do not see a lock being held. dlt_user_log_send_log() is called from dlt_user_log_write_finish() that is in turn used in DLT_LOG_X user level macros that can be used from any thread.

That dlt_user.dlt_log_handle is closed and then set to -1 at location above means there is also potential for corrupting data if e.g. another thread is scheduled that gets same fd (fd:s can be reused) for itself, and before thread is scheduled that sets dlt_user.dlt_log_handle to -1, another thread may be scheduled that writes DLT data to a file descriptor that now has nothing to do with DLT.

Am I missing something perhaps?

ssugiura commented 3 years ago

@martin-ejdestig Sorry for my late reply and thanks for pointing out. I assume you're right, the thread needs to acquire a lock in case of close() call.

ssugiura commented 3 years ago

We unfortunately do not have time to work on this for now, I would appreciate if you/anyone can contribute here for improvement. Thanks!