[RFC][PATCH v1] Fix and rename nla_strlcpy to nla_strcpy

[RFC][PATCH v1] Fix and rename nla_strlcpy to nla_strcpy

[laniel_francis]

First, I do hope you are all fine and the same for your relatives.

This 3 patches set answers to three first steps of this issue:
https://github.com/KSPP/linux/issues/110
Basically, I fix an inefficiency in nla_strlcpy where memset was called before
memcpy.
Then I modified the returned value to follow strscpy behavior and update where
this function is called and returned value is checked.
Finally, I rename nla_strlcpy to nla_strcpy and propagate the modifications in
all files where nla_strlcpy was called.

Unfortunately, I did not understand how to instanciate struct nlattr objects so
I tested the modified function by calling it directly on char *, this is why I
marked this patch as RFC.

If you have any remarks, feel free to share them so I can improve the code.

Here are the stats:
 drivers/infiniband/core/nldev.c            | 10 +++++-----
 drivers/misc/lkdtm/Makefile                |  1 +
 drivers/net/can/vxcan.c                    |  4 ++--
 drivers/net/veth.c                         |  4 ++--
 include/linux/genl_magic_struct.h          |  2 +-
 include/net/netlink.h                      |  4 ++--
 include/net/pkt_cls.h                      |  3 ++-
 kernel/taskstats.c                         |  2 +-
 lib/nlattr.c                               | 35 ++++++++++++++++++++++-------------
 net/core/fib_rules.c                       |  4 ++--
 net/core/rtnetlink.c                       | 12 ++++++------
 net/decnet/dn_dev.c                        |  2 +-
 net/ieee802154/nl-mac.c                    |  2 +-
 net/ipv4/devinet.c                         |  2 +-
 net/ipv4/fib_semantics.c                   |  2 +-
 net/ipv4/metrics.c                         |  2 +-
 net/netfilter/ipset/ip_set_hash_netiface.c |  4 ++--
 net/netfilter/nf_tables_api.c              |  6 +++---
 net/netfilter/nfnetlink_acct.c             |  2 +-
 net/netfilter/nfnetlink_cthelper.c         |  4 ++--
 net/netfilter/nft_ct.c                     |  2 +-
 net/netfilter/nft_log.c                    |  2 +-
 net/netlabel/netlabel_mgmt.c               |  2 +-
 net/nfc/netlink.c                          |  2 +-
 net/sched/act_api.c                        |  2 +-
 net/sched/act_ipt.c                        |  2 +-
 net/sched/act_simple.c                     |  4 ++--
 net/sched/cls_api.c                        |  2 +-
 net/sched/sch_api.c                        |  2 +-
 net/tipc/netlink_compat.c                  |  2 +-
 30 files changed, 70 insertions(+), 59 deletions(-)

Best regards.

[PATCH v1 1/3] Fix unefficient call to memset before memcpu in nla_strlcpy.

[laniel_francis]

From: Francis Laniel <laniel_francis@privacyrequired.com>

This patch solves part 1 of issue: https://github.com/KSPP/linux/issues/110

Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
---
 lib/nlattr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/nlattr.c b/lib/nlattr.c
index 74019c8ebf6b..ab96a5f4b9b8 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -731,8 +731,8 @@ size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
 	if (dstsize > 0) {
 		size_t len = (srclen >= dstsize) ? dstsize - 1 : srclen;
 
-		memset(dst, 0, dstsize);
 		memcpy(dst, src, len);
+		dst[len] = '\0';
 	}
 
 	return srclen;
-- 
2.20.1

Re: [PATCH v1 1/3] Fix unefficient call to memset before memcpu in nla_strlcpy.

[Kees Cook]

On Fri, Oct 16, 2020 at 02:52:14PM +0200, laniel_francis@privacyrequired.com wrote:
> From: Francis Laniel <laniel_francis@privacyrequired.com>
> 
> This patch solves part 1 of issue: https://github.com/KSPP/linux/issues/110
> 
> Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
> ---
>  lib/nlattr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/nlattr.c b/lib/nlattr.c
> index 74019c8ebf6b..ab96a5f4b9b8 100644
> --- a/lib/nlattr.c
> +++ b/lib/nlattr.c
> @@ -731,8 +731,8 @@ size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
>  	if (dstsize > 0) {
>  		size_t len = (srclen >= dstsize) ? dstsize - 1 : srclen;
>  
> -		memset(dst, 0, dstsize);
>  		memcpy(dst, src, len);
> +		dst[len] = '\0';

I don't think this is right: callers are likely depending on the entire
destination buffer to be zero-padded. I think you probably want:

		memset(dst + len, 0, dstsize - len);
  		memcpy(dst, src, len);

(but double-check my pointer math)

>  	}
>  
>  	return srclen;
> -- 
> 2.20.1
> 

-- 
Kees Cook

Re: [PATCH v1 1/3] Fix unefficient call to memset before memcpu in nla_strlcpy.

[Francis Laniel]

Le samedi 17 octobre 2020, 01:19:59 CEST Kees Cook a écrit :
> On Fri, Oct 16, 2020 at 02:52:14PM +0200, laniel_francis@privacyrequired.com 
wrote:
> > From: Francis Laniel <laniel_francis@privacyrequired.com>
> > 
> > This patch solves part 1 of issue:
> > https://github.com/KSPP/linux/issues/110
> > 
> > Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
> > ---
> > 
> >  lib/nlattr.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/lib/nlattr.c b/lib/nlattr.c
> > index 74019c8ebf6b..ab96a5f4b9b8 100644
> > --- a/lib/nlattr.c
> > +++ b/lib/nlattr.c
> > @@ -731,8 +731,8 @@ size_t nla_strlcpy(char *dst, const struct nlattr
> > *nla, size_t dstsize)> 
> >  	if (dstsize > 0) {
> >  	
> >  		size_t len = (srclen >= dstsize) ? dstsize - 1 : srclen;
> > 
> > -		memset(dst, 0, dstsize);
> > 
> >  		memcpy(dst, src, len);
> > 
> > +		dst[len] = '\0';
> 
> I don't think this is right: callers are likely depending on the entire
> destination buffer to be zero-padded. I think you probably want:
> 
> 		memset(dst + len, 0, dstsize - len);
>   		memcpy(dst, src, len);
> 
> (but double-check my pointer math)
> 

I did not understand that the content has to be zero-padded, now your comment 
on github is clearer!
I will modifiy it for next version and check the math on the paper.

> >  	}
> >  	
> >  	return srclen;

Re: [PATCH v1 1/3] Fix unefficient call to memset before memcpu in nla_strlcpy.

[Jann Horn]

On Fri, Oct 16, 2020 at 3:02 PM <laniel_francis@privacyrequired.com> wrote:
> This patch solves part 1 of issue: https://github.com/KSPP/linux/issues/110
>
> Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
> ---
>  lib/nlattr.c | 2 +-

Your patch touches networking-related code; please include the
"NETWORKING [GENERAL]" contacts listed in MAINTAINERS when sending the
next version of your patch series.

Re: [PATCH v1 1/3] Fix unefficient call to memset before memcpu in nla_strlcpy.

[Francis Laniel]

Le samedi 17 octobre 2020, 01:29:12 CEST Jann Horn a écrit :
> On Fri, Oct 16, 2020 at 3:02 PM <laniel_francis@privacyrequired.com> wrote:
> > This patch solves part 1 of issue:
> > https://github.com/KSPP/linux/issues/110
> > 
> > Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
> > ---
> > 
> >  lib/nlattr.c | 2 +-
> 
> Your patch touches networking-related code; please include the
> "NETWORKING [GENERAL]" contacts listed in MAINTAINERS when sending the
> next version of your patch series.

I will add the networking maintainers to the next version.

[PATCH v1 2/3] Modify return value of nla_strlcpy to match that of strscpy.

[laniel_francis]

From: Francis Laniel <laniel_francis@privacyrequired.com>

This patch solves part 2 of issue: https://github.com/KSPP/linux/issues/110

Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
---
 include/net/netlink.h |  2 +-
 include/net/pkt_cls.h |  3 ++-
 lib/nlattr.c          | 31 ++++++++++++++++++++-----------
 net/sched/act_api.c   |  2 +-
 net/sched/sch_api.c   |  2 +-
 5 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/include/net/netlink.h b/include/net/netlink.h
index 7356f41d23ba..446ca182e13d 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -506,7 +506,7 @@ int __nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
 		struct netlink_ext_ack *extack);
 int nla_policy_len(const struct nla_policy *, int);
 struct nlattr *nla_find(const struct nlattr *head, int len, int attrtype);
-size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize);
+ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize);
 char *nla_strdup(const struct nlattr *nla, gfp_t flags);
 int nla_memcpy(void *dest, const struct nlattr *src, int count);
 int nla_memcmp(const struct nlattr *nla, const void *data, size_t size);
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index d4d461236351..f42db07c399b 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -4,6 +4,7 @@
 
 #include <linux/pkt_cls.h>
 #include <linux/workqueue.h>
+#include <linux/errno.h>
 #include <net/sch_generic.h>
 #include <net/act_api.h>
 #include <net/net_namespace.h>
@@ -512,7 +513,7 @@ tcf_change_indev(struct net *net, struct nlattr *indev_tlv,
 	char indev[IFNAMSIZ];
 	struct net_device *dev;
 
-	if (nla_strlcpy(indev, indev_tlv, IFNAMSIZ) >= IFNAMSIZ) {
+	if (nla_strlcpy(indev, indev_tlv, IFNAMSIZ) == -E2BIG) {
 		NL_SET_ERR_MSG_ATTR(extack, indev_tlv,
 				    "Interface name too long");
 		return -EINVAL;
diff --git a/lib/nlattr.c b/lib/nlattr.c
index ab96a5f4b9b8..83dd233bbe3e 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -713,29 +713,38 @@ EXPORT_SYMBOL(nla_find);
  * @dst: where to copy the string to
  * @nla: attribute to copy the string from
  * @dstsize: size of destination buffer
+ * @returns: -E2BIG if @dstsize is 0 or source buffer length greater than
+ * @dstsize, otherwise it returns the number of copied characters (not
+ * including the trailing %NUL).
  *
  * Copies at most dstsize - 1 bytes into the destination buffer.
- * The result is always a valid NUL-terminated string. Unlike
- * strlcpy the destination buffer is always padded out.
- *
- * Returns the length of the source buffer.
+ * The result is always a valid NUL-terminated string.
  */
-size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
+ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
 {
+	size_t len;
+	ssize_t ret;
 	size_t srclen = nla_len(nla);
 	char *src = nla_data(nla);
 
+	if (dstsize == 0 || WARN_ON_ONCE(dstsize > INT_MAX))
+		return -E2BIG;
+
 	if (srclen > 0 && src[srclen - 1] == '\0')
 		srclen--;
 
-	if (dstsize > 0) {
-		size_t len = (srclen >= dstsize) ? dstsize - 1 : srclen;
-
-		memcpy(dst, src, len);
-		dst[len] = '\0';
+	if (srclen >= dstsize) {
+		len = dstsize - 1;
+		ret = -E2BIG;
+	} else {
+		len = srclen;
+		ret = len;
 	}
 
-	return srclen;
+	memcpy(dst, src, len);
+	dst[len] = '\0';
+
+	return ret;
 }
 EXPORT_SYMBOL(nla_strlcpy);
 
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index f66417d5d2c3..ed411d6c29fc 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -935,7 +935,7 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
 			NL_SET_ERR_MSG(extack, "TC action kind must be specified");
 			goto err_out;
 		}
-		if (nla_strlcpy(act_name, kind, IFNAMSIZ) >= IFNAMSIZ) {
+		if (nla_strlcpy(act_name, kind, IFNAMSIZ) == -E2BIG) {
 			NL_SET_ERR_MSG(extack, "TC action name too long");
 			goto err_out;
 		}
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 2a76a2f5ed88..3e89c666d1e8 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1170,7 +1170,7 @@ static struct Qdisc *qdisc_create(struct net_device *dev,
 #ifdef CONFIG_MODULES
 	if (ops == NULL && kind != NULL) {
 		char name[IFNAMSIZ];
-		if (nla_strlcpy(name, kind, IFNAMSIZ) < IFNAMSIZ) {
+		if (nla_strlcpy(name, kind, IFNAMSIZ) != -E2BIG) {
 			/* We dropped the RTNL semaphore in order to
 			 * perform the module load.  So, even if we
 			 * succeeded in loading the module we have to
-- 
2.20.1

Re: [PATCH v1 2/3] Modify return value of nla_strlcpy to match that of strscpy.

[Kees Cook]

On Fri, Oct 16, 2020 at 02:52:15PM +0200, laniel_francis@privacyrequired.com wrote:
> From: Francis Laniel <laniel_francis@privacyrequired.com>
> 
> This patch solves part 2 of issue: https://github.com/KSPP/linux/issues/110

As with the others, this needs a full commit log (imagine someone
doesn't have access to read that issue, etc).

> 
> Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
> ---
>  include/net/netlink.h |  2 +-
>  include/net/pkt_cls.h |  3 ++-
>  lib/nlattr.c          | 31 ++++++++++++++++++++-----------
>  net/sched/act_api.c   |  2 +-
>  net/sched/sch_api.c   |  2 +-
>  5 files changed, 25 insertions(+), 15 deletions(-)
> 
> diff --git a/include/net/netlink.h b/include/net/netlink.h
> index 7356f41d23ba..446ca182e13d 100644
> --- a/include/net/netlink.h
> +++ b/include/net/netlink.h
> @@ -506,7 +506,7 @@ int __nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
>  		struct netlink_ext_ack *extack);
>  int nla_policy_len(const struct nla_policy *, int);
>  struct nlattr *nla_find(const struct nlattr *head, int len, int attrtype);
> -size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize);
> +ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize);
>  char *nla_strdup(const struct nlattr *nla, gfp_t flags);
>  int nla_memcpy(void *dest, const struct nlattr *src, int count);
>  int nla_memcmp(const struct nlattr *nla, const void *data, size_t size);
> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
> index d4d461236351..f42db07c399b 100644
> --- a/include/net/pkt_cls.h
> +++ b/include/net/pkt_cls.h
> @@ -4,6 +4,7 @@
>  
>  #include <linux/pkt_cls.h>
>  #include <linux/workqueue.h>
> +#include <linux/errno.h>
>  #include <net/sch_generic.h>
>  #include <net/act_api.h>
>  #include <net/net_namespace.h>
> @@ -512,7 +513,7 @@ tcf_change_indev(struct net *net, struct nlattr *indev_tlv,
>  	char indev[IFNAMSIZ];
>  	struct net_device *dev;
>  
> -	if (nla_strlcpy(indev, indev_tlv, IFNAMSIZ) >= IFNAMSIZ) {
> +	if (nla_strlcpy(indev, indev_tlv, IFNAMSIZ) == -E2BIG) {

I would make these tests all be "< 0" rather than specifically -E2BIG.

>  		NL_SET_ERR_MSG_ATTR(extack, indev_tlv,
>  				    "Interface name too long");
>  		return -EINVAL;
> diff --git a/lib/nlattr.c b/lib/nlattr.c
> index ab96a5f4b9b8..83dd233bbe3e 100644
> --- a/lib/nlattr.c
> +++ b/lib/nlattr.c
> @@ -713,29 +713,38 @@ EXPORT_SYMBOL(nla_find);
>   * @dst: where to copy the string to
>   * @nla: attribute to copy the string from
>   * @dstsize: size of destination buffer
> + * @returns: -E2BIG if @dstsize is 0 or source buffer length greater than
> + * @dstsize, otherwise it returns the number of copied characters (not
> + * including the trailing %NUL).
>   *
>   * Copies at most dstsize - 1 bytes into the destination buffer.
> - * The result is always a valid NUL-terminated string. Unlike
> - * strlcpy the destination buffer is always padded out.
> - *
> - * Returns the length of the source buffer.
> + * The result is always a valid NUL-terminated string.
>   */
> -size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
> +ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
>  {
> +	size_t len;
> +	ssize_t ret;
>  	size_t srclen = nla_len(nla);
>  	char *src = nla_data(nla);
>  
> +	if (dstsize == 0 || WARN_ON_ONCE(dstsize > INT_MAX))
> +		return -E2BIG;

Cool, yeah, good to include.

> +
>  	if (srclen > 0 && src[srclen - 1] == '\0')
>  		srclen--;
>  
> -	if (dstsize > 0) {
> -		size_t len = (srclen >= dstsize) ? dstsize - 1 : srclen;
> -
> -		memcpy(dst, src, len);
> -		dst[len] = '\0';
> +	if (srclen >= dstsize) {
> +		len = dstsize - 1;
> +		ret = -E2BIG;
> +	} else {
> +		len = srclen;
> +		ret = len;
>  	}
>  
> -	return srclen;
> +	memcpy(dst, src, len);
> +	dst[len] = '\0';
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL(nla_strlcpy);

Otherwise, I think this looks good. (Though same note about
zero-padding.)

>  
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index f66417d5d2c3..ed411d6c29fc 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -935,7 +935,7 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
>  			NL_SET_ERR_MSG(extack, "TC action kind must be specified");
>  			goto err_out;
>  		}
> -		if (nla_strlcpy(act_name, kind, IFNAMSIZ) >= IFNAMSIZ) {
> +		if (nla_strlcpy(act_name, kind, IFNAMSIZ) == -E2BIG) {
>  			NL_SET_ERR_MSG(extack, "TC action name too long");
>  			goto err_out;
>  		}
> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> index 2a76a2f5ed88..3e89c666d1e8 100644
> --- a/net/sched/sch_api.c
> +++ b/net/sched/sch_api.c
> @@ -1170,7 +1170,7 @@ static struct Qdisc *qdisc_create(struct net_device *dev,
>  #ifdef CONFIG_MODULES
>  	if (ops == NULL && kind != NULL) {
>  		char name[IFNAMSIZ];
> -		if (nla_strlcpy(name, kind, IFNAMSIZ) < IFNAMSIZ) {
> +		if (nla_strlcpy(name, kind, IFNAMSIZ) != -E2BIG) {
>  			/* We dropped the RTNL semaphore in order to
>  			 * perform the module load.  So, even if we
>  			 * succeeded in loading the module we have to
> -- 
> 2.20.1
> 

-- 
Kees Cook

Re: [PATCH v1 2/3] Modify return value of nla_strlcpy to match that of strscpy.

[Francis Laniel]

Le samedi 17 octobre 2020, 01:23:10 CEST Kees Cook a écrit :
> On Fri, Oct 16, 2020 at 02:52:15PM +0200, laniel_francis@privacyrequired.com 
wrote:
> > From: Francis Laniel <laniel_francis@privacyrequired.com>
> > 
> > This patch solves part 2 of issue:
> > https://github.com/KSPP/linux/issues/110
> 
> As with the others, this needs a full commit log (imagine someone
> doesn't have access to read that issue, etc).

I will reexplain the whole problem into the commit message to give more 
context to it in the next version.

> 
> > Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
> > ---
> > 
> >  include/net/netlink.h |  2 +-
> >  include/net/pkt_cls.h |  3 ++-
> >  lib/nlattr.c          | 31 ++++++++++++++++++++-----------
> >  net/sched/act_api.c   |  2 +-
> >  net/sched/sch_api.c   |  2 +-
> >  5 files changed, 25 insertions(+), 15 deletions(-)
> > 
> > diff --git a/include/net/netlink.h b/include/net/netlink.h
> > index 7356f41d23ba..446ca182e13d 100644
> > --- a/include/net/netlink.h
> > +++ b/include/net/netlink.h
> > @@ -506,7 +506,7 @@ int __nla_parse(struct nlattr **tb, int maxtype, const
> > struct nlattr *head,> 
> >  		struct netlink_ext_ack *extack);
> >  
> >  int nla_policy_len(const struct nla_policy *, int);
> >  struct nlattr *nla_find(const struct nlattr *head, int len, int
> >  attrtype);
> > 
> > -size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize);
> > +ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize);
> > 
> >  char *nla_strdup(const struct nlattr *nla, gfp_t flags);
> >  int nla_memcpy(void *dest, const struct nlattr *src, int count);
> >  int nla_memcmp(const struct nlattr *nla, const void *data, size_t size);
> > 
> > diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
> > index d4d461236351..f42db07c399b 100644
> > --- a/include/net/pkt_cls.h
> > +++ b/include/net/pkt_cls.h
> > @@ -4,6 +4,7 @@
> > 
> >  #include <linux/pkt_cls.h>
> >  #include <linux/workqueue.h>
> > 
> > +#include <linux/errno.h>
> > 
> >  #include <net/sch_generic.h>
> >  #include <net/act_api.h>
> >  #include <net/net_namespace.h>
> > 
> > @@ -512,7 +513,7 @@ tcf_change_indev(struct net *net, struct nlattr
> > *indev_tlv,> 
> >  	char indev[IFNAMSIZ];
> >  	struct net_device *dev;
> > 
> > -	if (nla_strlcpy(indev, indev_tlv, IFNAMSIZ) >= IFNAMSIZ) {
> > +	if (nla_strlcpy(indev, indev_tlv, IFNAMSIZ) == -E2BIG) {
> 
> I would make these tests all be "< 0" rather than specifically -E2BIG.
> 

You are right, it would permit to add more -ESOMETHING to nla_sltrcpy without 
changing the caller each time.

> >  		NL_SET_ERR_MSG_ATTR(extack, indev_tlv,
> >  		
> >  				    "Interface name too long");
> >  		
> >  		return -EINVAL;
> > 
> > diff --git a/lib/nlattr.c b/lib/nlattr.c
> > index ab96a5f4b9b8..83dd233bbe3e 100644
> > --- a/lib/nlattr.c
> > +++ b/lib/nlattr.c
> > @@ -713,29 +713,38 @@ EXPORT_SYMBOL(nla_find);
> > 
> >   * @dst: where to copy the string to
> >   * @nla: attribute to copy the string from
> >   * @dstsize: size of destination buffer
> > 
> > + * @returns: -E2BIG if @dstsize is 0 or source buffer length greater than
> > + * @dstsize, otherwise it returns the number of copied characters (not
> > + * including the trailing %NUL).
> > 
> >   *
> >   * Copies at most dstsize - 1 bytes into the destination buffer.
> > 
> > - * The result is always a valid NUL-terminated string. Unlike
> > - * strlcpy the destination buffer is always padded out.
> > - *
> > - * Returns the length of the source buffer.
> > + * The result is always a valid NUL-terminated string.
> > 
> >   */
> > 
> > -size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
> > +ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
> > 
> >  {
> > 
> > +	size_t len;
> > +	ssize_t ret;
> > 
> >  	size_t srclen = nla_len(nla);
> >  	char *src = nla_data(nla);
> > 
> > +	if (dstsize == 0 || WARN_ON_ONCE(dstsize > INT_MAX))
> > +		return -E2BIG;
> 
> Cool, yeah, good to include.
> 

Thank you!

> > +
> > 
> >  	if (srclen > 0 && src[srclen - 1] == '\0')
> >  	
> >  		srclen--;
> > 
> > -	if (dstsize > 0) {
> > -		size_t len = (srclen >= dstsize) ? dstsize - 1 : srclen;
> > -
> > -		memcpy(dst, src, len);
> > -		dst[len] = '\0';
> > +	if (srclen >= dstsize) {
> > +		len = dstsize - 1;
> > +		ret = -E2BIG;
> > +	} else {
> > +		len = srclen;
> > +		ret = len;
> > 
> >  	}
> > 
> > -	return srclen;
> > +	memcpy(dst, src, len);
> > +	dst[len] = '\0';
> > +
> > +	return ret;
> > 
> >  }
> >  EXPORT_SYMBOL(nla_strlcpy);
> 
> Otherwise, I think this looks good. (Though same note about
> zero-padding.)
> 
> > diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> > index f66417d5d2c3..ed411d6c29fc 100644
> > --- a/net/sched/act_api.c
> > +++ b/net/sched/act_api.c
> > @@ -935,7 +935,7 @@ struct tc_action *tcf_action_init_1(struct net *net,
> > struct tcf_proto *tp,> 
> >  			NL_SET_ERR_MSG(extack, "TC action kind must be specified");
> >  			goto err_out;
> >  		
> >  		}
> > 
> > -		if (nla_strlcpy(act_name, kind, IFNAMSIZ) >= IFNAMSIZ) {
> > +		if (nla_strlcpy(act_name, kind, IFNAMSIZ) == -E2BIG) {
> > 
> >  			NL_SET_ERR_MSG(extack, "TC action name too long");
> >  			goto err_out;
> >  		
> >  		}
> > 
> > diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> > index 2a76a2f5ed88..3e89c666d1e8 100644
> > --- a/net/sched/sch_api.c
> > +++ b/net/sched/sch_api.c
> > @@ -1170,7 +1170,7 @@ static struct Qdisc *qdisc_create(struct net_device
> > *dev,> 
> >  #ifdef CONFIG_MODULES
> >  
> >  	if (ops == NULL && kind != NULL) {
> >  	
> >  		char name[IFNAMSIZ];
> > 
> > -		if (nla_strlcpy(name, kind, IFNAMSIZ) < IFNAMSIZ) {
> > +		if (nla_strlcpy(name, kind, IFNAMSIZ) != -E2BIG) {
> > 
> >  			/* We dropped the RTNL semaphore in order to
> >  			
> >  			 * perform the module load.  So, even if we
> >  			 * succeeded in loading the module we have to

Re: [PATCH v1 2/3] Modify return value of nla_strlcpy to match that of strscpy.

[Jann Horn]

On Fri, Oct 16, 2020 at 3:02 PM <laniel_francis@privacyrequired.com> wrote:
> This patch solves part 2 of issue: https://github.com/KSPP/linux/issues/110
>
> Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
> ---
>  include/net/netlink.h |  2 +-
>  include/net/pkt_cls.h |  3 ++-
>  lib/nlattr.c          | 31 ++++++++++++++++++++-----------
>  net/sched/act_api.c   |  2 +-
>  net/sched/sch_api.c   |  2 +-
>  5 files changed, 25 insertions(+), 15 deletions(-)
[...]
> diff --git a/lib/nlattr.c b/lib/nlattr.c
> index ab96a5f4b9b8..83dd233bbe3e 100644
> --- a/lib/nlattr.c
> +++ b/lib/nlattr.c
> @@ -713,29 +713,38 @@ EXPORT_SYMBOL(nla_find);
>   * @dst: where to copy the string to
>   * @nla: attribute to copy the string from
>   * @dstsize: size of destination buffer
> + * @returns: -E2BIG if @dstsize is 0 or source buffer length greater than
> + * @dstsize, otherwise it returns the number of copied characters (not
> + * including the trailing %NUL).
>   *
>   * Copies at most dstsize - 1 bytes into the destination buffer.
> - * The result is always a valid NUL-terminated string. Unlike
> - * strlcpy the destination buffer is always padded out.
> - *
> - * Returns the length of the source buffer.
> + * The result is always a valid NUL-terminated string.
>   */

What about tcf_proto_check_kind()?

Re: [PATCH v1 2/3] Modify return value of nla_strlcpy to match that of strscpy.

[Francis Laniel]

Le samedi 17 octobre 2020, 02:41:33 CEST Jann Horn a écrit :
> On Fri, Oct 16, 2020 at 3:02 PM <laniel_francis@privacyrequired.com> wrote:
> > This patch solves part 2 of issue:
> > https://github.com/KSPP/linux/issues/110
> > 
> > Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
> > ---
> > 
> >  include/net/netlink.h |  2 +-
> >  include/net/pkt_cls.h |  3 ++-
> >  lib/nlattr.c          | 31 ++++++++++++++++++++-----------
> >  net/sched/act_api.c   |  2 +-
> >  net/sched/sch_api.c   |  2 +-
> >  5 files changed, 25 insertions(+), 15 deletions(-)
> 
> [...]
> 
> > diff --git a/lib/nlattr.c b/lib/nlattr.c
> > index ab96a5f4b9b8..83dd233bbe3e 100644
> > --- a/lib/nlattr.c
> > +++ b/lib/nlattr.c
> > @@ -713,29 +713,38 @@ EXPORT_SYMBOL(nla_find);
> > 
> >   * @dst: where to copy the string to
> >   * @nla: attribute to copy the string from
> >   * @dstsize: size of destination buffer
> > 
> > + * @returns: -E2BIG if @dstsize is 0 or source buffer length greater than
> > + * @dstsize, otherwise it returns the number of copied characters (not
> > + * including the trailing %NUL).
> > 
> >   *
> >   * Copies at most dstsize - 1 bytes into the destination buffer.
> > 
> > - * The result is always a valid NUL-terminated string. Unlike
> > - * strlcpy the destination buffer is always padded out.
> > - *
> > - * Returns the length of the source buffer.
> > + * The result is always a valid NUL-terminated string.
> > 
> >   */
> 
> What about tcf_proto_check_kind()?

Good catch!
I just searched for "if (nla_strlcpy" in the code to propagate change.
I will add it to the next version!

[PATCH v1 3/3] Rename nla_strlcpy to nla_strcpy.

[laniel_francis]

From: Francis Laniel <laniel_francis@privacyrequired.com>

This patch solves part 3 of issue: https://github.com/KSPP/linux/issues/110

Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
---
 drivers/infiniband/core/nldev.c            | 10 +++++-----
 drivers/misc/lkdtm/Makefile                |  1 +
 drivers/net/can/vxcan.c                    |  4 ++--
 drivers/net/veth.c                         |  4 ++--
 include/linux/genl_magic_struct.h          |  2 +-
 include/net/netlink.h                      |  4 ++--
 include/net/pkt_cls.h                      |  2 +-
 kernel/taskstats.c                         |  2 +-
 lib/nlattr.c                               |  6 +++---
 net/core/fib_rules.c                       |  4 ++--
 net/core/rtnetlink.c                       | 12 ++++++------
 net/decnet/dn_dev.c                        |  2 +-
 net/ieee802154/nl-mac.c                    |  2 +-
 net/ipv4/devinet.c                         |  2 +-
 net/ipv4/fib_semantics.c                   |  2 +-
 net/ipv4/metrics.c                         |  2 +-
 net/netfilter/ipset/ip_set_hash_netiface.c |  4 ++--
 net/netfilter/nf_tables_api.c              |  6 +++---
 net/netfilter/nfnetlink_acct.c             |  2 +-
 net/netfilter/nfnetlink_cthelper.c         |  4 ++--
 net/netfilter/nft_ct.c                     |  2 +-
 net/netfilter/nft_log.c                    |  2 +-
 net/netlabel/netlabel_mgmt.c               |  2 +-
 net/nfc/netlink.c                          |  2 +-
 net/sched/act_api.c                        |  2 +-
 net/sched/act_ipt.c                        |  2 +-
 net/sched/act_simple.c                     |  4 ++--
 net/sched/cls_api.c                        |  2 +-
 net/sched/sch_api.c                        |  2 +-
 net/tipc/netlink_compat.c                  |  2 +-
 30 files changed, 50 insertions(+), 49 deletions(-)

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 12d29d54a081..10ac54b28ef9 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -932,7 +932,7 @@ static int nldev_set_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (tb[RDMA_NLDEV_ATTR_DEV_NAME]) {
 		char name[IB_DEVICE_NAME_MAX] = {};
 
-		nla_strlcpy(name, tb[RDMA_NLDEV_ATTR_DEV_NAME],
+		nla_strcpy(name, tb[RDMA_NLDEV_ATTR_DEV_NAME],
 			    IB_DEVICE_NAME_MAX);
 		if (strlen(name) == 0) {
 			err = -EINVAL;
@@ -1529,13 +1529,13 @@ static int nldev_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 	    !tb[RDMA_NLDEV_ATTR_LINK_TYPE] || !tb[RDMA_NLDEV_ATTR_NDEV_NAME])
 		return -EINVAL;
 
-	nla_strlcpy(ibdev_name, tb[RDMA_NLDEV_ATTR_DEV_NAME],
+	nla_strcpy(ibdev_name, tb[RDMA_NLDEV_ATTR_DEV_NAME],
 		    sizeof(ibdev_name));
 	if (strchr(ibdev_name, '%') || strlen(ibdev_name) == 0)
 		return -EINVAL;
 
-	nla_strlcpy(type, tb[RDMA_NLDEV_ATTR_LINK_TYPE], sizeof(type));
-	nla_strlcpy(ndev_name, tb[RDMA_NLDEV_ATTR_NDEV_NAME],
+	nla_strcpy(type, tb[RDMA_NLDEV_ATTR_LINK_TYPE], sizeof(type));
+	nla_strcpy(ndev_name, tb[RDMA_NLDEV_ATTR_NDEV_NAME],
 		    sizeof(ndev_name));
 
 	ndev = dev_get_by_name(sock_net(skb->sk), ndev_name);
@@ -1602,7 +1602,7 @@ static int nldev_get_chardev(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (err || !tb[RDMA_NLDEV_ATTR_CHARDEV_TYPE])
 		return -EINVAL;
 
-	nla_strlcpy(client_name, tb[RDMA_NLDEV_ATTR_CHARDEV_TYPE],
+	nla_strcpy(client_name, tb[RDMA_NLDEV_ATTR_CHARDEV_TYPE],
 		    sizeof(client_name));
 
 	if (tb[RDMA_NLDEV_ATTR_DEV_INDEX]) {
diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile
index c70b3822013f..d898f7b22045 100644
--- a/drivers/misc/lkdtm/Makefile
+++ b/drivers/misc/lkdtm/Makefile
@@ -10,6 +10,7 @@ lkdtm-$(CONFIG_LKDTM)		+= rodata_objcopy.o
 lkdtm-$(CONFIG_LKDTM)		+= usercopy.o
 lkdtm-$(CONFIG_LKDTM)		+= stackleak.o
 lkdtm-$(CONFIG_LKDTM)		+= cfi.o
+lkdtm-$(CONFIG_LKDTM)		+= fortify.o
 
 KASAN_SANITIZE_stackleak.o	:= n
 KCOV_INSTRUMENT_rodata.o	:= n
diff --git a/drivers/net/can/vxcan.c b/drivers/net/can/vxcan.c
index d6ba9426be4d..763ad76ac395 100644
--- a/drivers/net/can/vxcan.c
+++ b/drivers/net/can/vxcan.c
@@ -186,7 +186,7 @@ static int vxcan_newlink(struct net *net, struct net_device *dev,
 	}
 
 	if (ifmp && tbp[IFLA_IFNAME]) {
-		nla_strlcpy(ifname, tbp[IFLA_IFNAME], IFNAMSIZ);
+		nla_strcpy(ifname, tbp[IFLA_IFNAME], IFNAMSIZ);
 		name_assign_type = NET_NAME_USER;
 	} else {
 		snprintf(ifname, IFNAMSIZ, DRV_NAME "%%d");
@@ -223,7 +223,7 @@ static int vxcan_newlink(struct net *net, struct net_device *dev,
 
 	/* register first device */
 	if (tb[IFLA_IFNAME])
-		nla_strlcpy(dev->name, tb[IFLA_IFNAME], IFNAMSIZ);
+		nla_strcpy(dev->name, tb[IFLA_IFNAME], IFNAMSIZ);
 	else
 		snprintf(dev->name, IFNAMSIZ, DRV_NAME "%%d");
 
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 8c737668008a..c754e0a1fe40 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -1329,7 +1329,7 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
 	}
 
 	if (ifmp && tbp[IFLA_IFNAME]) {
-		nla_strlcpy(ifname, tbp[IFLA_IFNAME], IFNAMSIZ);
+		nla_strcpy(ifname, tbp[IFLA_IFNAME], IFNAMSIZ);
 		name_assign_type = NET_NAME_USER;
 	} else {
 		snprintf(ifname, IFNAMSIZ, DRV_NAME "%%d");
@@ -1379,7 +1379,7 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
 		eth_hw_addr_random(dev);
 
 	if (tb[IFLA_IFNAME])
-		nla_strlcpy(dev->name, tb[IFLA_IFNAME], IFNAMSIZ);
+		nla_strcpy(dev->name, tb[IFLA_IFNAME], IFNAMSIZ);
 	else
 		snprintf(dev->name, IFNAMSIZ, DRV_NAME "%%d");
 
diff --git a/include/linux/genl_magic_struct.h b/include/linux/genl_magic_struct.h
index eeae59d3ceb7..1ebec86d5c45 100644
--- a/include/linux/genl_magic_struct.h
+++ b/include/linux/genl_magic_struct.h
@@ -89,7 +89,7 @@ static inline int nla_put_u64_0pad(struct sk_buff *skb, int attrtype, u64 value)
 			nla_get_u64, nla_put_u64_0pad, false)
 #define __str_field(attr_nr, attr_flag, name, maxlen) \
 	__array(attr_nr, attr_flag, name, NLA_NUL_STRING, char, maxlen, \
-			nla_strlcpy, nla_put, false)
+			nla_strcpy, nla_put, false)
 #define __bin_field(attr_nr, attr_flag, name, maxlen) \
 	__array(attr_nr, attr_flag, name, NLA_BINARY, char, maxlen, \
 			nla_memcpy, nla_put, false)
diff --git a/include/net/netlink.h b/include/net/netlink.h
index 446ca182e13d..bbe9cd13c732 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -142,7 +142,7 @@
  * Attribute Misc:
  *   nla_memcpy(dest, nla, count)	copy attribute into memory
  *   nla_memcmp(nla, data, size)	compare attribute with memory area
- *   nla_strlcpy(dst, nla, size)	copy attribute to a sized string
+ *   nla_strcpy(dst, nla, size)	copy attribute to a sized string
  *   nla_strcmp(nla, str)		compare attribute with string
  *
  * Attribute Parsing:
@@ -506,7 +506,7 @@ int __nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
 		struct netlink_ext_ack *extack);
 int nla_policy_len(const struct nla_policy *, int);
 struct nlattr *nla_find(const struct nlattr *head, int len, int attrtype);
-ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize);
+ssize_t nla_strcpy(char *dst, const struct nlattr *nla, size_t dstsize);
 char *nla_strdup(const struct nlattr *nla, gfp_t flags);
 int nla_memcpy(void *dest, const struct nlattr *src, int count);
 int nla_memcmp(const struct nlattr *nla, const void *data, size_t size);
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index f42db07c399b..c661b48d4802 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -513,7 +513,7 @@ tcf_change_indev(struct net *net, struct nlattr *indev_tlv,
 	char indev[IFNAMSIZ];
 	struct net_device *dev;
 
-	if (nla_strlcpy(indev, indev_tlv, IFNAMSIZ) == -E2BIG) {
+	if (nla_strcpy(indev, indev_tlv, IFNAMSIZ) == -E2BIG) {
 		NL_SET_ERR_MSG_ATTR(extack, indev_tlv,
 				    "Interface name too long");
 		return -EINVAL;
diff --git a/kernel/taskstats.c b/kernel/taskstats.c
index a2802b6ff4bb..713f35649806 100644
--- a/kernel/taskstats.c
+++ b/kernel/taskstats.c
@@ -346,7 +346,7 @@ static int parse(struct nlattr *na, struct cpumask *mask)
 	data = kmalloc(len, GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
-	nla_strlcpy(data, na, len);
+	nla_strcpy(data, na, len);
 	ret = cpulist_parse(data, mask);
 	kfree(data);
 	return ret;
diff --git a/lib/nlattr.c b/lib/nlattr.c
index 83dd233bbe3e..ec930c39d56a 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -709,7 +709,7 @@ struct nlattr *nla_find(const struct nlattr *head, int len, int attrtype)
 EXPORT_SYMBOL(nla_find);
 
 /**
- * nla_strlcpy - Copy string attribute payload into a sized buffer
+ * nla_strcpy - Copy string attribute payload into a sized buffer
  * @dst: where to copy the string to
  * @nla: attribute to copy the string from
  * @dstsize: size of destination buffer
@@ -720,7 +720,7 @@ EXPORT_SYMBOL(nla_find);
  * Copies at most dstsize - 1 bytes into the destination buffer.
  * The result is always a valid NUL-terminated string.
  */
-ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
+ssize_t nla_strcpy(char *dst, const struct nlattr *nla, size_t dstsize)
 {
 	size_t len;
 	ssize_t ret;
@@ -746,7 +746,7 @@ ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
 
 	return ret;
 }
-EXPORT_SYMBOL(nla_strlcpy);
+EXPORT_SYMBOL(nla_strcpy);
 
 /**
  * nla_strdup - Copy string attribute payload into a newly allocated buffer
diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index 7bcfb16854cb..61d86780f409 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -563,7 +563,7 @@ static int fib_nl2rule(struct sk_buff *skb, struct nlmsghdr *nlh,
 		struct net_device *dev;
 
 		nlrule->iifindex = -1;
-		nla_strlcpy(nlrule->iifname, tb[FRA_IIFNAME], IFNAMSIZ);
+		nla_strcpy(nlrule->iifname, tb[FRA_IIFNAME], IFNAMSIZ);
 		dev = __dev_get_by_name(net, nlrule->iifname);
 		if (dev)
 			nlrule->iifindex = dev->ifindex;
@@ -573,7 +573,7 @@ static int fib_nl2rule(struct sk_buff *skb, struct nlmsghdr *nlh,
 		struct net_device *dev;
 
 		nlrule->oifindex = -1;
-		nla_strlcpy(nlrule->oifname, tb[FRA_OIFNAME], IFNAMSIZ);
+		nla_strcpy(nlrule->oifname, tb[FRA_OIFNAME], IFNAMSIZ);
 		dev = __dev_get_by_name(net, nlrule->oifname);
 		if (dev)
 			nlrule->oifindex = dev->ifindex;
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 68e0682450c6..5d70113b4f25 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1939,7 +1939,7 @@ static const struct rtnl_link_ops *linkinfo_to_kind_ops(const struct nlattr *nla
 	if (linfo[IFLA_INFO_KIND]) {
 		char kind[MODULE_NAME_LEN];
 
-		nla_strlcpy(kind, linfo[IFLA_INFO_KIND], sizeof(kind));
+		nla_strcpy(kind, linfo[IFLA_INFO_KIND], sizeof(kind));
 		ops = rtnl_link_ops_get(kind);
 	}
 
@@ -2953,9 +2953,9 @@ static struct net_device *rtnl_dev_get(struct net *net,
 	if (!ifname) {
 		ifname = buffer;
 		if (ifname_attr)
-			nla_strlcpy(ifname, ifname_attr, IFNAMSIZ);
+			nla_strcpy(ifname, ifname_attr, IFNAMSIZ);
 		else if (altifname_attr)
-			nla_strlcpy(ifname, altifname_attr, ALTIFNAMSIZ);
+			nla_strcpy(ifname, altifname_attr, ALTIFNAMSIZ);
 		else
 			return NULL;
 	}
@@ -2983,7 +2983,7 @@ static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 		goto errout;
 
 	if (tb[IFLA_IFNAME])
-		nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
+		nla_strcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
 	else
 		ifname[0] = '\0';
 
@@ -3264,7 +3264,7 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 		return err;
 
 	if (tb[IFLA_IFNAME])
-		nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
+		nla_strcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
 	else
 		ifname[0] = '\0';
 
@@ -3296,7 +3296,7 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 		memset(linkinfo, 0, sizeof(linkinfo));
 
 	if (linkinfo[IFLA_INFO_KIND]) {
-		nla_strlcpy(kind, linkinfo[IFLA_INFO_KIND], sizeof(kind));
+		nla_strcpy(kind, linkinfo[IFLA_INFO_KIND], sizeof(kind));
 		ops = rtnl_link_ops_get(kind);
 	} else {
 		kind[0] = '\0';
diff --git a/net/decnet/dn_dev.c b/net/decnet/dn_dev.c
index 15d42353f1a3..e6c83289eecc 100644
--- a/net/decnet/dn_dev.c
+++ b/net/decnet/dn_dev.c
@@ -658,7 +658,7 @@ static int dn_nl_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
 	ifa->ifa_dev = dn_db;
 
 	if (tb[IFA_LABEL])
-		nla_strlcpy(ifa->ifa_label, tb[IFA_LABEL], IFNAMSIZ);
+		nla_strcpy(ifa->ifa_label, tb[IFA_LABEL], IFNAMSIZ);
 	else
 		memcpy(ifa->ifa_label, dev->name, IFNAMSIZ);
 
diff --git a/net/ieee802154/nl-mac.c b/net/ieee802154/nl-mac.c
index 6d091e419d3e..45472ba629bb 100644
--- a/net/ieee802154/nl-mac.c
+++ b/net/ieee802154/nl-mac.c
@@ -149,7 +149,7 @@ static struct net_device *ieee802154_nl_get_dev(struct genl_info *info)
 	if (info->attrs[IEEE802154_ATTR_DEV_NAME]) {
 		char name[IFNAMSIZ + 1];
 
-		nla_strlcpy(name, info->attrs[IEEE802154_ATTR_DEV_NAME],
+		nla_strcpy(name, info->attrs[IEEE802154_ATTR_DEV_NAME],
 			    sizeof(name));
 		dev = dev_get_by_name(&init_net, name);
 	} else if (info->attrs[IEEE802154_ATTR_DEV_INDEX]) {
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 123a6d39438f..5fbb6bee6d82 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -881,7 +881,7 @@ static struct in_ifaddr *rtm_to_ifaddr(struct net *net, struct nlmsghdr *nlh,
 		ifa->ifa_broadcast = nla_get_in_addr(tb[IFA_BROADCAST]);
 
 	if (tb[IFA_LABEL])
-		nla_strlcpy(ifa->ifa_label, tb[IFA_LABEL], IFNAMSIZ);
+		nla_strcpy(ifa->ifa_label, tb[IFA_LABEL], IFNAMSIZ);
 	else
 		memcpy(ifa->ifa_label, dev->name, IFNAMSIZ);
 
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 1f75dc686b6b..8437d46a30c9 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -973,7 +973,7 @@ bool fib_metrics_match(struct fib_config *cfg, struct fib_info *fi)
 			char tmp[TCP_CA_NAME_MAX];
 			bool ecn_ca = false;
 
-			nla_strlcpy(tmp, nla, sizeof(tmp));
+			nla_strcpy(tmp, nla, sizeof(tmp));
 			val = tcp_ca_get_key_by_name(fi->fib_net, tmp, &ecn_ca);
 		} else {
 			if (nla_len(nla) != sizeof(u32))
diff --git a/net/ipv4/metrics.c b/net/ipv4/metrics.c
index 3205d5f7c8c9..99844fd6975d 100644
--- a/net/ipv4/metrics.c
+++ b/net/ipv4/metrics.c
@@ -31,7 +31,7 @@ static int ip_metrics_convert(struct net *net, struct nlattr *fc_mx,
 		if (type == RTAX_CC_ALGO) {
 			char tmp[TCP_CA_NAME_MAX];
 
-			nla_strlcpy(tmp, nla, sizeof(tmp));
+			nla_strcpy(tmp, nla, sizeof(tmp));
 			val = tcp_ca_get_key_by_name(net, tmp, &ecn_ca);
 			if (val == TCP_CA_UNSPEC) {
 				NL_SET_ERR_MSG(extack, "Unknown tcp congestion algorithm");
diff --git a/net/netfilter/ipset/ip_set_hash_netiface.c b/net/netfilter/ipset/ip_set_hash_netiface.c
index be5e95a0d876..5e114985ad9d 100644
--- a/net/netfilter/ipset/ip_set_hash_netiface.c
+++ b/net/netfilter/ipset/ip_set_hash_netiface.c
@@ -225,7 +225,7 @@ hash_netiface4_uadt(struct ip_set *set, struct nlattr *tb[],
 		if (e.cidr > HOST_MASK)
 			return -IPSET_ERR_INVALID_CIDR;
 	}
-	nla_strlcpy(e.iface, tb[IPSET_ATTR_IFACE], IFNAMSIZ);
+	nla_strcpy(e.iface, tb[IPSET_ATTR_IFACE], IFNAMSIZ);
 
 	if (tb[IPSET_ATTR_CADT_FLAGS]) {
 		u32 cadt_flags = ip_set_get_h32(tb[IPSET_ATTR_CADT_FLAGS]);
@@ -442,7 +442,7 @@ hash_netiface6_uadt(struct ip_set *set, struct nlattr *tb[],
 
 	ip6_netmask(&e.ip, e.cidr);
 
-	nla_strlcpy(e.iface, tb[IPSET_ATTR_IFACE], IFNAMSIZ);
+	nla_strcpy(e.iface, tb[IPSET_ATTR_IFACE], IFNAMSIZ);
 
 	if (tb[IPSET_ATTR_CADT_FLAGS]) {
 		u32 cadt_flags = ip_set_get_h32(tb[IPSET_ATTR_CADT_FLAGS]);
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 9957e0ed8658..38eb14ffbc4f 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1281,7 +1281,7 @@ static struct nft_chain *nft_chain_lookup(struct net *net,
 	if (nla == NULL)
 		return ERR_PTR(-EINVAL);
 
-	nla_strlcpy(search, nla, sizeof(search));
+	nla_strcpy(search, nla, sizeof(search));
 
 	WARN_ON(!rcu_read_lock_held() &&
 		!lockdep_commit_lock_is_held(net));
@@ -1721,7 +1721,7 @@ static struct nft_hook *nft_netdev_hook_alloc(struct net *net,
 		goto err_hook_alloc;
 	}
 
-	nla_strlcpy(ifname, attr, IFNAMSIZ);
+	nla_strcpy(ifname, attr, IFNAMSIZ);
 	dev = __dev_get_by_name(net, ifname);
 	if (!dev) {
 		err = -ENOENT;
@@ -5734,7 +5734,7 @@ struct nft_object *nft_obj_lookup(const struct net *net,
 	struct rhlist_head *tmp, *list;
 	struct nft_object *obj;
 
-	nla_strlcpy(search, nla, sizeof(search));
+	nla_strcpy(search, nla, sizeof(search));
 	k.name = search;
 
 	WARN_ON_ONCE(!rcu_read_lock_held() &&
diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c
index 5bfec829c12f..d657749724da 100644
--- a/net/netfilter/nfnetlink_acct.c
+++ b/net/netfilter/nfnetlink_acct.c
@@ -112,7 +112,7 @@ static int nfnl_acct_new(struct net *net, struct sock *nfnl,
 		nfacct->flags = flags;
 	}
 
-	nla_strlcpy(nfacct->name, tb[NFACCT_NAME], NFACCT_NAME_MAX);
+	nla_strcpy(nfacct->name, tb[NFACCT_NAME], NFACCT_NAME_MAX);
 
 	if (tb[NFACCT_BYTES]) {
 		atomic64_set(&nfacct->bytes,
diff --git a/net/netfilter/nfnetlink_cthelper.c b/net/netfilter/nfnetlink_cthelper.c
index 5b0d0a77379c..1900895fa3d5 100644
--- a/net/netfilter/nfnetlink_cthelper.c
+++ b/net/netfilter/nfnetlink_cthelper.c
@@ -146,7 +146,7 @@ nfnl_cthelper_expect_policy(struct nf_conntrack_expect_policy *expect_policy,
 	    !tb[NFCTH_POLICY_EXPECT_TIMEOUT])
 		return -EINVAL;
 
-	nla_strlcpy(expect_policy->name,
+	nla_strcpy(expect_policy->name,
 		    tb[NFCTH_POLICY_NAME], NF_CT_HELPER_NAME_LEN);
 	expect_policy->max_expected =
 		ntohl(nla_get_be32(tb[NFCTH_POLICY_EXPECT_MAX]));
@@ -233,7 +233,7 @@ nfnl_cthelper_create(const struct nlattr * const tb[],
 	if (ret < 0)
 		goto err1;
 
-	nla_strlcpy(helper->name,
+	nla_strcpy(helper->name,
 		    tb[NFCTH_NAME], NF_CT_HELPER_NAME_LEN);
 	size = ntohl(nla_get_be32(tb[NFCTH_PRIV_DATA_LEN]));
 	if (size > sizeof_field(struct nf_conn_help, data)) {
diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
index 322bd674963e..bb02ea2ef1bf 100644
--- a/net/netfilter/nft_ct.c
+++ b/net/netfilter/nft_ct.c
@@ -990,7 +990,7 @@ static int nft_ct_helper_obj_init(const struct nft_ctx *ctx,
 	if (!priv->l4proto)
 		return -ENOENT;
 
-	nla_strlcpy(name, tb[NFTA_CT_HELPER_NAME], sizeof(name));
+	nla_strcpy(name, tb[NFTA_CT_HELPER_NAME], sizeof(name));
 
 	if (tb[NFTA_CT_HELPER_L3PROTO])
 		family = ntohs(nla_get_be16(tb[NFTA_CT_HELPER_L3PROTO]));
diff --git a/net/netfilter/nft_log.c b/net/netfilter/nft_log.c
index 57899454a530..b55607b24942 100644
--- a/net/netfilter/nft_log.c
+++ b/net/netfilter/nft_log.c
@@ -152,7 +152,7 @@ static int nft_log_init(const struct nft_ctx *ctx,
 		priv->prefix = kmalloc(nla_len(nla) + 1, GFP_KERNEL);
 		if (priv->prefix == NULL)
 			return -ENOMEM;
-		nla_strlcpy(priv->prefix, nla, nla_len(nla) + 1);
+		nla_strcpy(priv->prefix, nla, nla_len(nla) + 1);
 	} else {
 		priv->prefix = (char *)nft_log_null_prefix;
 	}
diff --git a/net/netlabel/netlabel_mgmt.c b/net/netlabel/netlabel_mgmt.c
index eb1d66d20afb..3dd925c39ef4 100644
--- a/net/netlabel/netlabel_mgmt.c
+++ b/net/netlabel/netlabel_mgmt.c
@@ -95,7 +95,7 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
 			ret_val = -ENOMEM;
 			goto add_free_entry;
 		}
-		nla_strlcpy(entry->domain,
+		nla_strcpy(entry->domain,
 			    info->attrs[NLBL_MGMT_A_DOMAIN], tmp_size);
 	}
 
diff --git a/net/nfc/netlink.c b/net/nfc/netlink.c
index e894254c17d4..ee4a53d15ae3 100644
--- a/net/nfc/netlink.c
+++ b/net/nfc/netlink.c
@@ -1226,7 +1226,7 @@ static int nfc_genl_fw_download(struct sk_buff *skb, struct genl_info *info)
 	if (!dev)
 		return -ENODEV;
 
-	nla_strlcpy(firmware_name, info->attrs[NFC_ATTR_FIRMWARE_NAME],
+	nla_strcpy(firmware_name, info->attrs[NFC_ATTR_FIRMWARE_NAME],
 		    sizeof(firmware_name));
 
 	rc = nfc_fw_download(dev, firmware_name);
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index ed411d6c29fc..9c001af0c933 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -935,7 +935,7 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
 			NL_SET_ERR_MSG(extack, "TC action kind must be specified");
 			goto err_out;
 		}
-		if (nla_strlcpy(act_name, kind, IFNAMSIZ) == -E2BIG) {
+		if (nla_strcpy(act_name, kind, IFNAMSIZ) == -E2BIG) {
 			NL_SET_ERR_MSG(extack, "TC action name too long");
 			goto err_out;
 		}
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
index 8dc3bec0d325..9ab92a781104 100644
--- a/net/sched/act_ipt.c
+++ b/net/sched/act_ipt.c
@@ -166,7 +166,7 @@ static int __tcf_ipt_init(struct net *net, unsigned int id, struct nlattr *nla,
 	if (unlikely(!tname))
 		goto err1;
 	if (tb[TCA_IPT_TABLE] == NULL ||
-	    nla_strlcpy(tname, tb[TCA_IPT_TABLE], IFNAMSIZ) >= IFNAMSIZ)
+	    nla_strcpy(tname, tb[TCA_IPT_TABLE], IFNAMSIZ) >= IFNAMSIZ)
 		strcpy(tname, "mangle");
 
 	t = kmemdup(td, td->u.target_size, GFP_KERNEL);
diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
index a4f3d0f0daa9..6509dae17357 100644
--- a/net/sched/act_simple.c
+++ b/net/sched/act_simple.c
@@ -52,7 +52,7 @@ static int alloc_defdata(struct tcf_defact *d, const struct nlattr *defdata)
 	d->tcfd_defdata = kzalloc(SIMP_MAX_DATA, GFP_KERNEL);
 	if (unlikely(!d->tcfd_defdata))
 		return -ENOMEM;
-	nla_strlcpy(d->tcfd_defdata, defdata, SIMP_MAX_DATA);
+	nla_strcpy(d->tcfd_defdata, defdata, SIMP_MAX_DATA);
 	return 0;
 }
 
@@ -71,7 +71,7 @@ static int reset_policy(struct tc_action *a, const struct nlattr *defdata,
 	spin_lock_bh(&d->tcf_lock);
 	goto_ch = tcf_action_set_ctrlact(a, p->action, goto_ch);
 	memset(d->tcfd_defdata, 0, SIMP_MAX_DATA);
-	nla_strlcpy(d->tcfd_defdata, defdata, SIMP_MAX_DATA);
+	nla_strcpy(d->tcfd_defdata, defdata, SIMP_MAX_DATA);
 	spin_unlock_bh(&d->tcf_lock);
 	if (goto_ch)
 		tcf_chain_put_by_act(goto_ch);
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 41a55c6cbeb8..b96573c4a8a7 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -223,7 +223,7 @@ static inline u32 tcf_auto_prio(struct tcf_proto *tp)
 static bool tcf_proto_check_kind(struct nlattr *kind, char *name)
 {
 	if (kind)
-		return nla_strlcpy(name, kind, IFNAMSIZ) >= IFNAMSIZ;
+		return nla_strcpy(name, kind, IFNAMSIZ) >= IFNAMSIZ;
 	memset(name, 0, IFNAMSIZ);
 	return false;
 }
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 3e89c666d1e8..bc764da38fb5 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1170,7 +1170,7 @@ static struct Qdisc *qdisc_create(struct net_device *dev,
 #ifdef CONFIG_MODULES
 	if (ops == NULL && kind != NULL) {
 		char name[IFNAMSIZ];
-		if (nla_strlcpy(name, kind, IFNAMSIZ) != -E2BIG) {
+		if (nla_strcpy(name, kind, IFNAMSIZ) != -E2BIG) {
 			/* We dropped the RTNL semaphore in order to
 			 * perform the module load.  So, even if we
 			 * succeeded in loading the module we have to
diff --git a/net/tipc/netlink_compat.c b/net/tipc/netlink_compat.c
index 1c7aa51cc2a3..c973a61fe440 100644
--- a/net/tipc/netlink_compat.c
+++ b/net/tipc/netlink_compat.c
@@ -695,7 +695,7 @@ static int tipc_nl_compat_link_dump(struct tipc_nl_compat_msg *msg,
 
 	link_info.dest = nla_get_flag(link[TIPC_NLA_LINK_DEST]);
 	link_info.up = htonl(nla_get_flag(link[TIPC_NLA_LINK_UP]));
-	nla_strlcpy(link_info.str, link[TIPC_NLA_LINK_NAME],
+	nla_strcpy(link_info.str, link[TIPC_NLA_LINK_NAME],
 		    TIPC_MAX_LINK_NAME);
 
 	return tipc_add_tlv(msg->rep, TIPC_TLV_LINK_INFO,
-- 
2.20.1

Re: [PATCH v1 3/3] Rename nla_strlcpy to nla_strcpy.

[Kees Cook]

On Fri, Oct 16, 2020 at 02:52:16PM +0200, laniel_francis@privacyrequired.com wrote:
> From: Francis Laniel <laniel_francis@privacyrequired.com>
> 
> This patch solves part 3 of issue: https://github.com/KSPP/linux/issues/110

This needs a full commit log, but is straight forward. :) One note
below...

> 
> Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
> ---
>  drivers/infiniband/core/nldev.c            | 10 +++++-----
>  drivers/misc/lkdtm/Makefile                |  1 +
>  drivers/net/can/vxcan.c                    |  4 ++--
>  drivers/net/veth.c                         |  4 ++--
>  include/linux/genl_magic_struct.h          |  2 +-
>  include/net/netlink.h                      |  4 ++--
>  include/net/pkt_cls.h                      |  2 +-
>  kernel/taskstats.c                         |  2 +-
>  lib/nlattr.c                               |  6 +++---
>  net/core/fib_rules.c                       |  4 ++--
>  net/core/rtnetlink.c                       | 12 ++++++------
>  net/decnet/dn_dev.c                        |  2 +-
>  net/ieee802154/nl-mac.c                    |  2 +-
>  net/ipv4/devinet.c                         |  2 +-
>  net/ipv4/fib_semantics.c                   |  2 +-
>  net/ipv4/metrics.c                         |  2 +-
>  net/netfilter/ipset/ip_set_hash_netiface.c |  4 ++--
>  net/netfilter/nf_tables_api.c              |  6 +++---
>  net/netfilter/nfnetlink_acct.c             |  2 +-
>  net/netfilter/nfnetlink_cthelper.c         |  4 ++--
>  net/netfilter/nft_ct.c                     |  2 +-
>  net/netfilter/nft_log.c                    |  2 +-
>  net/netlabel/netlabel_mgmt.c               |  2 +-
>  net/nfc/netlink.c                          |  2 +-
>  net/sched/act_api.c                        |  2 +-
>  net/sched/act_ipt.c                        |  2 +-
>  net/sched/act_simple.c                     |  4 ++--
>  net/sched/cls_api.c                        |  2 +-
>  net/sched/sch_api.c                        |  2 +-
>  net/tipc/netlink_compat.c                  |  2 +-
>  30 files changed, 50 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
> index 12d29d54a081..10ac54b28ef9 100644
> --- a/drivers/infiniband/core/nldev.c
> +++ b/drivers/infiniband/core/nldev.c
> @@ -932,7 +932,7 @@ static int nldev_set_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
>  	if (tb[RDMA_NLDEV_ATTR_DEV_NAME]) {
>  		char name[IB_DEVICE_NAME_MAX] = {};
>  
> -		nla_strlcpy(name, tb[RDMA_NLDEV_ATTR_DEV_NAME],
> +		nla_strcpy(name, tb[RDMA_NLDEV_ATTR_DEV_NAME],
>  			    IB_DEVICE_NAME_MAX);
>  		if (strlen(name) == 0) {
>  			err = -EINVAL;
> @@ -1529,13 +1529,13 @@ static int nldev_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
>  	    !tb[RDMA_NLDEV_ATTR_LINK_TYPE] || !tb[RDMA_NLDEV_ATTR_NDEV_NAME])
>  		return -EINVAL;
>  
> -	nla_strlcpy(ibdev_name, tb[RDMA_NLDEV_ATTR_DEV_NAME],
> +	nla_strcpy(ibdev_name, tb[RDMA_NLDEV_ATTR_DEV_NAME],
>  		    sizeof(ibdev_name));
>  	if (strchr(ibdev_name, '%') || strlen(ibdev_name) == 0)
>  		return -EINVAL;
>  
> -	nla_strlcpy(type, tb[RDMA_NLDEV_ATTR_LINK_TYPE], sizeof(type));
> -	nla_strlcpy(ndev_name, tb[RDMA_NLDEV_ATTR_NDEV_NAME],
> +	nla_strcpy(type, tb[RDMA_NLDEV_ATTR_LINK_TYPE], sizeof(type));
> +	nla_strcpy(ndev_name, tb[RDMA_NLDEV_ATTR_NDEV_NAME],
>  		    sizeof(ndev_name));
>  
>  	ndev = dev_get_by_name(sock_net(skb->sk), ndev_name);
> @@ -1602,7 +1602,7 @@ static int nldev_get_chardev(struct sk_buff *skb, struct nlmsghdr *nlh,
>  	if (err || !tb[RDMA_NLDEV_ATTR_CHARDEV_TYPE])
>  		return -EINVAL;
>  
> -	nla_strlcpy(client_name, tb[RDMA_NLDEV_ATTR_CHARDEV_TYPE],
> +	nla_strcpy(client_name, tb[RDMA_NLDEV_ATTR_CHARDEV_TYPE],
>  		    sizeof(client_name));
>  
>  	if (tb[RDMA_NLDEV_ATTR_DEV_INDEX]) {
> diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile
> index c70b3822013f..d898f7b22045 100644
> --- a/drivers/misc/lkdtm/Makefile
> +++ b/drivers/misc/lkdtm/Makefile
> @@ -10,6 +10,7 @@ lkdtm-$(CONFIG_LKDTM)		+= rodata_objcopy.o
>  lkdtm-$(CONFIG_LKDTM)		+= usercopy.o
>  lkdtm-$(CONFIG_LKDTM)		+= stackleak.o
>  lkdtm-$(CONFIG_LKDTM)		+= cfi.o
> +lkdtm-$(CONFIG_LKDTM)		+= fortify.o

I think this sneaked its way into the wrong patch. ;)

>  
>  KASAN_SANITIZE_stackleak.o	:= n
>  KCOV_INSTRUMENT_rodata.o	:= n
> diff --git a/drivers/net/can/vxcan.c b/drivers/net/can/vxcan.c
> index d6ba9426be4d..763ad76ac395 100644
> --- a/drivers/net/can/vxcan.c
> +++ b/drivers/net/can/vxcan.c
> @@ -186,7 +186,7 @@ static int vxcan_newlink(struct net *net, struct net_device *dev,
>  	}
>  
>  	if (ifmp && tbp[IFLA_IFNAME]) {
> -		nla_strlcpy(ifname, tbp[IFLA_IFNAME], IFNAMSIZ);
> +		nla_strcpy(ifname, tbp[IFLA_IFNAME], IFNAMSIZ);
>  		name_assign_type = NET_NAME_USER;
>  	} else {
>  		snprintf(ifname, IFNAMSIZ, DRV_NAME "%%d");
> @@ -223,7 +223,7 @@ static int vxcan_newlink(struct net *net, struct net_device *dev,
>  
>  	/* register first device */
>  	if (tb[IFLA_IFNAME])
> -		nla_strlcpy(dev->name, tb[IFLA_IFNAME], IFNAMSIZ);
> +		nla_strcpy(dev->name, tb[IFLA_IFNAME], IFNAMSIZ);
>  	else
>  		snprintf(dev->name, IFNAMSIZ, DRV_NAME "%%d");
>  
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 8c737668008a..c754e0a1fe40 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -1329,7 +1329,7 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
>  	}
>  
>  	if (ifmp && tbp[IFLA_IFNAME]) {
> -		nla_strlcpy(ifname, tbp[IFLA_IFNAME], IFNAMSIZ);
> +		nla_strcpy(ifname, tbp[IFLA_IFNAME], IFNAMSIZ);
>  		name_assign_type = NET_NAME_USER;
>  	} else {
>  		snprintf(ifname, IFNAMSIZ, DRV_NAME "%%d");
> @@ -1379,7 +1379,7 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
>  		eth_hw_addr_random(dev);
>  
>  	if (tb[IFLA_IFNAME])
> -		nla_strlcpy(dev->name, tb[IFLA_IFNAME], IFNAMSIZ);
> +		nla_strcpy(dev->name, tb[IFLA_IFNAME], IFNAMSIZ);
>  	else
>  		snprintf(dev->name, IFNAMSIZ, DRV_NAME "%%d");
>  
> diff --git a/include/linux/genl_magic_struct.h b/include/linux/genl_magic_struct.h
> index eeae59d3ceb7..1ebec86d5c45 100644
> --- a/include/linux/genl_magic_struct.h
> +++ b/include/linux/genl_magic_struct.h
> @@ -89,7 +89,7 @@ static inline int nla_put_u64_0pad(struct sk_buff *skb, int attrtype, u64 value)
>  			nla_get_u64, nla_put_u64_0pad, false)
>  #define __str_field(attr_nr, attr_flag, name, maxlen) \
>  	__array(attr_nr, attr_flag, name, NLA_NUL_STRING, char, maxlen, \
> -			nla_strlcpy, nla_put, false)
> +			nla_strcpy, nla_put, false)
>  #define __bin_field(attr_nr, attr_flag, name, maxlen) \
>  	__array(attr_nr, attr_flag, name, NLA_BINARY, char, maxlen, \
>  			nla_memcpy, nla_put, false)
> diff --git a/include/net/netlink.h b/include/net/netlink.h
> index 446ca182e13d..bbe9cd13c732 100644
> --- a/include/net/netlink.h
> +++ b/include/net/netlink.h
> @@ -142,7 +142,7 @@
>   * Attribute Misc:
>   *   nla_memcpy(dest, nla, count)	copy attribute into memory
>   *   nla_memcmp(nla, data, size)	compare attribute with memory area
> - *   nla_strlcpy(dst, nla, size)	copy attribute to a sized string
> + *   nla_strcpy(dst, nla, size)	copy attribute to a sized string
>   *   nla_strcmp(nla, str)		compare attribute with string
>   *
>   * Attribute Parsing:
> @@ -506,7 +506,7 @@ int __nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
>  		struct netlink_ext_ack *extack);
>  int nla_policy_len(const struct nla_policy *, int);
>  struct nlattr *nla_find(const struct nlattr *head, int len, int attrtype);
> -ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize);
> +ssize_t nla_strcpy(char *dst, const struct nlattr *nla, size_t dstsize);
>  char *nla_strdup(const struct nlattr *nla, gfp_t flags);
>  int nla_memcpy(void *dest, const struct nlattr *src, int count);
>  int nla_memcmp(const struct nlattr *nla, const void *data, size_t size);
> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
> index f42db07c399b..c661b48d4802 100644
> --- a/include/net/pkt_cls.h
> +++ b/include/net/pkt_cls.h
> @@ -513,7 +513,7 @@ tcf_change_indev(struct net *net, struct nlattr *indev_tlv,
>  	char indev[IFNAMSIZ];
>  	struct net_device *dev;
>  
> -	if (nla_strlcpy(indev, indev_tlv, IFNAMSIZ) == -E2BIG) {
> +	if (nla_strcpy(indev, indev_tlv, IFNAMSIZ) == -E2BIG) {
>  		NL_SET_ERR_MSG_ATTR(extack, indev_tlv,
>  				    "Interface name too long");
>  		return -EINVAL;
> diff --git a/kernel/taskstats.c b/kernel/taskstats.c
> index a2802b6ff4bb..713f35649806 100644
> --- a/kernel/taskstats.c
> +++ b/kernel/taskstats.c
> @@ -346,7 +346,7 @@ static int parse(struct nlattr *na, struct cpumask *mask)
>  	data = kmalloc(len, GFP_KERNEL);
>  	if (!data)
>  		return -ENOMEM;
> -	nla_strlcpy(data, na, len);
> +	nla_strcpy(data, na, len);
>  	ret = cpulist_parse(data, mask);
>  	kfree(data);
>  	return ret;
> diff --git a/lib/nlattr.c b/lib/nlattr.c
> index 83dd233bbe3e..ec930c39d56a 100644
> --- a/lib/nlattr.c
> +++ b/lib/nlattr.c
> @@ -709,7 +709,7 @@ struct nlattr *nla_find(const struct nlattr *head, int len, int attrtype)
>  EXPORT_SYMBOL(nla_find);
>  
>  /**
> - * nla_strlcpy - Copy string attribute payload into a sized buffer
> + * nla_strcpy - Copy string attribute payload into a sized buffer
>   * @dst: where to copy the string to
>   * @nla: attribute to copy the string from
>   * @dstsize: size of destination buffer
> @@ -720,7 +720,7 @@ EXPORT_SYMBOL(nla_find);
>   * Copies at most dstsize - 1 bytes into the destination buffer.
>   * The result is always a valid NUL-terminated string.
>   */
> -ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
> +ssize_t nla_strcpy(char *dst, const struct nlattr *nla, size_t dstsize)
>  {
>  	size_t len;
>  	ssize_t ret;
> @@ -746,7 +746,7 @@ ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
>  
>  	return ret;
>  }
> -EXPORT_SYMBOL(nla_strlcpy);
> +EXPORT_SYMBOL(nla_strcpy);
>  
>  /**
>   * nla_strdup - Copy string attribute payload into a newly allocated buffer
> diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
> index 7bcfb16854cb..61d86780f409 100644
> --- a/net/core/fib_rules.c
> +++ b/net/core/fib_rules.c
> @@ -563,7 +563,7 @@ static int fib_nl2rule(struct sk_buff *skb, struct nlmsghdr *nlh,
>  		struct net_device *dev;
>  
>  		nlrule->iifindex = -1;
> -		nla_strlcpy(nlrule->iifname, tb[FRA_IIFNAME], IFNAMSIZ);
> +		nla_strcpy(nlrule->iifname, tb[FRA_IIFNAME], IFNAMSIZ);
>  		dev = __dev_get_by_name(net, nlrule->iifname);
>  		if (dev)
>  			nlrule->iifindex = dev->ifindex;
> @@ -573,7 +573,7 @@ static int fib_nl2rule(struct sk_buff *skb, struct nlmsghdr *nlh,
>  		struct net_device *dev;
>  
>  		nlrule->oifindex = -1;
> -		nla_strlcpy(nlrule->oifname, tb[FRA_OIFNAME], IFNAMSIZ);
> +		nla_strcpy(nlrule->oifname, tb[FRA_OIFNAME], IFNAMSIZ);
>  		dev = __dev_get_by_name(net, nlrule->oifname);
>  		if (dev)
>  			nlrule->oifindex = dev->ifindex;
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 68e0682450c6..5d70113b4f25 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1939,7 +1939,7 @@ static const struct rtnl_link_ops *linkinfo_to_kind_ops(const struct nlattr *nla
>  	if (linfo[IFLA_INFO_KIND]) {
>  		char kind[MODULE_NAME_LEN];
>  
> -		nla_strlcpy(kind, linfo[IFLA_INFO_KIND], sizeof(kind));
> +		nla_strcpy(kind, linfo[IFLA_INFO_KIND], sizeof(kind));
>  		ops = rtnl_link_ops_get(kind);
>  	}
>  
> @@ -2953,9 +2953,9 @@ static struct net_device *rtnl_dev_get(struct net *net,
>  	if (!ifname) {
>  		ifname = buffer;
>  		if (ifname_attr)
> -			nla_strlcpy(ifname, ifname_attr, IFNAMSIZ);
> +			nla_strcpy(ifname, ifname_attr, IFNAMSIZ);
>  		else if (altifname_attr)
> -			nla_strlcpy(ifname, altifname_attr, ALTIFNAMSIZ);
> +			nla_strcpy(ifname, altifname_attr, ALTIFNAMSIZ);
>  		else
>  			return NULL;
>  	}
> @@ -2983,7 +2983,7 @@ static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh,
>  		goto errout;
>  
>  	if (tb[IFLA_IFNAME])
> -		nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
> +		nla_strcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
>  	else
>  		ifname[0] = '\0';
>  
> @@ -3264,7 +3264,7 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
>  		return err;
>  
>  	if (tb[IFLA_IFNAME])
> -		nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
> +		nla_strcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
>  	else
>  		ifname[0] = '\0';
>  
> @@ -3296,7 +3296,7 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
>  		memset(linkinfo, 0, sizeof(linkinfo));
>  
>  	if (linkinfo[IFLA_INFO_KIND]) {
> -		nla_strlcpy(kind, linkinfo[IFLA_INFO_KIND], sizeof(kind));
> +		nla_strcpy(kind, linkinfo[IFLA_INFO_KIND], sizeof(kind));
>  		ops = rtnl_link_ops_get(kind);
>  	} else {
>  		kind[0] = '\0';
> diff --git a/net/decnet/dn_dev.c b/net/decnet/dn_dev.c
> index 15d42353f1a3..e6c83289eecc 100644
> --- a/net/decnet/dn_dev.c
> +++ b/net/decnet/dn_dev.c
> @@ -658,7 +658,7 @@ static int dn_nl_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
>  	ifa->ifa_dev = dn_db;
>  
>  	if (tb[IFA_LABEL])
> -		nla_strlcpy(ifa->ifa_label, tb[IFA_LABEL], IFNAMSIZ);
> +		nla_strcpy(ifa->ifa_label, tb[IFA_LABEL], IFNAMSIZ);
>  	else
>  		memcpy(ifa->ifa_label, dev->name, IFNAMSIZ);
>  
> diff --git a/net/ieee802154/nl-mac.c b/net/ieee802154/nl-mac.c
> index 6d091e419d3e..45472ba629bb 100644
> --- a/net/ieee802154/nl-mac.c
> +++ b/net/ieee802154/nl-mac.c
> @@ -149,7 +149,7 @@ static struct net_device *ieee802154_nl_get_dev(struct genl_info *info)
>  	if (info->attrs[IEEE802154_ATTR_DEV_NAME]) {
>  		char name[IFNAMSIZ + 1];
>  
> -		nla_strlcpy(name, info->attrs[IEEE802154_ATTR_DEV_NAME],
> +		nla_strcpy(name, info->attrs[IEEE802154_ATTR_DEV_NAME],
>  			    sizeof(name));
>  		dev = dev_get_by_name(&init_net, name);
>  	} else if (info->attrs[IEEE802154_ATTR_DEV_INDEX]) {
> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index 123a6d39438f..5fbb6bee6d82 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -881,7 +881,7 @@ static struct in_ifaddr *rtm_to_ifaddr(struct net *net, struct nlmsghdr *nlh,
>  		ifa->ifa_broadcast = nla_get_in_addr(tb[IFA_BROADCAST]);
>  
>  	if (tb[IFA_LABEL])
> -		nla_strlcpy(ifa->ifa_label, tb[IFA_LABEL], IFNAMSIZ);
> +		nla_strcpy(ifa->ifa_label, tb[IFA_LABEL], IFNAMSIZ);
>  	else
>  		memcpy(ifa->ifa_label, dev->name, IFNAMSIZ);
>  
> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
> index 1f75dc686b6b..8437d46a30c9 100644
> --- a/net/ipv4/fib_semantics.c
> +++ b/net/ipv4/fib_semantics.c
> @@ -973,7 +973,7 @@ bool fib_metrics_match(struct fib_config *cfg, struct fib_info *fi)
>  			char tmp[TCP_CA_NAME_MAX];
>  			bool ecn_ca = false;
>  
> -			nla_strlcpy(tmp, nla, sizeof(tmp));
> +			nla_strcpy(tmp, nla, sizeof(tmp));
>  			val = tcp_ca_get_key_by_name(fi->fib_net, tmp, &ecn_ca);
>  		} else {
>  			if (nla_len(nla) != sizeof(u32))
> diff --git a/net/ipv4/metrics.c b/net/ipv4/metrics.c
> index 3205d5f7c8c9..99844fd6975d 100644
> --- a/net/ipv4/metrics.c
> +++ b/net/ipv4/metrics.c
> @@ -31,7 +31,7 @@ static int ip_metrics_convert(struct net *net, struct nlattr *fc_mx,
>  		if (type == RTAX_CC_ALGO) {
>  			char tmp[TCP_CA_NAME_MAX];
>  
> -			nla_strlcpy(tmp, nla, sizeof(tmp));
> +			nla_strcpy(tmp, nla, sizeof(tmp));
>  			val = tcp_ca_get_key_by_name(net, tmp, &ecn_ca);
>  			if (val == TCP_CA_UNSPEC) {
>  				NL_SET_ERR_MSG(extack, "Unknown tcp congestion algorithm");
> diff --git a/net/netfilter/ipset/ip_set_hash_netiface.c b/net/netfilter/ipset/ip_set_hash_netiface.c
> index be5e95a0d876..5e114985ad9d 100644
> --- a/net/netfilter/ipset/ip_set_hash_netiface.c
> +++ b/net/netfilter/ipset/ip_set_hash_netiface.c
> @@ -225,7 +225,7 @@ hash_netiface4_uadt(struct ip_set *set, struct nlattr *tb[],
>  		if (e.cidr > HOST_MASK)
>  			return -IPSET_ERR_INVALID_CIDR;
>  	}
> -	nla_strlcpy(e.iface, tb[IPSET_ATTR_IFACE], IFNAMSIZ);
> +	nla_strcpy(e.iface, tb[IPSET_ATTR_IFACE], IFNAMSIZ);
>  
>  	if (tb[IPSET_ATTR_CADT_FLAGS]) {
>  		u32 cadt_flags = ip_set_get_h32(tb[IPSET_ATTR_CADT_FLAGS]);
> @@ -442,7 +442,7 @@ hash_netiface6_uadt(struct ip_set *set, struct nlattr *tb[],
>  
>  	ip6_netmask(&e.ip, e.cidr);
>  
> -	nla_strlcpy(e.iface, tb[IPSET_ATTR_IFACE], IFNAMSIZ);
> +	nla_strcpy(e.iface, tb[IPSET_ATTR_IFACE], IFNAMSIZ);
>  
>  	if (tb[IPSET_ATTR_CADT_FLAGS]) {
>  		u32 cadt_flags = ip_set_get_h32(tb[IPSET_ATTR_CADT_FLAGS]);
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 9957e0ed8658..38eb14ffbc4f 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -1281,7 +1281,7 @@ static struct nft_chain *nft_chain_lookup(struct net *net,
>  	if (nla == NULL)
>  		return ERR_PTR(-EINVAL);
>  
> -	nla_strlcpy(search, nla, sizeof(search));
> +	nla_strcpy(search, nla, sizeof(search));
>  
>  	WARN_ON(!rcu_read_lock_held() &&
>  		!lockdep_commit_lock_is_held(net));
> @@ -1721,7 +1721,7 @@ static struct nft_hook *nft_netdev_hook_alloc(struct net *net,
>  		goto err_hook_alloc;
>  	}
>  
> -	nla_strlcpy(ifname, attr, IFNAMSIZ);
> +	nla_strcpy(ifname, attr, IFNAMSIZ);
>  	dev = __dev_get_by_name(net, ifname);
>  	if (!dev) {
>  		err = -ENOENT;
> @@ -5734,7 +5734,7 @@ struct nft_object *nft_obj_lookup(const struct net *net,
>  	struct rhlist_head *tmp, *list;
>  	struct nft_object *obj;
>  
> -	nla_strlcpy(search, nla, sizeof(search));
> +	nla_strcpy(search, nla, sizeof(search));
>  	k.name = search;
>  
>  	WARN_ON_ONCE(!rcu_read_lock_held() &&
> diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c
> index 5bfec829c12f..d657749724da 100644
> --- a/net/netfilter/nfnetlink_acct.c
> +++ b/net/netfilter/nfnetlink_acct.c
> @@ -112,7 +112,7 @@ static int nfnl_acct_new(struct net *net, struct sock *nfnl,
>  		nfacct->flags = flags;
>  	}
>  
> -	nla_strlcpy(nfacct->name, tb[NFACCT_NAME], NFACCT_NAME_MAX);
> +	nla_strcpy(nfacct->name, tb[NFACCT_NAME], NFACCT_NAME_MAX);
>  
>  	if (tb[NFACCT_BYTES]) {
>  		atomic64_set(&nfacct->bytes,
> diff --git a/net/netfilter/nfnetlink_cthelper.c b/net/netfilter/nfnetlink_cthelper.c
> index 5b0d0a77379c..1900895fa3d5 100644
> --- a/net/netfilter/nfnetlink_cthelper.c
> +++ b/net/netfilter/nfnetlink_cthelper.c
> @@ -146,7 +146,7 @@ nfnl_cthelper_expect_policy(struct nf_conntrack_expect_policy *expect_policy,
>  	    !tb[NFCTH_POLICY_EXPECT_TIMEOUT])
>  		return -EINVAL;
>  
> -	nla_strlcpy(expect_policy->name,
> +	nla_strcpy(expect_policy->name,
>  		    tb[NFCTH_POLICY_NAME], NF_CT_HELPER_NAME_LEN);
>  	expect_policy->max_expected =
>  		ntohl(nla_get_be32(tb[NFCTH_POLICY_EXPECT_MAX]));
> @@ -233,7 +233,7 @@ nfnl_cthelper_create(const struct nlattr * const tb[],
>  	if (ret < 0)
>  		goto err1;
>  
> -	nla_strlcpy(helper->name,
> +	nla_strcpy(helper->name,
>  		    tb[NFCTH_NAME], NF_CT_HELPER_NAME_LEN);
>  	size = ntohl(nla_get_be32(tb[NFCTH_PRIV_DATA_LEN]));
>  	if (size > sizeof_field(struct nf_conn_help, data)) {
> diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
> index 322bd674963e..bb02ea2ef1bf 100644
> --- a/net/netfilter/nft_ct.c
> +++ b/net/netfilter/nft_ct.c
> @@ -990,7 +990,7 @@ static int nft_ct_helper_obj_init(const struct nft_ctx *ctx,
>  	if (!priv->l4proto)
>  		return -ENOENT;
>  
> -	nla_strlcpy(name, tb[NFTA_CT_HELPER_NAME], sizeof(name));
> +	nla_strcpy(name, tb[NFTA_CT_HELPER_NAME], sizeof(name));
>  
>  	if (tb[NFTA_CT_HELPER_L3PROTO])
>  		family = ntohs(nla_get_be16(tb[NFTA_CT_HELPER_L3PROTO]));
> diff --git a/net/netfilter/nft_log.c b/net/netfilter/nft_log.c
> index 57899454a530..b55607b24942 100644
> --- a/net/netfilter/nft_log.c
> +++ b/net/netfilter/nft_log.c
> @@ -152,7 +152,7 @@ static int nft_log_init(const struct nft_ctx *ctx,
>  		priv->prefix = kmalloc(nla_len(nla) + 1, GFP_KERNEL);
>  		if (priv->prefix == NULL)
>  			return -ENOMEM;
> -		nla_strlcpy(priv->prefix, nla, nla_len(nla) + 1);
> +		nla_strcpy(priv->prefix, nla, nla_len(nla) + 1);
>  	} else {
>  		priv->prefix = (char *)nft_log_null_prefix;
>  	}
> diff --git a/net/netlabel/netlabel_mgmt.c b/net/netlabel/netlabel_mgmt.c
> index eb1d66d20afb..3dd925c39ef4 100644
> --- a/net/netlabel/netlabel_mgmt.c
> +++ b/net/netlabel/netlabel_mgmt.c
> @@ -95,7 +95,7 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
>  			ret_val = -ENOMEM;
>  			goto add_free_entry;
>  		}
> -		nla_strlcpy(entry->domain,
> +		nla_strcpy(entry->domain,
>  			    info->attrs[NLBL_MGMT_A_DOMAIN], tmp_size);
>  	}
>  
> diff --git a/net/nfc/netlink.c b/net/nfc/netlink.c
> index e894254c17d4..ee4a53d15ae3 100644
> --- a/net/nfc/netlink.c
> +++ b/net/nfc/netlink.c
> @@ -1226,7 +1226,7 @@ static int nfc_genl_fw_download(struct sk_buff *skb, struct genl_info *info)
>  	if (!dev)
>  		return -ENODEV;
>  
> -	nla_strlcpy(firmware_name, info->attrs[NFC_ATTR_FIRMWARE_NAME],
> +	nla_strcpy(firmware_name, info->attrs[NFC_ATTR_FIRMWARE_NAME],
>  		    sizeof(firmware_name));
>  
>  	rc = nfc_fw_download(dev, firmware_name);
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index ed411d6c29fc..9c001af0c933 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -935,7 +935,7 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
>  			NL_SET_ERR_MSG(extack, "TC action kind must be specified");
>  			goto err_out;
>  		}
> -		if (nla_strlcpy(act_name, kind, IFNAMSIZ) == -E2BIG) {
> +		if (nla_strcpy(act_name, kind, IFNAMSIZ) == -E2BIG) {
>  			NL_SET_ERR_MSG(extack, "TC action name too long");
>  			goto err_out;
>  		}
> diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
> index 8dc3bec0d325..9ab92a781104 100644
> --- a/net/sched/act_ipt.c
> +++ b/net/sched/act_ipt.c
> @@ -166,7 +166,7 @@ static int __tcf_ipt_init(struct net *net, unsigned int id, struct nlattr *nla,
>  	if (unlikely(!tname))
>  		goto err1;
>  	if (tb[TCA_IPT_TABLE] == NULL ||
> -	    nla_strlcpy(tname, tb[TCA_IPT_TABLE], IFNAMSIZ) >= IFNAMSIZ)
> +	    nla_strcpy(tname, tb[TCA_IPT_TABLE], IFNAMSIZ) >= IFNAMSIZ)
>  		strcpy(tname, "mangle");
>  
>  	t = kmemdup(td, td->u.target_size, GFP_KERNEL);
> diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
> index a4f3d0f0daa9..6509dae17357 100644
> --- a/net/sched/act_simple.c
> +++ b/net/sched/act_simple.c
> @@ -52,7 +52,7 @@ static int alloc_defdata(struct tcf_defact *d, const struct nlattr *defdata)
>  	d->tcfd_defdata = kzalloc(SIMP_MAX_DATA, GFP_KERNEL);
>  	if (unlikely(!d->tcfd_defdata))
>  		return -ENOMEM;
> -	nla_strlcpy(d->tcfd_defdata, defdata, SIMP_MAX_DATA);
> +	nla_strcpy(d->tcfd_defdata, defdata, SIMP_MAX_DATA);
>  	return 0;
>  }
>  
> @@ -71,7 +71,7 @@ static int reset_policy(struct tc_action *a, const struct nlattr *defdata,
>  	spin_lock_bh(&d->tcf_lock);
>  	goto_ch = tcf_action_set_ctrlact(a, p->action, goto_ch);
>  	memset(d->tcfd_defdata, 0, SIMP_MAX_DATA);
> -	nla_strlcpy(d->tcfd_defdata, defdata, SIMP_MAX_DATA);
> +	nla_strcpy(d->tcfd_defdata, defdata, SIMP_MAX_DATA);
>  	spin_unlock_bh(&d->tcf_lock);
>  	if (goto_ch)
>  		tcf_chain_put_by_act(goto_ch);
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index 41a55c6cbeb8..b96573c4a8a7 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -223,7 +223,7 @@ static inline u32 tcf_auto_prio(struct tcf_proto *tp)
>  static bool tcf_proto_check_kind(struct nlattr *kind, char *name)
>  {
>  	if (kind)
> -		return nla_strlcpy(name, kind, IFNAMSIZ) >= IFNAMSIZ;
> +		return nla_strcpy(name, kind, IFNAMSIZ) >= IFNAMSIZ;
>  	memset(name, 0, IFNAMSIZ);
>  	return false;
>  }
> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> index 3e89c666d1e8..bc764da38fb5 100644
> --- a/net/sched/sch_api.c
> +++ b/net/sched/sch_api.c
> @@ -1170,7 +1170,7 @@ static struct Qdisc *qdisc_create(struct net_device *dev,
>  #ifdef CONFIG_MODULES
>  	if (ops == NULL && kind != NULL) {
>  		char name[IFNAMSIZ];
> -		if (nla_strlcpy(name, kind, IFNAMSIZ) != -E2BIG) {
> +		if (nla_strcpy(name, kind, IFNAMSIZ) != -E2BIG) {
>  			/* We dropped the RTNL semaphore in order to
>  			 * perform the module load.  So, even if we
>  			 * succeeded in loading the module we have to
> diff --git a/net/tipc/netlink_compat.c b/net/tipc/netlink_compat.c
> index 1c7aa51cc2a3..c973a61fe440 100644
> --- a/net/tipc/netlink_compat.c
> +++ b/net/tipc/netlink_compat.c
> @@ -695,7 +695,7 @@ static int tipc_nl_compat_link_dump(struct tipc_nl_compat_msg *msg,
>  
>  	link_info.dest = nla_get_flag(link[TIPC_NLA_LINK_DEST]);
>  	link_info.up = htonl(nla_get_flag(link[TIPC_NLA_LINK_UP]));
> -	nla_strlcpy(link_info.str, link[TIPC_NLA_LINK_NAME],
> +	nla_strcpy(link_info.str, link[TIPC_NLA_LINK_NAME],
>  		    TIPC_MAX_LINK_NAME);
>  
>  	return tipc_add_tlv(msg->rep, TIPC_TLV_LINK_INFO,
> -- 
> 2.20.1
> 

-- 
Kees Cook

[RFC][PATCH v2 0/3] Fix inefficiences and rename nla_strlcpy

[laniel_francis]

From: Francis Laniel <laniel_francis@privacyrequired.com>

Hi.


I hope your families, friends and yourselves are fine.

This patch set answers to first three issues listed in:
https://github.com/KSPP/linux/issues/110

To sum up, the first patch fixes an inefficiency where some bytes in dst were
written twice, one with 0 the other with src content.
The second one modifies nla_strlcpy to return the same value as strscpy,
i.e. number of bytes written or -E2BIG if src was truncated.
The third rename nla_strlcpy to nla_strcpy.

Unfortunately, I did not find how to create struct nlattr objects so I tested
my modifications on simple char*.
This is why I tag this patch set as RFC.

If you see any way to improve the code or have any remark, feel free to comment.


Best regards.

Francis Laniel (3):
  Fix unefficient call to memset before memcpu in nla_strlcpy.
  Modify return value of nla_strlcpy to match that of strscpy.
  Rename nla_strlcpy to nla_strcpy.

 drivers/infiniband/core/nldev.c            | 10 +++---
 drivers/net/can/vxcan.c                    |  4 +--
 drivers/net/veth.c                         |  4 +--
 include/linux/genl_magic_struct.h          |  2 +-
 include/net/netlink.h                      |  4 +--
 include/net/pkt_cls.h                      |  3 +-
 kernel/taskstats.c                         |  2 +-
 lib/nlattr.c                               | 36 ++++++++++++++--------
 net/core/fib_rules.c                       |  4 +--
 net/core/rtnetlink.c                       | 12 ++++----
 net/decnet/dn_dev.c                        |  2 +-
 net/ieee802154/nl-mac.c                    |  2 +-
 net/ipv4/devinet.c                         |  2 +-
 net/ipv4/fib_semantics.c                   |  2 +-
 net/ipv4/metrics.c                         |  2 +-
 net/netfilter/ipset/ip_set_hash_netiface.c |  4 +--
 net/netfilter/nf_tables_api.c              |  6 ++--
 net/netfilter/nfnetlink_acct.c             |  2 +-
 net/netfilter/nfnetlink_cthelper.c         |  4 +--
 net/netfilter/nft_ct.c                     |  2 +-
 net/netfilter/nft_log.c                    |  2 +-
 net/netlabel/netlabel_mgmt.c               |  2 +-
 net/nfc/netlink.c                          |  2 +-
 net/sched/act_api.c                        |  2 +-
 net/sched/act_ipt.c                        |  2 +-
 net/sched/act_simple.c                     |  4 +--
 net/sched/cls_api.c                        |  2 +-
 net/sched/sch_api.c                        |  2 +-
 net/tipc/netlink_compat.c                  |  2 +-
 29 files changed, 70 insertions(+), 59 deletions(-)

-- 
2.20.1

[RFC][PATCH v2 1/3] Fix unefficient call to memset before memcpu in nla_strlcpy.

[laniel_francis]

From: Francis Laniel <laniel_francis@privacyrequired.com>

Before this commit, nla_strlcpy first memseted dst to 0 then wrote src into it.
This is inefficient because bytes whom number is less than src length are written
twice.

This patch solves this issue by first writing src into dst then fill dst with
0's.
Note that, in the case where src length is higher than dst, only 0 is written.
Otherwise there are as many 0's written to fill dst.

For example, if src is "foo\0" and dst is 5 bytes long, the result will be:
1. "fooGG" after memcpy (G means garbage).
2. "foo\0\0" after memset.

Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
---
 lib/nlattr.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/nlattr.c b/lib/nlattr.c
index 74019c8ebf6b..07156e581997 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -731,8 +731,9 @@ size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
 	if (dstsize > 0) {
 		size_t len = (srclen >= dstsize) ? dstsize - 1 : srclen;
 
-		memset(dst, 0, dstsize);
 		memcpy(dst, src, len);
+		/* Zero pad end of dst. */
+		memset(dst + len, 0, dstsize - len);
 	}
 
 	return srclen;
-- 
2.20.1

[RFC][PATCH v2 2/3] Modify return value of nla_strlcpy to match that of strscpy.

[laniel_francis]

From: Francis Laniel <laniel_francis@privacyrequired.com>

nla_strlcpy now returns -E2BIG if src was truncated when written to dst.
It also returns this error value if dstsize is 0 or higher than INT_MAX.

For example, if src is "foo\0" and dst is 3 bytes long, the result will be:
1. "foG" after memcpy (G means garbage).
2. "fo\0" after memset.
3. -E2BIG is returned because src was not completely written into dst.

The callers of nla_strlcpy were modified to take into account this modification.

Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
---
 include/net/netlink.h |  2 +-
 include/net/pkt_cls.h |  3 ++-
 lib/nlattr.c          | 33 +++++++++++++++++++++------------
 net/sched/act_api.c   |  2 +-
 net/sched/cls_api.c   |  2 +-
 net/sched/sch_api.c   |  2 +-
 6 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/include/net/netlink.h b/include/net/netlink.h
index 7356f41d23ba..446ca182e13d 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -506,7 +506,7 @@ int __nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
 		struct netlink_ext_ack *extack);
 int nla_policy_len(const struct nla_policy *, int);
 struct nlattr *nla_find(const struct nlattr *head, int len, int attrtype);
-size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize);
+ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize);
 char *nla_strdup(const struct nlattr *nla, gfp_t flags);
 int nla_memcpy(void *dest, const struct nlattr *src, int count);
 int nla_memcmp(const struct nlattr *nla, const void *data, size_t size);
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index d4d461236351..85f4ac779399 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -4,6 +4,7 @@
 
 #include <linux/pkt_cls.h>
 #include <linux/workqueue.h>
+#include <linux/errno.h>
 #include <net/sch_generic.h>
 #include <net/act_api.h>
 #include <net/net_namespace.h>
@@ -512,7 +513,7 @@ tcf_change_indev(struct net *net, struct nlattr *indev_tlv,
 	char indev[IFNAMSIZ];
 	struct net_device *dev;
 
-	if (nla_strlcpy(indev, indev_tlv, IFNAMSIZ) >= IFNAMSIZ) {
+	if (nla_strlcpy(indev, indev_tlv, IFNAMSIZ) < 0) {
 		NL_SET_ERR_MSG_ATTR(extack, indev_tlv,
 				    "Interface name too long");
 		return -EINVAL;
diff --git a/lib/nlattr.c b/lib/nlattr.c
index 07156e581997..d692716bda78 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -713,30 +713,39 @@ EXPORT_SYMBOL(nla_find);
  * @dst: where to copy the string to
  * @nla: attribute to copy the string from
  * @dstsize: size of destination buffer
+ * @returns: -E2BIG if @dstsize is 0 or source buffer length greater than
+ * @dstsize, otherwise it returns the number of copied characters (not
+ * including the trailing %NUL).
  *
  * Copies at most dstsize - 1 bytes into the destination buffer.
- * The result is always a valid NUL-terminated string. Unlike
- * strlcpy the destination buffer is always padded out.
- *
- * Returns the length of the source buffer.
+ * Unlike strlcpy the destination buffer is always padded out.
  */
-size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
+ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
 {
+	size_t len;
+	ssize_t ret;
 	size_t srclen = nla_len(nla);
 	char *src = nla_data(nla);
 
+	if (dstsize == 0 || WARN_ON_ONCE(dstsize > INT_MAX))
+		return -E2BIG;
+
 	if (srclen > 0 && src[srclen - 1] == '\0')
 		srclen--;
 
-	if (dstsize > 0) {
-		size_t len = (srclen >= dstsize) ? dstsize - 1 : srclen;
-
-		memcpy(dst, src, len);
-		/* Zero pad end of dst. */
-		memset(dst + len, 0, dstsize - len);
+	if (srclen >= dstsize) {
+		len = dstsize - 1;
+		ret = -E2BIG;
+	} else {
+		len = srclen;
+		ret = len;
 	}
 
-	return srclen;
+	memcpy(dst, src, len);
+	/* Zero pad end of dst. */
+	memset(dst + len, 0, dstsize - len);
+
+	return ret;
 }
 EXPORT_SYMBOL(nla_strlcpy);
 
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index f66417d5d2c3..541574520c52 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -935,7 +935,7 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
 			NL_SET_ERR_MSG(extack, "TC action kind must be specified");
 			goto err_out;
 		}
-		if (nla_strlcpy(act_name, kind, IFNAMSIZ) >= IFNAMSIZ) {
+		if (nla_strlcpy(act_name, kind, IFNAMSIZ) < 0) {
 			NL_SET_ERR_MSG(extack, "TC action name too long");
 			goto err_out;
 		}
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 41a55c6cbeb8..f0bf64393cbf 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -223,7 +223,7 @@ static inline u32 tcf_auto_prio(struct tcf_proto *tp)
 static bool tcf_proto_check_kind(struct nlattr *kind, char *name)
 {
 	if (kind)
-		return nla_strlcpy(name, kind, IFNAMSIZ) >= IFNAMSIZ;
+		return nla_strlcpy(name, kind, IFNAMSIZ) > 0;
 	memset(name, 0, IFNAMSIZ);
 	return false;
 }
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 2a76a2f5ed88..f9b053b30a7b 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1170,7 +1170,7 @@ static struct Qdisc *qdisc_create(struct net_device *dev,
 #ifdef CONFIG_MODULES
 	if (ops == NULL && kind != NULL) {
 		char name[IFNAMSIZ];
-		if (nla_strlcpy(name, kind, IFNAMSIZ) < IFNAMSIZ) {
+		if (nla_strlcpy(name, kind, IFNAMSIZ) > 0) {
 			/* We dropped the RTNL semaphore in order to
 			 * perform the module load.  So, even if we
 			 * succeeded in loading the module we have to
-- 
2.20.1

Re: [RFC][PATCH v2 2/3] Modify return value of nla_strlcpy to match that of strscpy.

[Jakub Kicinski]

On Mon, 19 Oct 2020 17:23:30 +0200 laniel_francis@privacyrequired.com
wrote:
> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
> index d4d461236351..85f4ac779399 100644
> --- a/include/net/pkt_cls.h
> +++ b/include/net/pkt_cls.h
> @@ -4,6 +4,7 @@
>  
>  #include <linux/pkt_cls.h>
>  #include <linux/workqueue.h>
> +#include <linux/errno.h>

Stray include.

>  #include <net/sch_generic.h>
>  #include <net/act_api.h>
>  #include <net/net_namespace.h>
> diff --git a/lib/nlattr.c b/lib/nlattr.c
> index 07156e581997..d692716bda78 100644
> --- a/lib/nlattr.c
> +++ b/lib/nlattr.c
> @@ -713,30 +713,39 @@ EXPORT_SYMBOL(nla_find);
>   * @dst: where to copy the string to
>   * @nla: attribute to copy the string from
>   * @dstsize: size of destination buffer
> + * @returns: -E2BIG if @dstsize is 0 or source buffer length greater than

I don't think this is correct format for kdoc.

> + * @dstsize, otherwise it returns the number of copied characters (not
> + * including the trailing %NUL).
>   *
>   * Copies at most dstsize - 1 bytes into the destination buffer.
> - * The result is always a valid NUL-terminated string. Unlike
> - * strlcpy the destination buffer is always padded out.
> - *
> - * Returns the length of the source buffer.
> + * Unlike strlcpy the destination buffer is always padded out.
>   */
> -size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
> +ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
>  {
> +	size_t len;
> +	ssize_t ret;
>  	size_t srclen = nla_len(nla);
>  	char *src = nla_data(nla);

Sort local variables long to short.

>  
> +	if (dstsize == 0 || WARN_ON_ONCE(dstsize > INT_MAX))

You can make it > U16_MAX, attr len is 16 bit.

> +		return -E2BIG;
> +
>  	if (srclen > 0 && src[srclen - 1] == '\0')
>  		srclen--;
>  
> -	if (dstsize > 0) {
> -		size_t len = (srclen >= dstsize) ? dstsize - 1 : srclen;
> -
> -		memcpy(dst, src, len);
> -		/* Zero pad end of dst. */
> -		memset(dst + len, 0, dstsize - len);
> +	if (srclen >= dstsize) {
> +		len = dstsize - 1;
> +		ret = -E2BIG;
> +	} else {
> +		len = srclen;
> +		ret = len;
>  	}
>  
> -	return srclen;
> +	memcpy(dst, src, len);
> +	/* Zero pad end of dst. */
> +	memset(dst + len, 0, dstsize - len);
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL(nla_strlcpy);
>  

> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index 41a55c6cbeb8..f0bf64393cbf 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -223,7 +223,7 @@ static inline u32 tcf_auto_prio(struct tcf_proto *tp)
>  static bool tcf_proto_check_kind(struct nlattr *kind, char *name)
>  {
>  	if (kind)
> -		return nla_strlcpy(name, kind, IFNAMSIZ) >= IFNAMSIZ;
> +		return nla_strlcpy(name, kind, IFNAMSIZ) > 0;

Bug.

>  	memset(name, 0, IFNAMSIZ);
>  	return false;
>  }

Re: [RFC][PATCH v2 2/3] Modify return value of nla_strlcpy to match that of strscpy.

[Kees Cook]

On Mon, Oct 19, 2020 at 09:43:55AM -0700, Jakub Kicinski wrote:
> On Mon, 19 Oct 2020 17:23:30 +0200 laniel_francis@privacyrequired.com
> wrote:
> > -size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
> > +ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
> >  {
> > +	size_t len;
> > +	ssize_t ret;
> >  	size_t srclen = nla_len(nla);
> >  	char *src = nla_data(nla);
> 
> Sort local variables long to short.

Specifically, "reverse christmas tree":

 	size_t srclen = nla_len(nla);
 	char *src = nla_data(nla);
 	size_t len;
 	ssize_t ret;


> > diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> > index 41a55c6cbeb8..f0bf64393cbf 100644
> > --- a/net/sched/cls_api.c
> > +++ b/net/sched/cls_api.c
> > @@ -223,7 +223,7 @@ static inline u32 tcf_auto_prio(struct tcf_proto *tp)
> >  static bool tcf_proto_check_kind(struct nlattr *kind, char *name)
> >  {
> >  	if (kind)
> > -		return nla_strlcpy(name, kind, IFNAMSIZ) >= IFNAMSIZ;
> > +		return nla_strlcpy(name, kind, IFNAMSIZ) > 0;
> 
> Bug.
> 
> >  	memset(name, 0, IFNAMSIZ);
> >  	return false;
> >  }

Have you been able to exercise the changed code paths? (I would have
expected this to immediately fail, for example.)

-- 
Kees Cook

Re: [RFC][PATCH v2 2/3] Modify return value of nla_strlcpy to match that of strscpy.

[Jakub Kicinski]

On Mon, 19 Oct 2020 16:01:27 -0700 Kees Cook wrote:
> On Mon, Oct 19, 2020 at 09:43:55AM -0700, Jakub Kicinski wrote:
> > On Mon, 19 Oct 2020 17:23:30 +0200 laniel_francis@privacyrequired.com
> > wrote:  
> > > -size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
> > > +ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
> > >  {
> > > +	size_t len;
> > > +	ssize_t ret;
> > >  	size_t srclen = nla_len(nla);
> > >  	char *src = nla_data(nla);  
> > 
> > Sort local variables long to short.  
> 
> Specifically, "reverse christmas tree":
> 
>  	size_t srclen = nla_len(nla);
>  	char *src = nla_data(nla);
>  	size_t len;
>  	ssize_t ret;

Or even

 	size_t srclen = nla_len(nla);
 	char *src = nla_data(nla);
 	ssize_t ret;
 	size_t len;

;)

Re: [RFC][PATCH v2 2/3] Modify return value of nla_strlcpy to match that of strscpy.

[Francis Laniel]

Le mardi 20 octobre 2020, 01:34:49 CEST Jakub Kicinski a écrit :
> On Mon, 19 Oct 2020 16:01:27 -0700 Kees Cook wrote:
> > On Mon, Oct 19, 2020 at 09:43:55AM -0700, Jakub Kicinski wrote:
> > > On Mon, 19 Oct 2020 17:23:30 +0200 laniel_francis@privacyrequired.com
> > > 
> > > wrote:
> > > > -size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t
> > > > dstsize)
> > > > +ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t
> > > > dstsize)
> > > > 
> > > >  {
> > > > 
> > > > +	size_t len;
> > > > +	ssize_t ret;
> > > > 
> > > >  	size_t srclen = nla_len(nla);
> > > >  	char *src = nla_data(nla);
> > > 
> > > Sort local variables long to short.
> > 
> > Specifically, "reverse christmas tree":
> >  	size_t srclen = nla_len(nla);
> >  	char *src = nla_data(nla);
> >  	size_t len;
> >  	ssize_t ret;
> 
> Or even
> 
>  	size_t srclen = nla_len(nla);
>  	char *src = nla_data(nla);
>  	ssize_t ret;
>  	size_t len;
> 
> ;)

I reordered the variables names for the v3.
Just to know, is it a new rule? Because scripts/checkpatch.pl did not report 
anything and I was not aware of it.

Re: [RFC][PATCH v2 2/3] Modify return value of nla_strlcpy to match that of strscpy.

[Kees Cook]

On Tue, Oct 20, 2020 at 12:28:49PM +0200, Francis Laniel wrote:
> Le mardi 20 octobre 2020, 01:34:49 CEST Jakub Kicinski a écrit :
> > On Mon, 19 Oct 2020 16:01:27 -0700 Kees Cook wrote:
> > > On Mon, Oct 19, 2020 at 09:43:55AM -0700, Jakub Kicinski wrote:
> > > > On Mon, 19 Oct 2020 17:23:30 +0200 laniel_francis@privacyrequired.com
> > > > 
> > > > wrote:
> > > > > -size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t
> > > > > dstsize)
> > > > > +ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t
> > > > > dstsize)
> > > > > 
> > > > >  {
> > > > > 
> > > > > +	size_t len;
> > > > > +	ssize_t ret;
> > > > > 
> > > > >  	size_t srclen = nla_len(nla);
> > > > >  	char *src = nla_data(nla);
> > > > 
> > > > Sort local variables long to short.
> > > 
> > > Specifically, "reverse christmas tree":
> > >  	size_t srclen = nla_len(nla);
> > >  	char *src = nla_data(nla);
> > >  	size_t len;
> > >  	ssize_t ret;
> > 
> > Or even
> > 
> >  	size_t srclen = nla_len(nla);
> >  	char *src = nla_data(nla);
> >  	ssize_t ret;
> >  	size_t len;
> > 
> > ;)
> 
> I reordered the variables names for the v3.
> Just to know, is it a new rule? Because scripts/checkpatch.pl did not report 
> anything and I was not aware of it.

This is specific to netdev, but I actually can't find reference to this
in either Documentation/process/coding-style.rst nor
Documentation/networking/netdev-FAQ.rst.

I swear I found this written down before, but it eludes me now.

-- 
Kees Cook

Re: [RFC][PATCH v2 2/3] Modify return value of nla_strlcpy to match that of strscpy.

[Kees Cook]

On Mon, Oct 19, 2020 at 04:34:49PM -0700, Jakub Kicinski wrote:
> On Mon, 19 Oct 2020 16:01:27 -0700 Kees Cook wrote:
> > On Mon, Oct 19, 2020 at 09:43:55AM -0700, Jakub Kicinski wrote:
> > > On Mon, 19 Oct 2020 17:23:30 +0200 laniel_francis@privacyrequired.com
> > > wrote:  
> > > > -size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
> > > > +ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
> > > >  {
> > > > +	size_t len;
> > > > +	ssize_t ret;
> > > >  	size_t srclen = nla_len(nla);
> > > >  	char *src = nla_data(nla);  
> > > 
> > > Sort local variables long to short.  
> > 
> > Specifically, "reverse christmas tree":
> > 
> >  	size_t srclen = nla_len(nla);
> >  	char *src = nla_data(nla);
> >  	size_t len;
> >  	ssize_t ret;
> 
> Or even
> 
>  	size_t srclen = nla_len(nla);
>  	char *src = nla_data(nla);
>  	ssize_t ret;
>  	size_t len;
> 
> ;)

Oops, sorry, yes. ENOCOFFEE on my end. :)

-- 
Kees Cook

Re: [RFC][PATCH v2 2/3] Modify return value of nla_strlcpy to match that of strscpy.

[Francis Laniel]

Le mardi 20 octobre 2020, 01:01:27 CEST Kees Cook a écrit :
> On Mon, Oct 19, 2020 at 09:43:55AM -0700, Jakub Kicinski wrote:
> > On Mon, 19 Oct 2020 17:23:30 +0200 laniel_francis@privacyrequired.com
> > 
> > wrote:
> > > -size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
> > > +ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t
> > > dstsize)
> > > 
> > >  {
> > > 
> > > +	size_t len;
> > > +	ssize_t ret;
> > > 
> > >  	size_t srclen = nla_len(nla);
> > >  	char *src = nla_data(nla);
> > 
> > Sort local variables long to short.
> 
> Specifically, "reverse christmas tree":
> 
>  	size_t srclen = nla_len(nla);
>  	char *src = nla_data(nla);
>  	size_t len;
>  	ssize_t ret;
> 
> > > diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> > > index 41a55c6cbeb8..f0bf64393cbf 100644
> > > --- a/net/sched/cls_api.c
> > > +++ b/net/sched/cls_api.c
> > > @@ -223,7 +223,7 @@ static inline u32 tcf_auto_prio(struct tcf_proto
> > > *tp)
> > > 
> > >  static bool tcf_proto_check_kind(struct nlattr *kind, char *name)
> > >  {
> > >  
> > >  	if (kind)
> > > 
> > > -		return nla_strlcpy(name, kind, IFNAMSIZ) >= IFNAMSIZ;
> > > +		return nla_strlcpy(name, kind, IFNAMSIZ) > 0;
> > 
> > Bug.
> > 
> > >  	memset(name, 0, IFNAMSIZ);
> > >  	return false;
> > >  
> > >  }
> 
> Have you been able to exercise the changed code paths? (I would have
> expected this to immediately fail, for example.)

Unfortunately no...
As I said in the cover letter I only tested the modifications on char* and the 
tcf_proto_check_kind function seems not to be called in my VM...
I will try to trigger it though!

Re: [RFC][PATCH v2 2/3] Modify return value of nla_strlcpy to match that of strscpy.

[Francis Laniel]

Le lundi 19 octobre 2020, 18:43:55 CEST Jakub Kicinski a écrit :
> On Mon, 19 Oct 2020 17:23:30 +0200 laniel_francis@privacyrequired.com
> 
> wrote:
> > diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
> > index d4d461236351..85f4ac779399 100644
> > --- a/include/net/pkt_cls.h
> > +++ b/include/net/pkt_cls.h
> > @@ -4,6 +4,7 @@
> > 
> >  #include <linux/pkt_cls.h>
> >  #include <linux/workqueue.h>
> > 
> > +#include <linux/errno.h>
> 
> Stray include.

I removed it from my patch.
Did you use a tool to see this include is not used? If yes, which one?

> >  #include <net/sch_generic.h>
> >  #include <net/act_api.h>
> >  #include <net/net_namespace.h>
> > 
> > diff --git a/lib/nlattr.c b/lib/nlattr.c
> > index 07156e581997..d692716bda78 100644
> > --- a/lib/nlattr.c
> > +++ b/lib/nlattr.c
> > @@ -713,30 +713,39 @@ EXPORT_SYMBOL(nla_find);
> > 
> >   * @dst: where to copy the string to
> >   * @nla: attribute to copy the string from
> >   * @dstsize: size of destination buffer
> > 
> > + * @returns: -E2BIG if @dstsize is 0 or source buffer length greater than
> 
> I don't think this is correct format for kdoc.

I corrected it.
I will take the habit to run scripts/kernel-doc on my modifications.

> > + * @dstsize, otherwise it returns the number of copied characters (not
> > + * including the trailing %NUL).
> > 
> >   *
> >   * Copies at most dstsize - 1 bytes into the destination buffer.
> > 
> > - * The result is always a valid NUL-terminated string. Unlike
> > - * strlcpy the destination buffer is always padded out.
> > - *
> > - * Returns the length of the source buffer.
> > + * Unlike strlcpy the destination buffer is always padded out.
> > 
> >   */
> > 
> > -size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
> > +ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
> > 
> >  {
> > 
> > +	size_t len;
> > +	ssize_t ret;
> > 
> >  	size_t srclen = nla_len(nla);
> >  	char *src = nla_data(nla);
> 
> Sort local variables long to short.
> 
> > +	if (dstsize == 0 || WARN_ON_ONCE(dstsize > INT_MAX))
> 
> You can make it > U16_MAX, attr len is 16 bit.

Done for v3!

> 
> > +		return -E2BIG;
> > +
> > 
> >  	if (srclen > 0 && src[srclen - 1] == '\0')
> >  	
> >  		srclen--;
> > 
> > -	if (dstsize > 0) {
> > -		size_t len = (srclen >= dstsize) ? dstsize - 1 : srclen;
> > -
> > -		memcpy(dst, src, len);
> > -		/* Zero pad end of dst. */
> > -		memset(dst + len, 0, dstsize - len);
> > +	if (srclen >= dstsize) {
> > +		len = dstsize - 1;
> > +		ret = -E2BIG;
> > +	} else {
> > +		len = srclen;
> > +		ret = len;
> > 
> >  	}
> > 
> > -	return srclen;
> > +	memcpy(dst, src, len);
> > +	/* Zero pad end of dst. */
> > +	memset(dst + len, 0, dstsize - len);
> > +
> > +	return ret;
> > 
> >  }
> >  EXPORT_SYMBOL(nla_strlcpy);
> > 
> > diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> > index 41a55c6cbeb8..f0bf64393cbf 100644
> > --- a/net/sched/cls_api.c
> > +++ b/net/sched/cls_api.c
> > @@ -223,7 +223,7 @@ static inline u32 tcf_auto_prio(struct tcf_proto *tp)
> > 
> >  static bool tcf_proto_check_kind(struct nlattr *kind, char *name)
> >  {
> >  
> >  	if (kind)
> > 
> > -		return nla_strlcpy(name, kind, IFNAMSIZ) >= IFNAMSIZ;
> > +		return nla_strlcpy(name, kind, IFNAMSIZ) > 0;
> 
> Bug.
> 
> >  	memset(name, 0, IFNAMSIZ);
> >  	return false;
> >  
> >  }

[RFC][PATCH v2 3/3] Rename nla_strlcpy to nla_strcpy.

[laniel_francis]

From: Francis Laniel <laniel_francis@privacyrequired.com>

Calls to nla_strlcpy are now replaced by calls to nla_strcpy which is the new
name of this function.

Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
---
 drivers/infiniband/core/nldev.c            | 10 +++++-----
 drivers/net/can/vxcan.c                    |  4 ++--
 drivers/net/veth.c                         |  4 ++--
 include/linux/genl_magic_struct.h          |  2 +-
 include/net/netlink.h                      |  4 ++--
 include/net/pkt_cls.h                      |  2 +-
 kernel/taskstats.c                         |  2 +-
 lib/nlattr.c                               |  8 ++++----
 net/core/fib_rules.c                       |  4 ++--
 net/core/rtnetlink.c                       | 12 ++++++------
 net/decnet/dn_dev.c                        |  2 +-
 net/ieee802154/nl-mac.c                    |  2 +-
 net/ipv4/devinet.c                         |  2 +-
 net/ipv4/fib_semantics.c                   |  2 +-
 net/ipv4/metrics.c                         |  2 +-
 net/netfilter/ipset/ip_set_hash_netiface.c |  4 ++--
 net/netfilter/nf_tables_api.c              |  6 +++---
 net/netfilter/nfnetlink_acct.c             |  2 +-
 net/netfilter/nfnetlink_cthelper.c         |  4 ++--
 net/netfilter/nft_ct.c                     |  2 +-
 net/netfilter/nft_log.c                    |  2 +-
 net/netlabel/netlabel_mgmt.c               |  2 +-
 net/nfc/netlink.c                          |  2 +-
 net/sched/act_api.c                        |  2 +-
 net/sched/act_ipt.c                        |  2 +-
 net/sched/act_simple.c                     |  4 ++--
 net/sched/cls_api.c                        |  2 +-
 net/sched/sch_api.c                        |  2 +-
 net/tipc/netlink_compat.c                  |  2 +-
 29 files changed, 50 insertions(+), 50 deletions(-)

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 12d29d54a081..10ac54b28ef9 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -932,7 +932,7 @@ static int nldev_set_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (tb[RDMA_NLDEV_ATTR_DEV_NAME]) {
 		char name[IB_DEVICE_NAME_MAX] = {};
 
-		nla_strlcpy(name, tb[RDMA_NLDEV_ATTR_DEV_NAME],
+		nla_strcpy(name, tb[RDMA_NLDEV_ATTR_DEV_NAME],
 			    IB_DEVICE_NAME_MAX);
 		if (strlen(name) == 0) {
 			err = -EINVAL;
@@ -1529,13 +1529,13 @@ static int nldev_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 	    !tb[RDMA_NLDEV_ATTR_LINK_TYPE] || !tb[RDMA_NLDEV_ATTR_NDEV_NAME])
 		return -EINVAL;
 
-	nla_strlcpy(ibdev_name, tb[RDMA_NLDEV_ATTR_DEV_NAME],
+	nla_strcpy(ibdev_name, tb[RDMA_NLDEV_ATTR_DEV_NAME],
 		    sizeof(ibdev_name));
 	if (strchr(ibdev_name, '%') || strlen(ibdev_name) == 0)
 		return -EINVAL;
 
-	nla_strlcpy(type, tb[RDMA_NLDEV_ATTR_LINK_TYPE], sizeof(type));
-	nla_strlcpy(ndev_name, tb[RDMA_NLDEV_ATTR_NDEV_NAME],
+	nla_strcpy(type, tb[RDMA_NLDEV_ATTR_LINK_TYPE], sizeof(type));
+	nla_strcpy(ndev_name, tb[RDMA_NLDEV_ATTR_NDEV_NAME],
 		    sizeof(ndev_name));
 
 	ndev = dev_get_by_name(sock_net(skb->sk), ndev_name);
@@ -1602,7 +1602,7 @@ static int nldev_get_chardev(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (err || !tb[RDMA_NLDEV_ATTR_CHARDEV_TYPE])
 		return -EINVAL;
 
-	nla_strlcpy(client_name, tb[RDMA_NLDEV_ATTR_CHARDEV_TYPE],
+	nla_strcpy(client_name, tb[RDMA_NLDEV_ATTR_CHARDEV_TYPE],
 		    sizeof(client_name));
 
 	if (tb[RDMA_NLDEV_ATTR_DEV_INDEX]) {
diff --git a/drivers/net/can/vxcan.c b/drivers/net/can/vxcan.c
index d6ba9426be4d..763ad76ac395 100644
--- a/drivers/net/can/vxcan.c
+++ b/drivers/net/can/vxcan.c
@@ -186,7 +186,7 @@ static int vxcan_newlink(struct net *net, struct net_device *dev,
 	}
 
 	if (ifmp && tbp[IFLA_IFNAME]) {
-		nla_strlcpy(ifname, tbp[IFLA_IFNAME], IFNAMSIZ);
+		nla_strcpy(ifname, tbp[IFLA_IFNAME], IFNAMSIZ);
 		name_assign_type = NET_NAME_USER;
 	} else {
 		snprintf(ifname, IFNAMSIZ, DRV_NAME "%%d");
@@ -223,7 +223,7 @@ static int vxcan_newlink(struct net *net, struct net_device *dev,
 
 	/* register first device */
 	if (tb[IFLA_IFNAME])
-		nla_strlcpy(dev->name, tb[IFLA_IFNAME], IFNAMSIZ);
+		nla_strcpy(dev->name, tb[IFLA_IFNAME], IFNAMSIZ);
 	else
 		snprintf(dev->name, IFNAMSIZ, DRV_NAME "%%d");
 
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 8c737668008a..c754e0a1fe40 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -1329,7 +1329,7 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
 	}
 
 	if (ifmp && tbp[IFLA_IFNAME]) {
-		nla_strlcpy(ifname, tbp[IFLA_IFNAME], IFNAMSIZ);
+		nla_strcpy(ifname, tbp[IFLA_IFNAME], IFNAMSIZ);
 		name_assign_type = NET_NAME_USER;
 	} else {
 		snprintf(ifname, IFNAMSIZ, DRV_NAME "%%d");
@@ -1379,7 +1379,7 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
 		eth_hw_addr_random(dev);
 
 	if (tb[IFLA_IFNAME])
-		nla_strlcpy(dev->name, tb[IFLA_IFNAME], IFNAMSIZ);
+		nla_strcpy(dev->name, tb[IFLA_IFNAME], IFNAMSIZ);
 	else
 		snprintf(dev->name, IFNAMSIZ, DRV_NAME "%%d");
 
diff --git a/include/linux/genl_magic_struct.h b/include/linux/genl_magic_struct.h
index eeae59d3ceb7..1ebec86d5c45 100644
--- a/include/linux/genl_magic_struct.h
+++ b/include/linux/genl_magic_struct.h
@@ -89,7 +89,7 @@ static inline int nla_put_u64_0pad(struct sk_buff *skb, int attrtype, u64 value)
 			nla_get_u64, nla_put_u64_0pad, false)
 #define __str_field(attr_nr, attr_flag, name, maxlen) \
 	__array(attr_nr, attr_flag, name, NLA_NUL_STRING, char, maxlen, \
-			nla_strlcpy, nla_put, false)
+			nla_strcpy, nla_put, false)
 #define __bin_field(attr_nr, attr_flag, name, maxlen) \
 	__array(attr_nr, attr_flag, name, NLA_BINARY, char, maxlen, \
 			nla_memcpy, nla_put, false)
diff --git a/include/net/netlink.h b/include/net/netlink.h
index 446ca182e13d..bbe9cd13c732 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -142,7 +142,7 @@
  * Attribute Misc:
  *   nla_memcpy(dest, nla, count)	copy attribute into memory
  *   nla_memcmp(nla, data, size)	compare attribute with memory area
- *   nla_strlcpy(dst, nla, size)	copy attribute to a sized string
+ *   nla_strcpy(dst, nla, size)	copy attribute to a sized string
  *   nla_strcmp(nla, str)		compare attribute with string
  *
  * Attribute Parsing:
@@ -506,7 +506,7 @@ int __nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
 		struct netlink_ext_ack *extack);
 int nla_policy_len(const struct nla_policy *, int);
 struct nlattr *nla_find(const struct nlattr *head, int len, int attrtype);
-ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize);
+ssize_t nla_strcpy(char *dst, const struct nlattr *nla, size_t dstsize);
 char *nla_strdup(const struct nlattr *nla, gfp_t flags);
 int nla_memcpy(void *dest, const struct nlattr *src, int count);
 int nla_memcmp(const struct nlattr *nla, const void *data, size_t size);
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 85f4ac779399..167897d27ac9 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -513,7 +513,7 @@ tcf_change_indev(struct net *net, struct nlattr *indev_tlv,
 	char indev[IFNAMSIZ];
 	struct net_device *dev;
 
-	if (nla_strlcpy(indev, indev_tlv, IFNAMSIZ) < 0) {
+	if (nla_strcpy(indev, indev_tlv, IFNAMSIZ) < 0) {
 		NL_SET_ERR_MSG_ATTR(extack, indev_tlv,
 				    "Interface name too long");
 		return -EINVAL;
diff --git a/kernel/taskstats.c b/kernel/taskstats.c
index a2802b6ff4bb..713f35649806 100644
--- a/kernel/taskstats.c
+++ b/kernel/taskstats.c
@@ -346,7 +346,7 @@ static int parse(struct nlattr *na, struct cpumask *mask)
 	data = kmalloc(len, GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
-	nla_strlcpy(data, na, len);
+	nla_strcpy(data, na, len);
 	ret = cpulist_parse(data, mask);
 	kfree(data);
 	return ret;
diff --git a/lib/nlattr.c b/lib/nlattr.c
index d692716bda78..846d68f64ad2 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -709,7 +709,7 @@ struct nlattr *nla_find(const struct nlattr *head, int len, int attrtype)
 EXPORT_SYMBOL(nla_find);
 
 /**
- * nla_strlcpy - Copy string attribute payload into a sized buffer
+ * nla_strcpy - Copy string attribute payload into a sized buffer
  * @dst: where to copy the string to
  * @nla: attribute to copy the string from
  * @dstsize: size of destination buffer
@@ -718,9 +718,9 @@ EXPORT_SYMBOL(nla_find);
  * including the trailing %NUL).
  *
  * Copies at most dstsize - 1 bytes into the destination buffer.
- * Unlike strlcpy the destination buffer is always padded out.
+ * Unlike strcpy the destination buffer is always padded out.
  */
-ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
+ssize_t nla_strcpy(char *dst, const struct nlattr *nla, size_t dstsize)
 {
 	size_t len;
 	ssize_t ret;
@@ -747,7 +747,7 @@ ssize_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
 
 	return ret;
 }
-EXPORT_SYMBOL(nla_strlcpy);
+EXPORT_SYMBOL(nla_strcpy);
 
 /**
  * nla_strdup - Copy string attribute payload into a newly allocated buffer
diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index 7bcfb16854cb..61d86780f409 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -563,7 +563,7 @@ static int fib_nl2rule(struct sk_buff *skb, struct nlmsghdr *nlh,
 		struct net_device *dev;
 
 		nlrule->iifindex = -1;
-		nla_strlcpy(nlrule->iifname, tb[FRA_IIFNAME], IFNAMSIZ);
+		nla_strcpy(nlrule->iifname, tb[FRA_IIFNAME], IFNAMSIZ);
 		dev = __dev_get_by_name(net, nlrule->iifname);
 		if (dev)
 			nlrule->iifindex = dev->ifindex;
@@ -573,7 +573,7 @@ static int fib_nl2rule(struct sk_buff *skb, struct nlmsghdr *nlh,
 		struct net_device *dev;
 
 		nlrule->oifindex = -1;
-		nla_strlcpy(nlrule->oifname, tb[FRA_OIFNAME], IFNAMSIZ);
+		nla_strcpy(nlrule->oifname, tb[FRA_OIFNAME], IFNAMSIZ);
 		dev = __dev_get_by_name(net, nlrule->oifname);
 		if (dev)
 			nlrule->oifindex = dev->ifindex;
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 68e0682450c6..5d70113b4f25 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1939,7 +1939,7 @@ static const struct rtnl_link_ops *linkinfo_to_kind_ops(const struct nlattr *nla
 	if (linfo[IFLA_INFO_KIND]) {
 		char kind[MODULE_NAME_LEN];
 
-		nla_strlcpy(kind, linfo[IFLA_INFO_KIND], sizeof(kind));
+		nla_strcpy(kind, linfo[IFLA_INFO_KIND], sizeof(kind));
 		ops = rtnl_link_ops_get(kind);
 	}
 
@@ -2953,9 +2953,9 @@ static struct net_device *rtnl_dev_get(struct net *net,
 	if (!ifname) {
 		ifname = buffer;
 		if (ifname_attr)
-			nla_strlcpy(ifname, ifname_attr, IFNAMSIZ);
+			nla_strcpy(ifname, ifname_attr, IFNAMSIZ);
 		else if (altifname_attr)
-			nla_strlcpy(ifname, altifname_attr, ALTIFNAMSIZ);
+			nla_strcpy(ifname, altifname_attr, ALTIFNAMSIZ);
 		else
 			return NULL;
 	}
@@ -2983,7 +2983,7 @@ static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 		goto errout;
 
 	if (tb[IFLA_IFNAME])
-		nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
+		nla_strcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
 	else
 		ifname[0] = '\0';
 
@@ -3264,7 +3264,7 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 		return err;
 
 	if (tb[IFLA_IFNAME])
-		nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
+		nla_strcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
 	else
 		ifname[0] = '\0';
 
@@ -3296,7 +3296,7 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 		memset(linkinfo, 0, sizeof(linkinfo));
 
 	if (linkinfo[IFLA_INFO_KIND]) {
-		nla_strlcpy(kind, linkinfo[IFLA_INFO_KIND], sizeof(kind));
+		nla_strcpy(kind, linkinfo[IFLA_INFO_KIND], sizeof(kind));
 		ops = rtnl_link_ops_get(kind);
 	} else {
 		kind[0] = '\0';
diff --git a/net/decnet/dn_dev.c b/net/decnet/dn_dev.c
index 15d42353f1a3..e6c83289eecc 100644
--- a/net/decnet/dn_dev.c
+++ b/net/decnet/dn_dev.c
@@ -658,7 +658,7 @@ static int dn_nl_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
 	ifa->ifa_dev = dn_db;
 
 	if (tb[IFA_LABEL])
-		nla_strlcpy(ifa->ifa_label, tb[IFA_LABEL], IFNAMSIZ);
+		nla_strcpy(ifa->ifa_label, tb[IFA_LABEL], IFNAMSIZ);
 	else
 		memcpy(ifa->ifa_label, dev->name, IFNAMSIZ);
 
diff --git a/net/ieee802154/nl-mac.c b/net/ieee802154/nl-mac.c
index 6d091e419d3e..45472ba629bb 100644
--- a/net/ieee802154/nl-mac.c
+++ b/net/ieee802154/nl-mac.c
@@ -149,7 +149,7 @@ static struct net_device *ieee802154_nl_get_dev(struct genl_info *info)
 	if (info->attrs[IEEE802154_ATTR_DEV_NAME]) {
 		char name[IFNAMSIZ + 1];
 
-		nla_strlcpy(name, info->attrs[IEEE802154_ATTR_DEV_NAME],
+		nla_strcpy(name, info->attrs[IEEE802154_ATTR_DEV_NAME],
 			    sizeof(name));
 		dev = dev_get_by_name(&init_net, name);
 	} else if (info->attrs[IEEE802154_ATTR_DEV_INDEX]) {
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 123a6d39438f..5fbb6bee6d82 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -881,7 +881,7 @@ static struct in_ifaddr *rtm_to_ifaddr(struct net *net, struct nlmsghdr *nlh,
 		ifa->ifa_broadcast = nla_get_in_addr(tb[IFA_BROADCAST]);
 
 	if (tb[IFA_LABEL])
-		nla_strlcpy(ifa->ifa_label, tb[IFA_LABEL], IFNAMSIZ);
+		nla_strcpy(ifa->ifa_label, tb[IFA_LABEL], IFNAMSIZ);
 	else
 		memcpy(ifa->ifa_label, dev->name, IFNAMSIZ);
 
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 1f75dc686b6b..8437d46a30c9 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -973,7 +973,7 @@ bool fib_metrics_match(struct fib_config *cfg, struct fib_info *fi)
 			char tmp[TCP_CA_NAME_MAX];
 			bool ecn_ca = false;
 
-			nla_strlcpy(tmp, nla, sizeof(tmp));
+			nla_strcpy(tmp, nla, sizeof(tmp));
 			val = tcp_ca_get_key_by_name(fi->fib_net, tmp, &ecn_ca);
 		} else {
 			if (nla_len(nla) != sizeof(u32))
diff --git a/net/ipv4/metrics.c b/net/ipv4/metrics.c
index 3205d5f7c8c9..99844fd6975d 100644
--- a/net/ipv4/metrics.c
+++ b/net/ipv4/metrics.c
@@ -31,7 +31,7 @@ static int ip_metrics_convert(struct net *net, struct nlattr *fc_mx,
 		if (type == RTAX_CC_ALGO) {
 			char tmp[TCP_CA_NAME_MAX];
 
-			nla_strlcpy(tmp, nla, sizeof(tmp));
+			nla_strcpy(tmp, nla, sizeof(tmp));
 			val = tcp_ca_get_key_by_name(net, tmp, &ecn_ca);
 			if (val == TCP_CA_UNSPEC) {
 				NL_SET_ERR_MSG(extack, "Unknown tcp congestion algorithm");
diff --git a/net/netfilter/ipset/ip_set_hash_netiface.c b/net/netfilter/ipset/ip_set_hash_netiface.c
index be5e95a0d876..5e114985ad9d 100644
--- a/net/netfilter/ipset/ip_set_hash_netiface.c
+++ b/net/netfilter/ipset/ip_set_hash_netiface.c
@@ -225,7 +225,7 @@ hash_netiface4_uadt(struct ip_set *set, struct nlattr *tb[],
 		if (e.cidr > HOST_MASK)
 			return -IPSET_ERR_INVALID_CIDR;
 	}
-	nla_strlcpy(e.iface, tb[IPSET_ATTR_IFACE], IFNAMSIZ);
+	nla_strcpy(e.iface, tb[IPSET_ATTR_IFACE], IFNAMSIZ);
 
 	if (tb[IPSET_ATTR_CADT_FLAGS]) {
 		u32 cadt_flags = ip_set_get_h32(tb[IPSET_ATTR_CADT_FLAGS]);
@@ -442,7 +442,7 @@ hash_netiface6_uadt(struct ip_set *set, struct nlattr *tb[],
 
 	ip6_netmask(&e.ip, e.cidr);
 
-	nla_strlcpy(e.iface, tb[IPSET_ATTR_IFACE], IFNAMSIZ);
+	nla_strcpy(e.iface, tb[IPSET_ATTR_IFACE], IFNAMSIZ);
 
 	if (tb[IPSET_ATTR_CADT_FLAGS]) {
 		u32 cadt_flags = ip_set_get_h32(tb[IPSET_ATTR_CADT_FLAGS]);
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 9957e0ed8658..38eb14ffbc4f 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1281,7 +1281,7 @@ static struct nft_chain *nft_chain_lookup(struct net *net,
 	if (nla == NULL)
 		return ERR_PTR(-EINVAL);
 
-	nla_strlcpy(search, nla, sizeof(search));
+	nla_strcpy(search, nla, sizeof(search));
 
 	WARN_ON(!rcu_read_lock_held() &&
 		!lockdep_commit_lock_is_held(net));
@@ -1721,7 +1721,7 @@ static struct nft_hook *nft_netdev_hook_alloc(struct net *net,
 		goto err_hook_alloc;
 	}
 
-	nla_strlcpy(ifname, attr, IFNAMSIZ);
+	nla_strcpy(ifname, attr, IFNAMSIZ);
 	dev = __dev_get_by_name(net, ifname);
 	if (!dev) {
 		err = -ENOENT;
@@ -5734,7 +5734,7 @@ struct nft_object *nft_obj_lookup(const struct net *net,
 	struct rhlist_head *tmp, *list;
 	struct nft_object *obj;
 
-	nla_strlcpy(search, nla, sizeof(search));
+	nla_strcpy(search, nla, sizeof(search));
 	k.name = search;
 
 	WARN_ON_ONCE(!rcu_read_lock_held() &&
diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c
index 5bfec829c12f..d657749724da 100644
--- a/net/netfilter/nfnetlink_acct.c
+++ b/net/netfilter/nfnetlink_acct.c
@@ -112,7 +112,7 @@ static int nfnl_acct_new(struct net *net, struct sock *nfnl,
 		nfacct->flags = flags;
 	}
 
-	nla_strlcpy(nfacct->name, tb[NFACCT_NAME], NFACCT_NAME_MAX);
+	nla_strcpy(nfacct->name, tb[NFACCT_NAME], NFACCT_NAME_MAX);
 
 	if (tb[NFACCT_BYTES]) {
 		atomic64_set(&nfacct->bytes,
diff --git a/net/netfilter/nfnetlink_cthelper.c b/net/netfilter/nfnetlink_cthelper.c
index 5b0d0a77379c..1900895fa3d5 100644
--- a/net/netfilter/nfnetlink_cthelper.c
+++ b/net/netfilter/nfnetlink_cthelper.c
@@ -146,7 +146,7 @@ nfnl_cthelper_expect_policy(struct nf_conntrack_expect_policy *expect_policy,
 	    !tb[NFCTH_POLICY_EXPECT_TIMEOUT])
 		return -EINVAL;
 
-	nla_strlcpy(expect_policy->name,
+	nla_strcpy(expect_policy->name,
 		    tb[NFCTH_POLICY_NAME], NF_CT_HELPER_NAME_LEN);
 	expect_policy->max_expected =
 		ntohl(nla_get_be32(tb[NFCTH_POLICY_EXPECT_MAX]));
@@ -233,7 +233,7 @@ nfnl_cthelper_create(const struct nlattr * const tb[],
 	if (ret < 0)
 		goto err1;
 
-	nla_strlcpy(helper->name,
+	nla_strcpy(helper->name,
 		    tb[NFCTH_NAME], NF_CT_HELPER_NAME_LEN);
 	size = ntohl(nla_get_be32(tb[NFCTH_PRIV_DATA_LEN]));
 	if (size > sizeof_field(struct nf_conn_help, data)) {
diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
index 322bd674963e..bb02ea2ef1bf 100644
--- a/net/netfilter/nft_ct.c
+++ b/net/netfilter/nft_ct.c
@@ -990,7 +990,7 @@ static int nft_ct_helper_obj_init(const struct nft_ctx *ctx,
 	if (!priv->l4proto)
 		return -ENOENT;
 
-	nla_strlcpy(name, tb[NFTA_CT_HELPER_NAME], sizeof(name));
+	nla_strcpy(name, tb[NFTA_CT_HELPER_NAME], sizeof(name));
 
 	if (tb[NFTA_CT_HELPER_L3PROTO])
 		family = ntohs(nla_get_be16(tb[NFTA_CT_HELPER_L3PROTO]));
diff --git a/net/netfilter/nft_log.c b/net/netfilter/nft_log.c
index 57899454a530..b55607b24942 100644
--- a/net/netfilter/nft_log.c
+++ b/net/netfilter/nft_log.c
@@ -152,7 +152,7 @@ static int nft_log_init(const struct nft_ctx *ctx,
 		priv->prefix = kmalloc(nla_len(nla) + 1, GFP_KERNEL);
 		if (priv->prefix == NULL)
 			return -ENOMEM;
-		nla_strlcpy(priv->prefix, nla, nla_len(nla) + 1);
+		nla_strcpy(priv->prefix, nla, nla_len(nla) + 1);
 	} else {
 		priv->prefix = (char *)nft_log_null_prefix;
 	}
diff --git a/net/netlabel/netlabel_mgmt.c b/net/netlabel/netlabel_mgmt.c
index eb1d66d20afb..3dd925c39ef4 100644
--- a/net/netlabel/netlabel_mgmt.c
+++ b/net/netlabel/netlabel_mgmt.c
@@ -95,7 +95,7 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
 			ret_val = -ENOMEM;
 			goto add_free_entry;
 		}
-		nla_strlcpy(entry->domain,
+		nla_strcpy(entry->domain,
 			    info->attrs[NLBL_MGMT_A_DOMAIN], tmp_size);
 	}
 
diff --git a/net/nfc/netlink.c b/net/nfc/netlink.c
index e894254c17d4..ee4a53d15ae3 100644
--- a/net/nfc/netlink.c
+++ b/net/nfc/netlink.c
@@ -1226,7 +1226,7 @@ static int nfc_genl_fw_download(struct sk_buff *skb, struct genl_info *info)
 	if (!dev)
 		return -ENODEV;
 
-	nla_strlcpy(firmware_name, info->attrs[NFC_ATTR_FIRMWARE_NAME],
+	nla_strcpy(firmware_name, info->attrs[NFC_ATTR_FIRMWARE_NAME],
 		    sizeof(firmware_name));
 
 	rc = nfc_fw_download(dev, firmware_name);
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 541574520c52..9d15fec41a83 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -935,7 +935,7 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
 			NL_SET_ERR_MSG(extack, "TC action kind must be specified");
 			goto err_out;
 		}
-		if (nla_strlcpy(act_name, kind, IFNAMSIZ) < 0) {
+		if (nla_strcpy(act_name, kind, IFNAMSIZ) < 0) {
 			NL_SET_ERR_MSG(extack, "TC action name too long");
 			goto err_out;
 		}
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
index 8dc3bec0d325..9ab92a781104 100644
--- a/net/sched/act_ipt.c
+++ b/net/sched/act_ipt.c
@@ -166,7 +166,7 @@ static int __tcf_ipt_init(struct net *net, unsigned int id, struct nlattr *nla,
 	if (unlikely(!tname))
 		goto err1;
 	if (tb[TCA_IPT_TABLE] == NULL ||
-	    nla_strlcpy(tname, tb[TCA_IPT_TABLE], IFNAMSIZ) >= IFNAMSIZ)
+	    nla_strcpy(tname, tb[TCA_IPT_TABLE], IFNAMSIZ) >= IFNAMSIZ)
 		strcpy(tname, "mangle");
 
 	t = kmemdup(td, td->u.target_size, GFP_KERNEL);
diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
index a4f3d0f0daa9..6509dae17357 100644
--- a/net/sched/act_simple.c
+++ b/net/sched/act_simple.c
@@ -52,7 +52,7 @@ static int alloc_defdata(struct tcf_defact *d, const struct nlattr *defdata)
 	d->tcfd_defdata = kzalloc(SIMP_MAX_DATA, GFP_KERNEL);
 	if (unlikely(!d->tcfd_defdata))
 		return -ENOMEM;
-	nla_strlcpy(d->tcfd_defdata, defdata, SIMP_MAX_DATA);
+	nla_strcpy(d->tcfd_defdata, defdata, SIMP_MAX_DATA);
 	return 0;
 }
 
@@ -71,7 +71,7 @@ static int reset_policy(struct tc_action *a, const struct nlattr *defdata,
 	spin_lock_bh(&d->tcf_lock);
 	goto_ch = tcf_action_set_ctrlact(a, p->action, goto_ch);
 	memset(d->tcfd_defdata, 0, SIMP_MAX_DATA);
-	nla_strlcpy(d->tcfd_defdata, defdata, SIMP_MAX_DATA);
+	nla_strcpy(d->tcfd_defdata, defdata, SIMP_MAX_DATA);
 	spin_unlock_bh(&d->tcf_lock);
 	if (goto_ch)
 		tcf_chain_put_by_act(goto_ch);
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index f0bf64393cbf..f990b86247bf 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -223,7 +223,7 @@ static inline u32 tcf_auto_prio(struct tcf_proto *tp)
 static bool tcf_proto_check_kind(struct nlattr *kind, char *name)
 {
 	if (kind)
-		return nla_strlcpy(name, kind, IFNAMSIZ) > 0;
+		return nla_strcpy(name, kind, IFNAMSIZ) > 0;
 	memset(name, 0, IFNAMSIZ);
 	return false;
 }
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index f9b053b30a7b..92717fd74dfe 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1170,7 +1170,7 @@ static struct Qdisc *qdisc_create(struct net_device *dev,
 #ifdef CONFIG_MODULES
 	if (ops == NULL && kind != NULL) {
 		char name[IFNAMSIZ];
-		if (nla_strlcpy(name, kind, IFNAMSIZ) > 0) {
+		if (nla_strcpy(name, kind, IFNAMSIZ) > 0) {
 			/* We dropped the RTNL semaphore in order to
 			 * perform the module load.  So, even if we
 			 * succeeded in loading the module we have to
diff --git a/net/tipc/netlink_compat.c b/net/tipc/netlink_compat.c
index 1c7aa51cc2a3..c973a61fe440 100644
--- a/net/tipc/netlink_compat.c
+++ b/net/tipc/netlink_compat.c
@@ -695,7 +695,7 @@ static int tipc_nl_compat_link_dump(struct tipc_nl_compat_msg *msg,
 
 	link_info.dest = nla_get_flag(link[TIPC_NLA_LINK_DEST]);
 	link_info.up = htonl(nla_get_flag(link[TIPC_NLA_LINK_UP]));
-	nla_strlcpy(link_info.str, link[TIPC_NLA_LINK_NAME],
+	nla_strcpy(link_info.str, link[TIPC_NLA_LINK_NAME],
 		    TIPC_MAX_LINK_NAME);
 
 	return tipc_add_tlv(msg->rep, TIPC_TLV_LINK_INFO,
-- 
2.20.1

Re: [RFC][PATCH v2 0/3] Fix inefficiences and rename nla_strlcpy

[Jakub Kicinski]

On Mon, 19 Oct 2020 17:23:28 +0200 laniel_francis@privacyrequired.com
wrote:
> To sum up, the first patch fixes an inefficiency where some bytes in dst were
> written twice, one with 0 the other with src content.
> The second one modifies nla_strlcpy to return the same value as strscpy,
> i.e. number of bytes written or -E2BIG if src was truncated.
> The third rename nla_strlcpy to nla_strcpy.
> 
> Unfortunately, I did not find how to create struct nlattr objects so I tested
> my modifications on simple char*.
> This is why I tag this patch set as RFC.
> 
> If you see any way to improve the code or have any remark, feel free to comment.

You follow semantics of strscpy, yet rename to strcpy. Wouldn't it be
more intuitive for developers to rename to nla_strscpy?

Please make sure that you CC netdev on the next iteration.

Re: [RFC][PATCH v2 0/3] Fix inefficiences and rename nla_strlcpy

[Kees Cook]

On Mon, Oct 19, 2020 at 09:45:15AM -0700, Jakub Kicinski wrote:
> On Mon, 19 Oct 2020 17:23:28 +0200 laniel_francis@privacyrequired.com
> wrote:
> > To sum up, the first patch fixes an inefficiency where some bytes in dst were
> > written twice, one with 0 the other with src content.
> > The second one modifies nla_strlcpy to return the same value as strscpy,
> > i.e. number of bytes written or -E2BIG if src was truncated.
> > The third rename nla_strlcpy to nla_strcpy.
> > 
> > Unfortunately, I did not find how to create struct nlattr objects so I tested
> > my modifications on simple char*.
> > This is why I tag this patch set as RFC.
> > 
> > If you see any way to improve the code or have any remark, feel free to comment.
> 
> You follow semantics of strscpy, yet rename to strcpy. Wouldn't it be
> more intuitive for developers to rename to nla_strscpy?

It's closer to strscpy_pad() but that seems a long name. What's
preferred from the NLA perspective?

-- 
Kees Cook

Re: [RFC][PATCH v2 0/3] Fix inefficiences and rename nla_strlcpy

[Jakub Kicinski]

On Mon, 19 Oct 2020 15:58:36 -0700 Kees Cook wrote:
> On Mon, Oct 19, 2020 at 09:45:15AM -0700, Jakub Kicinski wrote:
> > On Mon, 19 Oct 2020 17:23:28 +0200 laniel_francis@privacyrequired.com
> > wrote:  
> > > To sum up, the first patch fixes an inefficiency where some bytes in dst were
> > > written twice, one with 0 the other with src content.
> > > The second one modifies nla_strlcpy to return the same value as strscpy,
> > > i.e. number of bytes written or -E2BIG if src was truncated.
> > > The third rename nla_strlcpy to nla_strcpy.
> > > 
> > > Unfortunately, I did not find how to create struct nlattr objects so I tested
> > > my modifications on simple char*.
> > > This is why I tag this patch set as RFC.
> > > 
> > > If you see any way to improve the code or have any remark, feel free to comment.  
> > 
> > You follow semantics of strscpy, yet rename to strcpy. Wouldn't it be
> > more intuitive for developers to rename to nla_strscpy?  
> 
> It's closer to strscpy_pad() but that seems a long name. What's
> preferred from the NLA perspective?

I think the pad part is pretty much implied in the netlink world.
All this stuff goes to user space, so we can't have uninit memory. 
We may get more informed opinions once this hits netdev@.

Re: [RFC][PATCH v2 0/3] Fix inefficiences and rename nla_strlcpy

[Francis Laniel]

Le mardi 20 octobre 2020, 01:34:12 CEST Jakub Kicinski a écrit :
> On Mon, 19 Oct 2020 15:58:36 -0700 Kees Cook wrote:
> > On Mon, Oct 19, 2020 at 09:45:15AM -0700, Jakub Kicinski wrote:
> > > On Mon, 19 Oct 2020 17:23:28 +0200 laniel_francis@privacyrequired.com
> > > 
> > > wrote:
> > > > To sum up, the first patch fixes an inefficiency where some bytes in
> > > > dst were written twice, one with 0 the other with src content.
> > > > The second one modifies nla_strlcpy to return the same value as
> > > > strscpy,
> > > > i.e. number of bytes written or -E2BIG if src was truncated.
> > > > The third rename nla_strlcpy to nla_strcpy.
> > > > 
> > > > Unfortunately, I did not find how to create struct nlattr objects so I
> > > > tested my modifications on simple char*.
> > > > This is why I tag this patch set as RFC.
> > > > 
> > > > If you see any way to improve the code or have any remark, feel free
> > > > to comment.> > 
> > > You follow semantics of strscpy, yet rename to strcpy. Wouldn't it be
> > > more intuitive for developers to rename to nla_strscpy?
> > 
> > It's closer to strscpy_pad() but that seems a long name. What's
> > preferred from the NLA perspective?
> 
> I think the pad part is pretty much implied in the netlink world.
> All this stuff goes to user space, so we can't have uninit memory.
> We may get more informed opinions once this hits netdev@.

I will rename it nla_strscpy for the next version and we will discuss the name 
with the netdev list.

Re: [RFC][PATCH v2 0/3] Fix inefficiences and rename nla_strlcpy

[Kees Cook]

[process nit: please don't use "--in-reply-to=" for git send-email, as
it ends up collecting all the versioned threads together, which makes
them harder to review. Each version should be a separate thread.]

-- 
Kees Cook

Re: [RFC][PATCH v2 0/3] Fix inefficiences and rename nla_strlcpy

[Francis Laniel]

Le mardi 20 octobre 2020, 01:03:18 CEST Kees Cook a écrit :
> [process nit: please don't use "--in-reply-to=" for git send-email, as
> it ends up collecting all the versioned threads together, which makes
> them harder to review. Each version should be a separate thread.]

Thank you for the advice!
Will be done for next patch set!