summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorChris Luke <chrisy@flirble.org>2017-09-18 08:51:22 -0400
committerFlorin Coras <florin.coras@gmail.com>2017-09-18 18:26:04 +0000
commitb2bcad6238b7e8a669ae29c74079eb9bb9fbb694 (patch)
treea8657766e3ae782ed8dacc4ac1ea559a0e60ae86
parent2019748a0ef815852281aae0a603f0e970fa9d91 (diff)
Fixes for issues Coverity has reported (VPP-972)
177117: fstat() returns -1 on error; the code is checking for any positive value instead 175142: final return could never be reached; simple refactoring 175235,175236: Warning suppressed with an explicit cast to (void) 174817: Final return couldn't be reached; is is_in_order is 0 then 'rv' is already returned above 172095,172093: If is_is_set does not get set to 1, then return 0 has already been invoked 174405: Re-kill this (nothing sets rv) 171136: Looks like a cmd line flag to set test_bytes was missing; added it, and refactored the argc/argv processing to avoid two other potential segv's 176813: Add range checking for term width/height. First stab at a reasonable range is 1-512 for both. 175350: Fix implicit casting in shift operation 174272: Not a c+p error; try using a coverity annotation to ignore it 174273,175320: Annotated FORWARD_NULL Change-Id: I58d0f860fc2209f59f8d1b6b344d631b8d429ace Signed-off-by: Chris Luke <chrisy@flirble.org>
-rw-r--r--src/uri/sock_test_client.c1
-rw-r--r--src/uri/uri_socket_test.c24
-rw-r--r--src/uri/vppcom.c8
-rw-r--r--src/vlib/linux/physmem.c2
-rw-r--r--src/vlib/unix/cli.c30
-rw-r--r--src/vnet/mpls/mpls_api.c2
-rw-r--r--src/vnet/session/session.c5
-rwxr-xr-xsrc/vnet/session/session_cli.c37
-rw-r--r--src/vnet/session/session_lookup.c8
-rw-r--r--src/vppinfra/linux/mem.c2
10 files changed, 74 insertions, 45 deletions
diff --git a/src/uri/sock_test_client.c b/src/uri/sock_test_client.c
index ab8e5a0e4a7..151c90b2960 100644
--- a/src/uri/sock_test_client.c
+++ b/src/uri/sock_test_client.c
@@ -429,6 +429,7 @@ exit_client (void)
tsock = &scm->test_socket[i];
tsock->cfg.test = SOCK_TEST_TYPE_EXIT;
+ /* coverity[COPY_PASTE_ERROR] */
if (ctrl->cfg.verbose)
{
printf ("\nCLIENT (fd %d): Sending exit cfg to server...\n",
diff --git a/src/uri/uri_socket_test.c b/src/uri/uri_socket_test.c
index 5f7084d5b20..4469b03d4c2 100644
--- a/src/uri/uri_socket_test.c
+++ b/src/uri/uri_socket_test.c
@@ -36,8 +36,6 @@ main (int argc, char *argv[])
if (argc >= 3)
{
- bytes = ((long) atoi (argv[4])) << 20;
- no_echo = atoi (argv[3]);
portno = atoi (argv[2]);
server = gethostbyname (argv[1]);
if (server == NULL)
@@ -45,6 +43,28 @@ main (int argc, char *argv[])
clib_unix_warning ("gethostbyname");
exit (1);
}
+
+ argc -= 3;
+ argv += 3;
+
+ if (argc)
+ {
+ bytes = ((long) atoi (argv[0])) << 20;
+ argc--;
+ argv++;
+ }
+ if (argc)
+ {
+ no_echo = atoi (argv[0]);
+ argc--;
+ argv++;
+ }
+ if (argc)
+ {
+ test_bytes = atoi (argv[0]);
+ argc--;
+ argv++;
+ }
}
else
{
diff --git a/src/uri/vppcom.c b/src/uri/vppcom.c
index 8a8a806caf0..c7ae0ea5eff 100644
--- a/src/uri/vppcom.c
+++ b/src/uri/vppcom.c
@@ -1478,7 +1478,7 @@ vppcom_cfg_read (char *conf_fname)
while (unformat_check_input (input) != UNFORMAT_END_OF_INPUT)
{
- unformat_user (input, unformat_line_input, line_input);
+ (void) unformat_user (input, unformat_line_input, line_input);
unformat_skip_white_space (line_input);
if (unformat (line_input, "vppcom {"))
@@ -2359,12 +2359,14 @@ vppcom_select (unsigned long n_bits, unsigned long *read_map,
clib_bitmap_get (vcm->ex_bitmap, session_index) && (rv < 0))
{
// TBD: clib_warning
+ /* coverity[FORWARD_NULL] */
clib_bitmap_set_no_check (except_map, session_index, 1);
bits_set++;
}
else if (rv > 0)
{
// TBD: clib_warning
+ /* coverity[FORWARD_NULL] */
clib_bitmap_set_no_check (read_map, session_index, 1);
bits_set++;
}
@@ -2387,9 +2389,10 @@ vppcom_select (unsigned long n_bits, unsigned long *read_map,
rv = vppcom_session_write_ready (session, session_index);
clib_spinlock_unlock (&vcm->sessions_lockp);
- if (rv > 0)
+ if (rv > 0 )
{
// TBD: clib_warning
+ /* coverity[FORWARD_NULL] */
clib_bitmap_set_no_check (write_map, session_index, 1);
bits_set++;
}
@@ -2415,6 +2418,7 @@ vppcom_select (unsigned long n_bits, unsigned long *read_map,
if (rv < 0)
{
// TBD: clib_warning
+ /* coverity[FORWARD_NULL] */
clib_bitmap_set_no_check (except_map, session_index, 1);
bits_set++;
}
diff --git a/src/vlib/linux/physmem.c b/src/vlib/linux/physmem.c
index 3cc42a0676d..6d3f7c55d32 100644
--- a/src/vlib/linux/physmem.c
+++ b/src/vlib/linux/physmem.c
@@ -157,7 +157,7 @@ unix_physmem_region_alloc (vlib_main_t * vm, char *name, u32 size,
pr->mem = alloc.addr;
pr->log2_page_size = alloc.log2_page_size;
pr->n_pages = alloc.n_pages;
- pr->size = pr->n_pages << pr->log2_page_size;
+ pr->size = (u64) pr->n_pages << (u64) pr->log2_page_size;
pr->page_mask = (1 << pr->log2_page_size) - 1;
pr->numa_node = numa_node;
pr->name = format (0, "%s", name);
diff --git a/src/vlib/unix/cli.c b/src/vlib/unix/cli.c
index 1567cc2ac0a..1624ce387fb 100644
--- a/src/vlib/unix/cli.c
+++ b/src/vlib/unix/cli.c
@@ -91,6 +91,15 @@
* protocol message. This is a saftey measure. */
#define UNIX_CLI_MAX_DEPTH_TELNET 24
+/** Minimum terminal width we will accept */
+#define UNIX_CLI_MIN_TERMINAL_WIDTH 1
+/** Maximum terminal width we will accept */
+#define UNIX_CLI_MAX_TERMINAL_WIDTH 512
+/** Minimum terminal height we will accept */
+#define UNIX_CLI_MIN_TERMINAL_HEIGHT 1
+/** Maximum terminal height we will accept */
+#define UNIX_CLI_MAX_TERMINAL_HEIGHT 512
+
/** Unix standard in */
#define UNIX_CLI_STDIN_FD 0
@@ -1164,10 +1173,21 @@ unix_cli_process_telnet (unix_main_t * um,
/* Window size */
if (i != 8) /* check message is correct size */
break;
+
cf->width =
clib_net_to_host_u16 (*((u16 *) (input_vector + 3)));
+ if (cf->width > UNIX_CLI_MAX_TERMINAL_WIDTH)
+ cf->width = UNIX_CLI_MAX_TERMINAL_WIDTH;
+ if (cf->width < UNIX_CLI_MIN_TERMINAL_WIDTH)
+ cf->width = UNIX_CLI_MIN_TERMINAL_WIDTH;
+
cf->height =
clib_net_to_host_u16 (*((u16 *) (input_vector + 5)));
+ if (cf->height > UNIX_CLI_MAX_TERMINAL_HEIGHT)
+ cf->height = UNIX_CLI_MAX_TERMINAL_HEIGHT;
+ if (cf->height < UNIX_CLI_MIN_TERMINAL_HEIGHT)
+ cf->height = UNIX_CLI_MIN_TERMINAL_HEIGHT;
+
/* reindex pager buffer */
unix_cli_pager_reindex (cf);
/* redraw page */
@@ -2539,8 +2559,18 @@ unix_cli_resize_interrupt (int signum)
/* We can't trust ws.XXX... */
return;
}
+
cf->width = ws.ws_col;
+ if (cf->width > UNIX_CLI_MAX_TERMINAL_WIDTH)
+ cf->width = UNIX_CLI_MAX_TERMINAL_WIDTH;
+ if (cf->width < UNIX_CLI_MIN_TERMINAL_WIDTH)
+ cf->width = UNIX_CLI_MIN_TERMINAL_WIDTH;
+
cf->height = ws.ws_row;
+ if (cf->height > UNIX_CLI_MAX_TERMINAL_HEIGHT)
+ cf->height = UNIX_CLI_MAX_TERMINAL_HEIGHT;
+ if (cf->height < UNIX_CLI_MIN_TERMINAL_HEIGHT)
+ cf->height = UNIX_CLI_MIN_TERMINAL_HEIGHT;
/* Reindex the pager buffer */
unix_cli_pager_reindex (cf);
diff --git a/src/vnet/mpls/mpls_api.c b/src/vnet/mpls/mpls_api.c
index 988c2c98232..762c40ffa8f 100644
--- a/src/vnet/mpls/mpls_api.c
+++ b/src/vnet/mpls/mpls_api.c
@@ -96,7 +96,7 @@ vl_api_mpls_table_add_del_t_handler (vl_api_mpls_table_add_del_t * mp)
else
mpls_table_delete (ntohl (mp->mt_table_id), 1);
- rv = (rv == 0) ? vnm->api_errno : rv;
+ // NB: Nothing sets rv; none of the above returns an error
REPLY_MACRO (VL_API_MPLS_TABLE_ADD_DEL_REPLY);
}
diff --git a/src/vnet/session/session.c b/src/vnet/session/session.c
index 4544f9a0f93..792e6612dc1 100644
--- a/src/vnet/session/session.c
+++ b/src/vnet/session/session.c
@@ -267,10 +267,7 @@ stream_session_enqueue_data (transport_connection_t * tc, vlib_buffer_t * b,
}
}
- if (is_in_order)
- return enqueued;
-
- return 0;
+ return enqueued;
}
/** Check if we have space in rx fifo to push more bytes */
diff --git a/src/vnet/session/session_cli.c b/src/vnet/session/session_cli.c
index d9f516beab9..8c30a1df7eb 100755
--- a/src/vnet/session/session_cli.c
+++ b/src/vnet/session/session_cli.c
@@ -127,13 +127,8 @@ unformat_stream_session_id (unformat_input_t * input, va_list * args)
*is_ip4 = 0;
tuple_is_set = 1;
}
- else
- return 0;
-
- if (tuple_is_set)
- return 1;
- return 0;
+ return tuple_is_set;
}
uword
@@ -144,21 +139,12 @@ unformat_stream_session (unformat_input_t * input, va_list * args)
u8 proto = ~0;
ip46_address_t lcl, rmt;
u32 lcl_port = 0, rmt_port = 0;
- u8 is_ip4 = 0, s_type = ~0, id_is_set = 0;
+ u8 is_ip4 = 0, s_type = ~0;
- if (unformat (input, "%U", unformat_stream_session_id, &proto, &lcl, &rmt,
- &lcl_port, &rmt_port, &is_ip4))
- {
- id_is_set = 1;
- }
- else
+ if (!unformat (input, "%U", unformat_stream_session_id, &proto, &lcl, &rmt,
+ &lcl_port, &rmt_port, &is_ip4))
return 0;
- if (!id_is_set)
- {
- return 0;
- }
-
s_type = session_type_from_proto_and_ip (proto, is_ip4);
if (is_ip4)
s = stream_session_lookup4 (&lcl.ip4, &rmt.ip4,
@@ -185,21 +171,12 @@ unformat_transport_connection (unformat_input_t * input, va_list * args)
u8 proto = ~0;
ip46_address_t lcl, rmt;
u32 lcl_port = 0, rmt_port = 0;
- u8 is_ip4 = 0, s_type = ~0, id_is_set = 0;
+ u8 is_ip4 = 0, s_type = ~0;
- if (unformat (input, "%U", unformat_stream_session_id, &proto, &lcl, &rmt,
- &lcl_port, &rmt_port, &is_ip4))
- {
- id_is_set = 1;
- }
- else
+ if (!unformat (input, "%U", unformat_stream_session_id, &proto, &lcl, &rmt,
+ &lcl_port, &rmt_port, &is_ip4))
return 0;
- if (!id_is_set)
- {
- return 0;
- }
-
proto = (proto == (u8) ~ 0) ? suggested_proto : proto;
if (proto == (u8) ~ 0)
return 0;
diff --git a/src/vnet/session/session_lookup.c b/src/vnet/session/session_lookup.c
index 0f9abf9aa74..4487b1c305f 100644
--- a/src/vnet/session/session_lookup.c
+++ b/src/vnet/session/session_lookup.c
@@ -233,15 +233,15 @@ stream_session_half_open_table_add (transport_connection_t * tc, u64 value)
{
make_v4_ss_kv_from_tc (&kv4, tc);
kv4.value = value;
- clib_bihash_add_del_16_8 (&sl->v4_half_open_hash, &kv4,
- 1 /* is_add */ );
+ (void) clib_bihash_add_del_16_8 (&sl->v4_half_open_hash, &kv4,
+ 1 /* is_add */ );
}
else
{
make_v6_ss_kv_from_tc (&kv6, tc);
kv6.value = value;
- clib_bihash_add_del_48_8 (&sl->v6_half_open_hash, &kv6,
- 1 /* is_add */ );
+ (void) clib_bihash_add_del_48_8 (&sl->v6_half_open_hash, &kv6,
+ 1 /* is_add */ );
}
}
diff --git a/src/vppinfra/linux/mem.c b/src/vppinfra/linux/mem.c
index 665ddf6194f..df46763ab2c 100644
--- a/src/vppinfra/linux/mem.c
+++ b/src/vppinfra/linux/mem.c
@@ -49,7 +49,7 @@ int
clib_mem_vm_get_log2_page_size (int fd)
{
struct stat st = { 0 };
- if (fstat (fd, &st))
+ if (fstat (fd, &st) == -1)
return 0;
return min_log2 (st.st_blksize);
}