Opened 7 years ago

Closed 7 years ago

#410 closed defect (wontfix)

Valgrind falsepositive with ngx_string length 0

Reported by: markus-linnala.myopenid.com Owned by:
Priority: minor Milestone:
Component: nginx-core Version: 1.3.x
Keywords: Cc:
uname -a:
nginx -V: nginx version: nginx/1.5.5

Description

I modified http://mdounin.ru/hg/nginx-tests to use valgrind.

==10811== Conditional jump or move depends on uninitialised value(s)
==10811== at 0x4A0941F: memcpy@@GLIBC_2.14 (mc_replace_strmem.c:653)
==10811== by 0x4248A0: ngx_vslprintf (ngx_string.c:236)
==10811== by 0x421138: ngx_log_error_core (ngx_log.c:119)
==10811== by 0x44F4B2: ngx_http_output_filter (ngx_http_core_module.c:1966)
==10811== by 0x4535F9: ngx_http_send_special (ngx_http_request.c:3280)
==10811== by 0x465283: ngx_http_upstream_finalize_request (ngx_http_upstream.c:3604)
==10811== by 0x465C9D: ngx_http_upstream_process_request (ngx_http_upstream.c:3200)
==10811== by 0x465DF1: ngx_http_upstream_process_upstream (ngx_http_upstream.c:3131)
==10811== by 0x4694CE: ngx_http_upstream_process_header (ngx_http_upstream.c:2532)
==10811== by 0x46595C: ngx_http_upstream_handler (ngx_http_upstream.c:996)
==10811== by 0x4411EE: ngx_epoll_process_events (ngx_epoll_module.c:691)
==10811== by 0x437A61: ngx_process_events_and_timers (ngx_event.c:249)
==10811==

Version is 1.5.5.

Test was: http_try_files.t

Problem is that memcpy looks v->data before looking lenght of copy and v->data is unititialized. Memcpy does not do anything if length is zero and therefore this is falsepositive.

There is other traces leading to the warning, most of them are debug messages with zero length strings. To see this you need to have --with-debug.

Attachments (1)

valgrind-falsepositive-with-ngx_string-len-0.patch (739 bytes ) - added by markus-linnala.myopenid.com 7 years ago.

Download all attachments as: .zip

Change History (3)

by markus-linnala.myopenid.com, 7 years ago

comment:1 by markus-linnala.myopenid.com, 7 years ago

I looked this more and this is valgrind 3.6.1 bug. The valgrind version does not check length of string as it
should. This is fixed on newer valgrind 3.8.1

From valgrind source:

564 #define MEMCPY_BODY \
565 { \
566 if (is_overlap(dst, src, len, len)) \
567 RECORD_OVERLAP_ERROR("memcpy", dst, src, len); \
568 \
569 const Addr WS = sizeof(UWord); /* 8 or 4 */ \
570 const Addr WM = WS - 1; /* 7 or 3 */ \
571 \
572 if (dst < src) { \ <<<--- HERE use string before length.

Last edited 7 years ago by markus-linnala.myopenid.com (previous) (diff)

comment:2 by Maxim Dounin, 7 years ago

Resolution: wontfix
Status: newclosed

I don't think we should workaround bugs of old Valgrind versions.

Note: See TracTickets for help on using tickets.