Skip to content

Remove all strncpy() uses #90

@kees

Description

@kees

The strncpy() function is actively dangerous to use since it may not NUL-terminate the destination string, resulting in potential memory content exposures, unbounded reads, or crashes. Replacing uses requires some careful attention, though, since strncpy gets used also for two other cases:

  • length-bounded (not NUL-terminated) strings. For example, see the difference between NLA_STRING where the length stored separately, and NLA_NUL_STRING which uses a "traditional" NUL-terminated string. For the cases where strncpy() is used to copy non-NUL-terminated strings, the destination buffer needs to be marked with the __nonstring attribute, so that compiler diagnostics will avoid warning about cases where the character array is considered NUL-terminated.
  • destinations require trailing NUL-padding. For example, when a fixed-size buffer is used to carry a string (NUL-terminated or not) across API boundaries, like when writing an ESSIDs WiFi hardware where the driver uses a fixed-size buffer. For NUL-terminated destinations, this can be done safely with strscpy_pad(), or memcpy_and_pad() when the destination is length-bounded.

https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings

The workflow to replace strncpy(), therefore, needs to be:

  • if the destination buffer is NUL-terminated:
    • if allocation of the destination buffer is happening at the same time, refactor the code to use kstrndup()
    • if the destination buffer requires trailing NUL-padding, use strscpy_pad()
    • otherwise, use strscpy()
  • if the destination buffer is length-bounded (i.e. not required to be NUL-terminated):
    • mark the destination buffer variable (or structure member) with the __nonstring attribute and:
      • if the destination buffer requires trailing padding, use memcpy_and_pad(dst, sizeof(dst), src, min(sizeof(dst), strnlen(src, sizeof(dst)), pad_char) (perhaps this needs a macro)
      • otherwise, use memcpy(dst, src, min(sizeof(dst), strnlen(src, sizeof(dst)) (perhaps this needs a macro)

One can find instances to replace using this search: git grep '\bstrncpy\b' | grep -vE '^(Documentation|tools|samples|scripts|arch/.*/lib)/' | grep -vE 'fortify|include/linux/string.h|lib/string.c'

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions