lkml.org 
[lkml]   [2023]   [Oct]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    Subject[PATCH] atmel: replace deprecated strncpy/strcpy with strscpy
    From
    strncpy() is deprecated for use on NUL-terminated destination strings
    [1] and as such we should prefer more robust and less ambiguous string
    interfaces. strcpy() is also deprecated [2] and shouldn't be used.

    We expect priv->firmware_id to be NUL-terminated based on its usage
    with seq_printf() and strlen() in atmel.c:
    1420 | seq_printf(m, "%s loaded by host\n", priv->firmware_id);
    ...
    3884 | if (strlen(priv->firmware_id) == 0) {

    NUL-padding is not required, which is evident by the usage of a plain
    strcpy():
    3891 | strcpy(priv->firmware_id, "atmel_at76c502.bin");

    Considering the above, a suitable replacement is `strscpy` [2] due to
    the fact that it guarantees NUL-termination on the destination buffer
    without unnecessarily NUL-padding.

    Let's also replace hard-coded lengths to be `sizeof(...)` for buffers
    that the compiler can detect the size for as this is less error prone.

    Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
    Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strcpy [2]
    Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [3]
    Link: https://github.com/KSPP/linux/issues/90
    Cc: linux-hardening@vger.kernel.org
    Signed-off-by: Justin Stitt <justinstitt@google.com>
    ---
    Note: build-tested only.

    Found with: $ rg "strncpy\("
    ---
    drivers/net/wireless/atmel/atmel.c | 8 ++++----
    1 file changed, 4 insertions(+), 4 deletions(-)

    diff --git a/drivers/net/wireless/atmel/atmel.c b/drivers/net/wireless/atmel/atmel.c
    index 7c2d1c588156..199b9144ef63 100644
    --- a/drivers/net/wireless/atmel/atmel.c
    +++ b/drivers/net/wireless/atmel/atmel.c
    @@ -2011,7 +2011,7 @@ static int atmel_get_name(struct net_device *dev,
    union iwreq_data *wrqu,
    char *extra)
    {
    - strcpy(wrqu->name, "IEEE 802.11-DS");
    + strscpy(wrqu->name, "IEEE 802.11-DS", sizeof(wrqu->name));
    return 0;
    }

    @@ -2651,8 +2651,7 @@ static int atmel_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)

    priv->firmware = new_firmware;
    priv->firmware_length = com.len;
    - strncpy(priv->firmware_id, com.id, 31);
    - priv->firmware_id[31] = '\0';
    + strscpy(priv->firmware_id, com.id, sizeof(priv->firmware_id));
    break;

    case ATMELRD:
    @@ -3889,7 +3888,8 @@ static int reset_atmel_card(struct net_device *dev)
    printk(KERN_INFO
    "%s: if not, use the firmware= module parameter.\n",
    dev->name);
    - strcpy(priv->firmware_id, "atmel_at76c502.bin");
    + strscpy(priv->firmware_id, "atmel_at76c502.bin",
    + sizeof(priv->firmware_id));
    }
    err = request_firmware(&fw_entry, priv->firmware_id, priv->sys_dev);
    if (err != 0) {
    ---
    base-commit: cbf3a2cb156a2c911d8f38d8247814b4c07f49a2
    change-id: 20231016-strncpy-drivers-net-wireless-atmel-atmel-c-7ca951cf7cfa
    Best regards,
    --
    Justin Stitt <justinstitt@google.com>

    \
     
     \ /
      Last update: 2023-10-16 22:24    [W:3.309 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site