Opened 11 years ago

Closed 11 years ago

#369 closed defect (fixed)

Valgrind warning for using unititialised bytes when writing cache file

Reported by: markus-linnala.myopenid.com Owned by: Maxim Dounin
Priority: minor Milestone:
Component: nginx-core Version: 1.2.x
Keywords: valgrind proxy Cc:
uname -a: Linux workstation 3.6.11-4.fc16.x86_64 #1 SMP Tue Jan 8 20:57:42 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux
nginx -V: nginx version: nginx/1.2.9 (no pool)
built by gcc 4.6.3 20120306 (Red Hat 4.6.3-2) (GCC)
configure arguments: --prefix=/home/maage/nginx --with-debug

Description

% valgrind --tool=memcheck --track-origins=yes --malloc-fill=7E ~/nginx/sbin/nginx -p $(pwd)/t/servroot/ -c $(pwd)/t/servroot/conf/nginx.conf
==3476== Memcheck, a memory error detector
==3476== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==3476== Using Valgrind-3.6.1 and LibVEX; rerun with -h for copyright info
==3476== Command: /home/maage/nginx/sbin/nginx -p /home/maage/a/nginx-1.2.9/t/servroot/ -c /home/maage/a/nginx-1.2.9/t/servroot/conf/nginx.conf
==3476==
==3476== Syscall param pwrite64(buf) points to uninitialised byte(s)
==3476==    at 0x3B1980EEF3: __pwrite_nocancel (in /lib64/libpthread-2.14.90.so)
==3476==    by 0x41DBD0: ngx_write_file (ngx_files.c:80)
==3476==    by 0x41DDB9: ngx_write_chain_to_file (ngx_files.c:209)
==3476==    by 0x40AB7E: ngx_write_chain_to_temp_file (ngx_file.c:36)
==3476==    by 0x41C04A: ngx_event_pipe_write_chain_to_temp_file (ngx_event_pipe.c:753)
==3476==    by 0x41D351: ngx_event_pipe (ngx_event_pipe.c:441)
==3476==    by 0x43F23D: ngx_http_upstream_process_upstream (ngx_http_upstream.c:2663)
==3476==    by 0x4411E3: ngx_http_upstream_process_header (ngx_http_upstream.c:2362)
==3476==    by 0x43F3A4: ngx_http_upstream_handler (ngx_http_upstream.c:949)
==3476==    by 0x422EC1: ngx_epoll_process_events (ngx_epoll_module.c:683)
==3476==    by 0x41A1F9: ngx_process_events_and_timers (ngx_event.c:247)
==3476==    by 0x42275A: ngx_single_process_cycle (ngx_process_cycle.c:315)
==3476==  Address 0x4c7bb72 is 34 bytes inside a block of size 4,096 alloc'd
==3476==    at 0x4A074CD: malloc (vg_replace_malloc.c:236)
==3476==    by 0x41D955: ngx_alloc (ngx_alloc.c:22)
==3476==    by 0x40521D: ngx_malloc (ngx_palloc.c:119)
==3476==    by 0x405267: ngx_palloc (ngx_palloc.c:140)
==3476==    by 0x4405AF: ngx_http_upstream_process_header (ngx_http_upstream.c:1538)
==3476==    by 0x43F3A4: ngx_http_upstream_handler (ngx_http_upstream.c:949)
==3476==    by 0x422EC1: ngx_epoll_process_events (ngx_epoll_module.c:683)
==3476==    by 0x41A1F9: ngx_process_events_and_timers (ngx_event.c:247)
==3476==    by 0x42275A: ngx_single_process_cycle (ngx_process_cycle.c:315)
==3476==    by 0x4040E3: main (nginx.c:409)
==3476==  Uninitialised value was created by a heap allocation
==3476==    at 0x4A074CD: malloc (vg_replace_malloc.c:236)
==3476==    by 0x41D955: ngx_alloc (ngx_alloc.c:22)
==3476==    by 0x40521D: ngx_malloc (ngx_palloc.c:119)
==3476==    by 0x405267: ngx_palloc (ngx_palloc.c:140)
==3476==    by 0x4405AF: ngx_http_upstream_process_header (ngx_http_upstream.c:1538)
==3476==    by 0x43F3A4: ngx_http_upstream_handler (ngx_http_upstream.c:949)
==3476==    by 0x422EC1: ngx_epoll_process_events (ngx_epoll_module.c:683)
==3476==    by 0x41A1F9: ngx_process_events_and_timers (ngx_event.c:247)
==3476==    by 0x42275A: ngx_single_process_cycle (ngx_process_cycle.c:315)
==3476==    by 0x4040E3: main (nginx.c:409)
==3476==

Config:

worker_processes  1;
daemon off;
master_process off;
http {
    proxy_cache_path  /tmp/ngx_cache keys_zone=test_cache:10m;
    proxy_temp_path   /tmp/ngx_temp 1 2;
    server {
        listen          1984;
        server_name     _;
        client_max_body_size 30M;
        location /proxy {
            proxy_pass  $scheme://127.0.0.1:$server_port/etc/issue;
            proxy_cache test_cache;
            proxy_cache_valid  1h;
        }
        location = /etc/issue {
            root               /;
        }
    }
}
events {
    worker_connections  64;
}

As I look cachefile, there is six '~' chars (7E above) at the end of first line. You need to have clear cache to see the warning.

This is because writing members of struct ngx_http_file_cache_header_t does not fill its size fully. And therefore nginx writes unitialised bytes at the end of the ngx_http_file_cache_header_t.

With this patch I can silence warning.

--- src/http/ngx_http_file_cache.c-	2013-06-04 21:55:09.000000000 +0300
+++ src/http/ngx_http_file_cache.c	2013-06-04 21:55:44.000000000 +0300
@@ -873,6 +873,8 @@ ngx_http_file_cache_set_header(ngx_http_
     ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
                    "http file cache set header");
 
+    ngx_memzero(h, sizeof(ngx_http_file_cache_header_t));
+
     c = r->cache;
 
     h->valid_sec = c->valid_sec;

Change History (2)

comment:1 by Maxim Dounin, 11 years ago

Owner: set to Maxim Dounin
Status: newassigned

Yes, thanks, I have patch sitting here to silence some false-positive valgrind warnings, including this one.

comment:2 by Maxim Dounin, 11 years ago

Resolution: fixed
Status: assignedclosed

Fixed by 711fa02afae8, thanks for prodding this.

Note: See TracTickets for help on using tickets.