Skip to content

Commit

Permalink
lib, ovs-vsctl: Add zero-initializations.
Browse files Browse the repository at this point in the history
This commit adds zero-initializations by changing `SFL_ALLOC` from
`malloc` to `xzalloc`, adding a `memset` call to `sflAlloc`,
initializing a `pollfd` struct variable with zeroes, and changing some
calls to `xmalloc` to `xzalloc`. This is to prevent potential data leaks
or undefined behavior from potentially uninitialized variables.

Some variables would always be initialized by either the code flow or
the compiler. Thus, some of the associated Coverity reports might be
false positives. That said, it is still considered best practice to
zero-initialize variables upfront just in case to ensure the overall
resilience and security of OVS, as long as they do not impact
performance-critical code. As a bonus, it would also make static
analyzer tools, such as Coverity, happy.

Reviewed-by: Simon Horman <simon.horman@corigine.com>
Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
  • Loading branch information
jamestiotio authored and igsilya committed Sep 1, 2023
1 parent 1116459 commit 40546cd
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 7 deletions.
2 changes: 1 addition & 1 deletion lib/latch-unix.c
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ latch_set(struct latch *latch)
bool
latch_is_set(const struct latch *latch)
{
struct pollfd pfd;
struct pollfd pfd = {0};
int retval;

pfd.fd = latch->fds[0];
Expand Down
12 changes: 10 additions & 2 deletions lib/sflow_agent.c
Original file line number Diff line number Diff line change
Expand Up @@ -510,8 +510,16 @@ void sfl_agent_sysError(SFLAgent *agent, char *modName, char *msg)

static void * sflAlloc(SFLAgent *agent, size_t bytes)
{
if(agent->allocFn) return (*agent->allocFn)(agent->magic, agent, bytes);
else return SFL_ALLOC(bytes);
void *alloc;

if (agent->allocFn) {
alloc = (*agent->allocFn)(agent->magic, agent, bytes);
ovs_assert(alloc);
memset(alloc, 0, bytes);
} else {
alloc = SFL_ALLOC(bytes);
}
return alloc;
}

static void sflFree(SFLAgent *agent, void *obj)
Expand Down
2 changes: 1 addition & 1 deletion lib/sflow_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ void sfl_agent_sysError(SFLAgent *agent, char *modName, char *msg);

u_int32_t sfl_receiver_samplePacketsSent(SFLReceiver *receiver);

#define SFL_ALLOC malloc
#define SFL_ALLOC xzalloc
#define SFL_FREE free

#endif /* SFLOW_API_H */
Expand Down
8 changes: 5 additions & 3 deletions utilities/ovs-vsctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -575,15 +575,18 @@ add_bridge_to_cache(struct vsctl_context *vsctl_ctx,
struct ovsrec_bridge *br_cfg, const char *name,
struct vsctl_bridge *parent, int vlan)
{
struct vsctl_bridge *br = xmalloc(sizeof *br);
struct vsctl_bridge *br = xzalloc(sizeof *br);

br->br_cfg = br_cfg;
br->name = xstrdup(name);
ovs_list_init(&br->ports);
br->parent = parent;
br->vlan = vlan;
hmap_init(&br->children);

if (parent) {
struct vsctl_bridge *conflict = find_vlan_bridge(parent, vlan);

if (conflict) {
VLOG_WARN("%s: bridge has multiple VLAN bridges (%s and %s) "
"for VLAN %d, but only one is allowed",
Expand Down Expand Up @@ -659,7 +662,7 @@ static struct vsctl_port *
add_port_to_cache(struct vsctl_context *vsctl_ctx, struct vsctl_bridge *parent,
struct ovsrec_port *port_cfg)
{
struct vsctl_port *port;
struct vsctl_port *port = xzalloc(sizeof *port);

if (port_cfg->tag
&& *port_cfg->tag >= 0 && *port_cfg->tag <= 4095) {
Expand All @@ -671,7 +674,6 @@ add_port_to_cache(struct vsctl_context *vsctl_ctx, struct vsctl_bridge *parent,
}
}

port = xmalloc(sizeof *port);
ovs_list_push_back(&parent->ports, &port->ports_node);
ovs_list_init(&port->ifaces);
port->port_cfg = port_cfg;
Expand Down

0 comments on commit 40546cd

Please sign in to comment.