Re: [PATCH] Initialize nscd stats data [BZ #17892]

["Carlos O'Donell" <carlos at redhat dot com>]

On 01/28/2015 12:57 AM, Siddhesh Poyarekar wrote:
> Hi,
> 
> nscd stats data that is sent from the daemon to the binary invoked with
> 'nscd -g' does not always initialize selinux data.  Valgrind complains
> about it, so just initialize the entire struct once to get rid of that
> warning.  I haven't done a full analysis of the code changed, but the
> net change in the code size of the send_stats function is just 2
> bytes.  The valgrind warning:
> 
> #: valgrind nscd -d
> ==11384== Memcheck, a memory error detector
> ==11384== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
> ==11384== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
> ==11384== Command: nscd -d
> ==11384== 
> Fri 25 Apr 2014 10:34:53 AM CEST - 11384: handle_request: request received (Version = 2) from PID 11396
> Fri 25 Apr 2014 10:34:53 AM CEST - 11384:       GETSTAT
> ==11384== Thread 6:
> ==11384== Syscall param socketcall.sendto(msg) points to uninitialised byte(s)
> ==11384==    at 0x4E4ACDC: send (in /lib64/libpthread-2.12.so)
> ==11384==    by 0x11AF6B: send_stats (in /usr/sbin/nscd)
> ==11384==    by 0x112F75: nscd_run_worker (in /usr/sbin/nscd)
> ==11384==    by 0x4E439D0: start_thread (in /lib64/libpthread-2.12.so)
> ==11384==    by 0x599AB6C: clone (in /lib64/libc-2.12.so)
> ==11384==  Address 0x15708395 is on thread 6's stack
> 
> OK to commit to 2.21?

Yes. Please commit this immediately.

We should be zeroing stack structures before using them to send
data to remote systems. While nscd won't leak directly important
information, it's leaking some information that might some day
be used indirectly.

The cost of the memset is also miniscule compared to the cost of the
sendto() and thus I'm not worried about the performance of this
function. I'm more concerned about having a clean valgrind run and
thus avoid confusing users when they see the above.

Cheers,
Carlos.

> Siddhesh
> 
> 	[BZ #17892]
> 	* nscd/nscd_stat.c (send_stats): Initialize DATA.
> 
> diff --git a/nscd/nscd_stat.c b/nscd/nscd_stat.c
> index 0f1f3c0..7aaa21b 100644
> --- a/nscd/nscd_stat.c
> +++ b/nscd/nscd_stat.c
> @@ -94,6 +94,8 @@ send_stats (int fd, struct database_dyn dbs[lastdb])
>    struct statdata data;
>    int cnt;
>  
> +  memset (&data, 0, sizeof (data));
> +
>    memcpy (data.version, compilation, sizeof (compilation));
>    data.debug_level = debug_level;
>    data.runtime = time (NULL) - start_time;
>