[ovs-dev] [coverity 11/14] lib: Replace remaining unsafe strncpy()s with ovs_strlcpy().

Ben Pfaff blp at nicira.com
Tue Feb 22 17:54:00 UTC 2011


On Mon, Feb 21, 2011 at 05:44:54PM -0800, Justin Pettit wrote:
> --- a/lib/ofp-print.c
> +++ b/lib/ofp-print.c
> @@ -1315,7 +1315,7 @@ ofp_print_ofpst_table_reply(struct ds *string, const struct ofp_header *oh,
>  
>      for (; n--; ts++) {
>          char name[OFP_MAX_TABLE_NAME_LEN + 1];
> -        strncpy(name, ts->name, sizeof name);
> +        ovs_strlcpy(name, ts->name, sizeof name);
>          name[OFP_MAX_TABLE_NAME_LEN] = '\0';

I don't see any bug in the original code, but this actually introduces a
new bug: if ts->name does not have a null terminator, then ovs_strlcpy()
will access past the end of ts->name.

I guess that you could change ovs_strlcpy() to use strnlen() instead of
strlen(), in which case this change would be a code improvement.  (We'd
need to check for and replace strnlen() if it was missing, since it is
new in POSIX 2008.)

The same goes for the change in name_table_reset() in route-table.c.

I don't have any objection to the change to route_table_get_name() in
route-table.c.

In make_sockaddr_un__() in socket-util.c, strncpy() is intentionally
used to make sure that bytes past the end of the sun_path string are
zeroed.  This keeps the kernel from inventing creative interpretations
of those bytes and might suppress valgrind warnings (dunno).  If you
wanted to change the function to memset the whole sockaddr_un at the top
instead, then use ovs_strlcpy(), that's fine with me too.




More information about the dev mailing list