Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 13 Jun 2019 22:20:37 -0700
From:   Kees Cook <keescook@chromium.org>
To:     Stephen Kitt <steve@sk2.org>
Cc:     corbet@lwn.net, federico.vaga@vaga.pv.it,
        linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] docs: stop suggesting strlcpy
Message-ID: <201906132220.F12C1111@keescook>
References: <20190613162548.19792-1-steve@sk2.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190613162548.19792-1-steve@sk2.org>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Jun 13, 2019 at 06:25:48PM +0200, Stephen Kitt wrote:
> Since strlcpy is deprecated, the documentation shouldn't suggest using
> it. This patch fixes the examples to use strscpy instead. It also uses
> sizeof instead of underlying constants as far as possible, to simplify
> future changes to the corresponding data structures.
> 
> Signed-off-by: Stephen Kitt <steve@sk2.org>

Yes please!

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  Documentation/hid/hid-transport.txt                         | 6 +++---
>  Documentation/i2c/instantiating-devices                     | 2 +-
>  Documentation/i2c/upgrading-clients                         | 4 ++--
>  Documentation/kernel-hacking/locking.rst                    | 6 +++---
>  Documentation/translations/it_IT/kernel-hacking/locking.rst | 6 +++---
>  5 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/hid/hid-transport.txt b/Documentation/hid/hid-transport.txt
> index 3dcba9fd4a3a..4f41d67f1b4b 100644
> --- a/Documentation/hid/hid-transport.txt
> +++ b/Documentation/hid/hid-transport.txt
> @@ -194,9 +194,9 @@ with HID core:
>  		goto err_<...>;
>  	}
>  
> -	strlcpy(hid->name, <device-name-src>, 127);
> -	strlcpy(hid->phys, <device-phys-src>, 63);
> -	strlcpy(hid->uniq, <device-uniq-src>, 63);
> +	strscpy(hid->name, <device-name-src>, sizeof(hid->name));
> +	strscpy(hid->phys, <device-phys-src>, sizeof(hid->phys));
> +	strscpy(hid->uniq, <device-uniq-src>, sizeof(hid->uniq));
>  
>  	hid->ll_driver = &custom_ll_driver;
>  	hid->bus = <device-bus>;
> diff --git a/Documentation/i2c/instantiating-devices b/Documentation/i2c/instantiating-devices
> index 0d85ac1935b7..8bc7d99133e3 100644
> --- a/Documentation/i2c/instantiating-devices
> +++ b/Documentation/i2c/instantiating-devices
> @@ -137,7 +137,7 @@ static int usb_hcd_nxp_probe(struct platform_device *pdev)
>  	(...)
>  	i2c_adap = i2c_get_adapter(2);
>  	memset(&i2c_info, 0, sizeof(struct i2c_board_info));
> -	strlcpy(i2c_info.type, "isp1301_nxp", I2C_NAME_SIZE);
> +	strscpy(i2c_info.type, "isp1301_nxp", sizeof(i2c_info.type));
>  	isp1301_i2c_client = i2c_new_probed_device(i2c_adap, &i2c_info,
>  						   normal_i2c, NULL);
>  	i2c_put_adapter(i2c_adap);
> diff --git a/Documentation/i2c/upgrading-clients b/Documentation/i2c/upgrading-clients
> index ccba3ffd6e80..96392cc5b5c7 100644
> --- a/Documentation/i2c/upgrading-clients
> +++ b/Documentation/i2c/upgrading-clients
> @@ -43,7 +43,7 @@ static int example_attach(struct i2c_adapter *adap, int addr, int kind)
>  	example->client.adapter = adap;
>  
>  	i2c_set_clientdata(&state->i2c_client, state);
> -	strlcpy(client->i2c_client.name, "example", I2C_NAME_SIZE);
> +	strscpy(client->i2c_client.name, "example", sizeof(client->i2c_client.name));
>  
>  	ret = i2c_attach_client(&state->i2c_client);
>  	if (ret < 0) {
> @@ -138,7 +138,7 @@ can be removed:
>  -	example->client.flags   = 0;
>  -	example->client.adapter = adap;
>  -
> --	strlcpy(client->i2c_client.name, "example", I2C_NAME_SIZE);
> +-	strscpy(client->i2c_client.name, "example", sizeof(client->i2c_client.name));
>  
>  The i2c_set_clientdata is now:
>  
> diff --git a/Documentation/kernel-hacking/locking.rst b/Documentation/kernel-hacking/locking.rst
> index 519673df0e82..dc698ea456e0 100644
> --- a/Documentation/kernel-hacking/locking.rst
> +++ b/Documentation/kernel-hacking/locking.rst
> @@ -451,7 +451,7 @@ to protect the cache and all the objects within it. Here's the code::
>              if ((obj = kmalloc(sizeof(*obj), GFP_KERNEL)) == NULL)
>                      return -ENOMEM;
>  
> -            strlcpy(obj->name, name, sizeof(obj->name));
> +            strscpy(obj->name, name, sizeof(obj->name));
>              obj->id = id;
>              obj->popularity = 0;
>  
> @@ -660,7 +660,7 @@ Here is the code::
>       }
>  
>      @@ -63,6 +94,7 @@
> -             strlcpy(obj->name, name, sizeof(obj->name));
> +             strscpy(obj->name, name, sizeof(obj->name));
>               obj->id = id;
>               obj->popularity = 0;
>      +        obj->refcnt = 1; /* The cache holds a reference */
> @@ -774,7 +774,7 @@ the lock is no longer used to protect the reference count itself.
>       }
>  
>      @@ -94,7 +76,7 @@
> -             strlcpy(obj->name, name, sizeof(obj->name));
> +             strscpy(obj->name, name, sizeof(obj->name));
>               obj->id = id;
>               obj->popularity = 0;
>      -        obj->refcnt = 1; /* The cache holds a reference */
> diff --git a/Documentation/translations/it_IT/kernel-hacking/locking.rst b/Documentation/translations/it_IT/kernel-hacking/locking.rst
> index 0ef31666663b..5fd8a1abd2be 100644
> --- a/Documentation/translations/it_IT/kernel-hacking/locking.rst
> +++ b/Documentation/translations/it_IT/kernel-hacking/locking.rst
> @@ -468,7 +468,7 @@ e tutti gli oggetti che contiene. Ecco il codice::
>              if ((obj = kmalloc(sizeof(*obj), GFP_KERNEL)) == NULL)
>                      return -ENOMEM;
>  
> -            strlcpy(obj->name, name, sizeof(obj->name));
> +            strscpy(obj->name, name, sizeof(obj->name));
>              obj->id = id;
>              obj->popularity = 0;
>  
> @@ -678,7 +678,7 @@ Ecco il codice::
>       }
>  
>      @@ -63,6 +94,7 @@
> -             strlcpy(obj->name, name, sizeof(obj->name));
> +             strscpy(obj->name, name, sizeof(obj->name));
>               obj->id = id;
>               obj->popularity = 0;
>      +        obj->refcnt = 1; /* The cache holds a reference */
> @@ -792,7 +792,7 @@ contatore stesso.
>       }
>  
>      @@ -94,7 +76,7 @@
> -             strlcpy(obj->name, name, sizeof(obj->name));
> +             strscpy(obj->name, name, sizeof(obj->name));
>               obj->id = id;
>               obj->popularity = 0;
>      -        obj->refcnt = 1; /* The cache holds a reference */
> -- 
> 2.11.0
> 

-- 
Kees Cook