osmocom
/
wireshark
11
0
Fork 1
Code Issues Pull Requests Projects Releases Wiki Activity

Fix a mis-merging. 

Also, remove the "make sure we're not fetching a bogus structure" tests.

Add a comment explaining how a compiler bug where it's overly optimizing
a combination of tests could cause the valgrind errors we were seeing,
so we're zeroing the entire structure, padding included, to avoid that.

Change-Id: I24f94b2cbceec5234c1da82b891f609648075839
Reviewed-on: https://code.wireshark.org/review/19149
Reviewed-by: Guy Harris <guy@alum.mit.edu>
This commit is contained in:
Guy Harris 2016-12-08 12:34:59 -08:00
parent a02d8e3c4e
commit d438170c87
1 changed files with 22 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
epan/dissectors/packet-cops.c
View File

@ -805,9 +805,6 @@ typedef struct _cops_conv_info_t {
typedef struct _cops_call_t
{
#if 1
guint32 magic;
#endif
guint8 op_code;
gboolean solicited;
guint32 req_num;
@ -1055,11 +1052,29 @@ dissect_cops_pdu(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data
}
if (!pinfo->fd->flags.visited) {
/*
* XXX - yes, we're setting all the fields in this
* structure, but there's padding between op_code
* and solicited, and that can't be set.
*
* For some reason, on some platforms, valgrind is
* complaining about a test of the solicited field
* accessing uninitialized data, perhaps because
* the 8 bytes containing op_code and solicited is
* being loaded as a unit. If the compiler is, for
* example, turning a test of
*
* cops_call->op_code == COPS_MSG_KA && !(cops_call->solicited)
*
* into a load of those 8 bytes and a comparison against a value
* with op_code being COPS_MSG_KA, solicited being false (0),
* *and* the padding being zero, it's buggy, but overly-"clever"
* buggy compilers do exist, so....)
*
* So we use wmem_new0() to forcibly zero out the entire
* structure before filling it in.
*/
cops_call = wmem_new0(wmem_file_scope(), cops_call_t);
cops_call = wmem_new(wmem_file_scope(), cops_call_t);
#if 1
cops_call->magic = 0xbeeff00d;
#endif
cops_call->op_code = op_code;
cops_call->solicited = is_solicited;
cops_call->req_num = pinfo->num;
@ -1070,9 +1085,6 @@ cops_call->magic = 0xbeeff00d;
else {
for (i=0; i < pdus_array->len; i++) {
cops_call = (cops_call_t*)g_ptr_array_index(pdus_array, i);
#if 1
DISSECTOR_ASSERT(cops_call->magic == 0xbeeff00d);
#endif
if ( cops_call->req_num == pinfo->num
&& cops_call->rsp_num != 0) {
ti = proto_tree_add_uint_format(cops_tree, hf_cops_response_in, tvb, 0, 0, cops_call->rsp_num,
@ -1092,9 +1104,6 @@ DISSECTOR_ASSERT(cops_call->magic == 0xbeeff00d);
if (!pinfo->fd->flags.visited) {
for (i=0; i < pdus_array->len; i++) {
cops_call = (cops_call_t*)g_ptr_array_index(pdus_array, i);
#if 1
DISSECTOR_ASSERT(cops_call->magic == 0xbeeff00d);
#endif
if (nstime_cmp(&pinfo->abs_ts, &cops_call->req_time) <= 0 || cops_call->rsp_num != 0)
continue;
@ -1122,9 +1131,6 @@ DISSECTOR_ASSERT(cops_call->magic == 0xbeeff00d);
else {
for (i=0; i < pdus_array->len; i++) {
cops_call = (cops_call_t*)g_ptr_array_index(pdus_array, i);
#if 1
DISSECTOR_ASSERT(cops_call->magic == 0xbeeff00d);
#endif
if ( cops_call->rsp_num == pinfo->num ) {
ti = proto_tree_add_uint_format(cops_tree, hf_cops_response_to, tvb, 0, 0, cops_call->req_num,
"Response to a request in frame %u", cops_call->req_num);