valgrind warning in libnetfilter_log.c

valgrind warning in libnetfilter_log.c

[Hugo Mildenberger]

This is output from valgrind, pointing to a possible problem in 
libnetfilter_log.c +307. 

==16332== Syscall param socketcall.sendmsg(msg.msg_iov[i]) points to 
uninitialised byte(s)
==16332==    at 0x40E0316: sendmsg (socket.S:65)
==16332==    by 0x42632AA: nflog_set_mode (libnetfilter_log.c:307)
==16332==    by 0x424315B: start (ulogd_inppkt_NFLOG.c:466)
==16332==    by 0x804C5E9: create_stack (ulogd.c:961)
==16332==    by 0x804E85B: cfg_parser_action_plugin (cfg_parser.l:524)
==16332==    by 0x805040F: cfg_parser (cfg_parser.l:590)
==16332==    by 0x805061B: cfg_parser_parse_file (cfg_parser.l:773)
==16332==    by 0x804D4B1: config_parse_file (conffile.c:260)
==16332==    by 0x804B497: main (ulogd.c:1405)
==16332==  Address 0xBE8B29F5 is on thread 1's stack

excerpt from libnetfilter_log.c:
int nflog_set_mode(struct nflog_g_handle *gh,
                   u_int8_t mode, u_int32_t range)
{
        char buf[NFNL_HEADER_LEN
                +NFA_LENGTH(sizeof(struct nfulnl_msg_config_mode))];
        struct nfulnl_msg_config_mode params;
        struct nlmsghdr *nmh = (struct nlmsghdr *) buf;

        nfnl_fill_hdr(gh->h->nfnlssh, nmh, 0, AF_UNSPEC, gh->id,
                      NFULNL_MSG_CONFIG, NLM_F_REQUEST|NLM_F_ACK);

        params.copy_range = htonl(range);       /* copy_range is short */
        params.copy_mode = mode;
        nfnl_addattr_l(nmh, sizeof(buf), NFULA_CFG_MODE, &params,
                       sizeof(params));

        return nfnl_talk(gh->h->nfnlh, nmh, 0, 0, NULL, NULL, NULL);
}

The warning  could not be suppressed by zeroing the message buffer in 
nflog_setmode, but it could be suppressed by zeroing  struct 
nfulnl_msg_config_mode params: 

excerpt from libnetfilter_log/linux_nfnetlink_log.h:
struct nfulnl_msg_config_mode {
        u_int32_t       copy_range;
        u_int8_t        copy_mode;
        u_int8_t        _pad;
} __attribute__ ((packed));

Therefore, this warning is caused by "_pad" remaining uninitialised. 
Referencing "_pad" explicitely would be  somewhat unpleasant. Using memset 
would induce additional computation only to suppress a warning. Best solution 
would be to extend .copy_mode from u_int8_t to u_int16_t and drop ._pad 
completely. There are more issues like that in linux_nfnetlink_log.h. I 
wonder why this approach was choosen to achieve 16-bit alignment: "_pad " 
will be transmitted anyway.

Re: valgrind warning in libnetfilter_log.c

[Pablo Neira Ayuso]

Hugo Mildenberger wrote:
> The warning  could not be suppressed by zeroing the message buffer in 
> nflog_setmode, but it could be suppressed by zeroing  struct 
> nfulnl_msg_config_mode params: 
> 
> excerpt from libnetfilter_log/linux_nfnetlink_log.h:
> struct nfulnl_msg_config_mode {
>         u_int32_t       copy_range;
>         u_int8_t        copy_mode;
>         u_int8_t        _pad;
> } __attribute__ ((packed));
> 
> Therefore, this warning is caused by "_pad" remaining uninitialised. 
> Referencing "_pad" explicitely would be  somewhat unpleasant. Using memset 
> would induce additional computation only to suppress a warning. Best solution 
> would be to extend .copy_mode from u_int8_t to u_int16_t and drop ._pad 
> completely. There are more issues like that in linux_nfnetlink_log.h. I 
> wonder why this approach was choosen to achieve 16-bit alignment: "_pad " 
> will be transmitted anyway.

AFAICS, we can run into backward compatibility issues if remove _pad by
extending copy_mode. Hugo, I'd appreciate a patch that uses memset so
that we get rid of this warning.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers