Skip to content

Commit

Permalink
tile gxio: use better string copy primitive
Browse files Browse the repository at this point in the history
Both strncpy and strlcpy suffer from the fact that they do
partial copies of strings into the destination when the target
buffer is too small.  This is frequently pointless since an
overflow of the target buffer may make the result invalid.

strncpy() makes it relatively hard to even detect the error
condition, and with strlcpy() you have to duplicate the buffer
size parameter to test to see if the result exceeds it.
By returning zero in the failure case, we both make testing
for it easy, and by simply not copying anything in that case,
we make it mandatory for callers to test the error code.

To catch lazy programmers who don't check, we also place a NUL at
the start of the destination buffer (if there is space) to
ensure that the result is an invalid string.

At some point it may make sense to promote strscpy() to
a global platform-independent function, but other than the
reviewers, no one was interested on LKML, so for now leave
the strscpy() function as file-static.

Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
Reviewed-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>
  • Loading branch information
cmetcalf-tilera committed Oct 2, 2014
1 parent 514b82a commit bceb7ef
Showing 1 changed file with 32 additions and 5 deletions.
37 changes: 32 additions & 5 deletions arch/tile/gxio/mpipe.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,32 @@
/* HACK: Avoid pointless "shadow" warnings. */
#define link link_shadow

/**
* strscpy - Copy a C-string into a sized buffer, but only if it fits
* @dest: Where to copy the string to
* @src: Where to copy the string from
* @size: size of destination buffer
*
* Use this routine to avoid copying too-long strings.
* The routine returns the total number of bytes copied
* (including the trailing NUL) or zero if the buffer wasn't
* big enough. To ensure that programmers pay attention
* to the return code, the destination has a single NUL
* written at the front (if size is non-zero) when the
* buffer is not big enough.
*/
static size_t strscpy(char *dest, const char *src, size_t size)
{
size_t len = strnlen(src, size) + 1;
if (len > size) {
if (size)
dest[0] = '\0';
return 0;
}
memcpy(dest, src, len);
return len;
}

int gxio_mpipe_init(gxio_mpipe_context_t *context, unsigned int mpipe_index)
{
char file[32];
Expand Down Expand Up @@ -511,8 +537,8 @@ int gxio_mpipe_link_instance(const char *link_name)
if (!context)
return GXIO_ERR_NO_DEVICE;

strncpy(name.name, link_name, sizeof(name.name));
name.name[GXIO_MPIPE_LINK_NAME_LEN - 1] = '\0';
if (strscpy(name.name, link_name, sizeof(name.name)) == 0)
return GXIO_ERR_NO_DEVICE;

return gxio_mpipe_info_instance_aux(context, name);
}
Expand All @@ -529,7 +555,8 @@ int gxio_mpipe_link_enumerate_mac(int idx, char *link_name, uint8_t *link_mac)

rv = gxio_mpipe_info_enumerate_aux(context, idx, &name, &mac);
if (rv >= 0) {
strncpy(link_name, name.name, sizeof(name.name));
if (strscpy(link_name, name.name, sizeof(name.name)) == 0)
return GXIO_ERR_INVAL_MEMORY_SIZE;
memcpy(link_mac, mac.mac, sizeof(mac.mac));
}

Expand All @@ -545,8 +572,8 @@ int gxio_mpipe_link_open(gxio_mpipe_link_t *link,
_gxio_mpipe_link_name_t name;
int rv;

strncpy(name.name, link_name, sizeof(name.name));
name.name[GXIO_MPIPE_LINK_NAME_LEN - 1] = '\0';
if (strscpy(name.name, link_name, sizeof(name.name)) == 0)
return GXIO_ERR_NO_DEVICE;

rv = gxio_mpipe_link_open_aux(context, name, flags);
if (rv < 0)
Expand Down

0 comments on commit bceb7ef

Please sign in to comment.