[lttng-dev] [PATCH lttng-tools 01/24] Implement lttng_strncpy safe string copy

[Mathieu Desnoyers]

----- On May 17, 2016, at 11:55 AM, Jeremie Galarneau jeremie.galarneau at efficios.com wrote:

> On Mon, May 16, 2016 at 9:42 PM, Mathieu Desnoyers
> <mathieu.desnoyers at efficios.com> wrote:
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
>> ---
>>  src/common/macros.h | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/src/common/macros.h b/src/common/macros.h
>> index 308d3d5..6147eff 100644
>> --- a/src/common/macros.h
>> +++ b/src/common/macros.h
>> @@ -20,6 +20,7 @@
>>  #define _MACROS_H
>>
>>  #include <stdlib.h>
>> +#include <string.h>
>>
>>  /*
>>   * Takes a pointer x and transform it so we can use it to access members
>> @@ -76,4 +77,25 @@ void *zmalloc(size_t len)
>>  #define LTTNG_HIDDEN __attribute__((visibility("hidden")))
>>  #endif
>>
>> +/*
>> + * lttng_strncpy returns 0 on success, or nonzero on failure.
>> + * It checks that the @src string fits into @dest_len before performing
>> + * the copy. On failure, no copy has been performed.
> 
> Added comment to mention that dest_len includes the NULL delimiter.
> 
>> + */
>> +static inline
>> +int lttng_strncpy(char *dest, const char *src, size_t dest_len)
>> +{
>> +       if (strlen(src) >= dest_len) {
> 
> Switching strlen() to strnlen() to protect against cases such as in
> "Fix: illegal memory access in init_ust_event_from_agent_event" where
> the source may not be NULL-terminated (even though it should be). I
> don't want to change the behavior this close to the release.
> 
> A lot of code currently assumes names/identifiers to be bounded by
> LTTNG_SYMBOL_NAME_LEN which could cause the strlen() run
> to overrun here.

Good idea! thanks!

Mathieu

> 
> Jérémie
> 
>> +               return -1;
>> +       }
>> +       strncpy(dest, src, dest_len);
>> +       /*
>> +        * Be extra careful and put final \0 at the end after strncpy(),
>> +        * even though we checked the length before. This makes Coverity
>> +        * happy.
>> +        */
>> +       dest[dest_len - 1] = '\0';
>> +       return 0;
>> +}
>> +
>>  #endif /* _MACROS_H */
>> --
>> 2.1.4
>>
> 
> 
> 
> --
> Jérémie Galarneau
> EfficiOS Inc.
> http://www.efficios.com

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com