confluentinc / librdkafka

The Apache Kafka C/C++ library
Other
293 stars 3.15k forks source link

MemorySanitizer: use-of-uninitialized-value in rdtime.h #4865

Open fdr400 opened 1 month ago

fdr400 commented 1 month ago

Description

librdkafka fails with MemorySanitizer with use-of-unitialized-value in src/rdtime.h.

The error is precisely:

==3601446==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x5555559e8cfd in rd_timeout_init_timespec /home/fdr400/tmp/librdkafka/src/rdtime.h:255:21
    #1 0x5555559e7a76 in rd_kafka_q_serve /home/fdr400/tmp/librdkafka/src/rdkafka_queue.c:543:9
    #2 0x55555564f71e in rd_kafka_thread_main /home/fdr400/tmp/librdkafka/src/rdkafka.c:2155:17
    #3 0x7ffff789c86a in start_thread nptl/pthread_create.c:444:30
    #4 0x7ffff7929c3b in clone3 misc/../sysdeps/unix/sysv/linux/x86_64/clone3.S:78

SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/fdr400/tmp/librdkafka/src/rdtime.h:255:21 in rd_timeout_init_timespec
Exiting

Error is in rd_timeout_init_timespec and in rd_timeout_init_timespec_us. Their argument tspec is always uninitialized and timespec_get(tspec, TIME_UTC); is not initializing it entirely.

I found the comment about the same issue in zstd code: https://github.com/facebook/zstd/blob/dev/programs/timefn.c#L74

Importance

I am from @userver-framework, we have a Kafka client based on librdkafka, which represents non-blocking and scalable interfaces for producers and consumers. We have huge number of tests and CI checks both for internal (in company's repository) and external (in Github CI) repositories, under different compilers, build options and sanitizers. But we cannot enable MSAN sanitizer for Kafka client because of the error.

P.S. I checked that with my fix, our tests works.

P.P.S. Client code: https://github.com/userver-framework/userver/tree/develop/kafka

How to reproduce

I made a repo with instruction and small code example to reproduce the error and check that my fix works. Repository link: https://github.com/fdr400/rdkafka-producer-msan

Checklist

IMPORTANT: We will close issues where the checklist has not been completed.

Please provide the following information:

emasab commented 1 month ago

Hi, thanks for the report, the problems seems to be that timespec_get can return an error an in that case the tspec isn't initialized. That should be the problem I think. Could you try if initializing it to zero on error fixes your warning?

fdr400 commented 1 month ago

@emasab i tried to do so and it fixes the problem, but i don't think it is better solution, because

  1. it is easy to forget about initialization of tspec before calling the rd_timeout_init_timespec
  2. it may lead to some performance issues caused by unnecessary memset in runtime

Initalization inside rd_timeout_init_timespec also is not a good idea, because, i suppose, it is unexpected from function that it zeroing the input argument. It can return the constructed tspec, but it is not the C style

So, the sanitizer unpoison is enough good solution, because it does nothing in Release and Debug builds

fdr400 commented 1 week ago

@emasab