• Justin Stitt's avatar
    md: replace deprecated strncpy with memcpy · ceb04163
    Justin Stitt authored
    `strncpy` is deprecated for use on NUL-terminated destination strings
    [1] and as such we should prefer more robust and less ambiguous string
    interfaces.
    
    There are three such strncpy uses that this patch addresses:
    
    The respective destination buffers are:
    1) mddev->clevel
    2) clevel
    3) mddev->metadata_type
    
    We expect mddev->clevel to be NUL-terminated due to its use with format
    strings:
    |       ret = sprintf(page, "%s\n", mddev->clevel);
    
    Furthermore, we can see that mddev->clevel is not expected to be
    NUL-padded as `md_clean()` merely set's its first byte to NULL -- not
    the entire buffer:
    |       static void md_clean(struct mddev *mddev)
    |       {
    |       	mddev->array_sectors = 0;
    |       	mddev->external_size = 0;
    |               ...
    |       	mddev->level = LEVEL_NONE;
    |       	mddev->clevel[0] = 0;
    |               ...
    
    A suitable replacement for this instance is `memcpy` as we know the
    number of bytes to copy and perform manual NUL-termination at a
    specified offset. This really decays to just a byte copy from one buffer
    to another. `strscpy` is also a considerable replacement but using
    `slen` as the length argument would result in truncation of the last
    byte unless something like `slen + 1` was provided which isn't the most
    idiomatic strscpy usage.
    
    For the next case, the destination buffer `clevel` is expected to be
    NUL-terminated based on its usage within kstrtol() which expects
    NUL-terminated strings. Note that, in context, this code removes a
    trailing newline which is seemingly not required as kstrtol() can handle
    trailing newlines implicitly. However, there exists further usage of
    clevel (or buf) that would also like to have the newline removed. All in
    all, with similar reasoning to the first case, let's just use memcpy as
    this is just a byte copy and NUL-termination is handled manually.
    
    The third and final case concerning `mddev->metadata_type` is more or
    less the same as the other two. We expect that it be NUL-terminated
    based on its usage with seq_printf:
    |       seq_printf(seq, " super external:%s",
    |       	   mddev->metadata_type);
    ... and we can surmise that NUL-padding isn't required either due to how
    it is handled in md_clean():
    |       static void md_clean(struct mddev *mddev)
    |       {
    |       ...
    |       mddev->metadata_type[0] = 0;
    |       ...
    
    So really, all these instances have precisely calculated lengths and
    purposeful NUL-termination so we can just use memcpy to remove ambiguity
    surrounding strncpy.
    
    Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
    Link: https://github.com/KSPP/linux/issues/90
    Cc: linux-hardening@vger.kernel.org
    Signed-off-by: default avatarJustin Stitt <justinstitt@google.com>
    Reviewed-by: default avatarKees Cook <keescook@chromium.org>
    Signed-off-by: default avatarSong Liu <song@kernel.org>
    Link: https://lore.kernel.org/r/20230925-strncpy-drivers-md-md-c-v1-1-2b0093b89c2b@google.com
    ceb04163
md.c 263 KB