Skip to content

Commit c0a645a

Browse files
committed
Merge branch 'fix-inefficiences-and-rename-nla_strlcpy'
Francis Laniel says: ==================== Fix inefficiences and rename nla_strlcpy This patch set answers to first three issues listed in: KSPP/linux#110 To sum up, the patch contributions are the following: 1. the first patch fixes an inefficiency where some bytes in dst were written twice, one with 0 the other with src content. 2. 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. It also modifies code that calls nla_strlcpy and checks for its return value. 3. The third renames nla_strlcpy to nla_strscpy. Unfortunately, I did not find how to create struct nlattr objects so I tested my modifications on simple char* and with GDB using tc to get to tcf_proto_check_kind. ==================== Link: https://lore.kernel.org/r/20201115170806.3578-1-laniel_francis@privacyrequired.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2 parents 41294e6 + 872f690 commit c0a645a

File tree

29 files changed

+73
-61
lines changed

29 files changed

+73
-61
lines changed

drivers/infiniband/core/nldev.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -932,7 +932,7 @@ static int nldev_set_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
932932
if (tb[RDMA_NLDEV_ATTR_DEV_NAME]) {
933933
char name[IB_DEVICE_NAME_MAX] = {};
934934

935-
nla_strlcpy(name, tb[RDMA_NLDEV_ATTR_DEV_NAME],
935+
nla_strscpy(name, tb[RDMA_NLDEV_ATTR_DEV_NAME],
936936
IB_DEVICE_NAME_MAX);
937937
if (strlen(name) == 0) {
938938
err = -EINVAL;
@@ -1529,13 +1529,13 @@ static int nldev_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
15291529
!tb[RDMA_NLDEV_ATTR_LINK_TYPE] || !tb[RDMA_NLDEV_ATTR_NDEV_NAME])
15301530
return -EINVAL;
15311531

1532-
nla_strlcpy(ibdev_name, tb[RDMA_NLDEV_ATTR_DEV_NAME],
1532+
nla_strscpy(ibdev_name, tb[RDMA_NLDEV_ATTR_DEV_NAME],
15331533
sizeof(ibdev_name));
15341534
if (strchr(ibdev_name, '%') || strlen(ibdev_name) == 0)
15351535
return -EINVAL;
15361536

1537-
nla_strlcpy(type, tb[RDMA_NLDEV_ATTR_LINK_TYPE], sizeof(type));
1538-
nla_strlcpy(ndev_name, tb[RDMA_NLDEV_ATTR_NDEV_NAME],
1537+
nla_strscpy(type, tb[RDMA_NLDEV_ATTR_LINK_TYPE], sizeof(type));
1538+
nla_strscpy(ndev_name, tb[RDMA_NLDEV_ATTR_NDEV_NAME],
15391539
sizeof(ndev_name));
15401540

15411541
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,
16021602
if (err || !tb[RDMA_NLDEV_ATTR_CHARDEV_TYPE])
16031603
return -EINVAL;
16041604

1605-
nla_strlcpy(client_name, tb[RDMA_NLDEV_ATTR_CHARDEV_TYPE],
1605+
nla_strscpy(client_name, tb[RDMA_NLDEV_ATTR_CHARDEV_TYPE],
16061606
sizeof(client_name));
16071607

16081608
if (tb[RDMA_NLDEV_ATTR_DEV_INDEX]) {

drivers/net/can/vxcan.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ static int vxcan_newlink(struct net *net, struct net_device *dev,
186186
}
187187

188188
if (ifmp && tbp[IFLA_IFNAME]) {
189-
nla_strlcpy(ifname, tbp[IFLA_IFNAME], IFNAMSIZ);
189+
nla_strscpy(ifname, tbp[IFLA_IFNAME], IFNAMSIZ);
190190
name_assign_type = NET_NAME_USER;
191191
} else {
192192
snprintf(ifname, IFNAMSIZ, DRV_NAME "%%d");
@@ -223,7 +223,7 @@ static int vxcan_newlink(struct net *net, struct net_device *dev,
223223

224224
/* register first device */
225225
if (tb[IFLA_IFNAME])
226-
nla_strlcpy(dev->name, tb[IFLA_IFNAME], IFNAMSIZ);
226+
nla_strscpy(dev->name, tb[IFLA_IFNAME], IFNAMSIZ);
227227
else
228228
snprintf(dev->name, IFNAMSIZ, DRV_NAME "%%d");
229229

drivers/net/veth.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1329,7 +1329,7 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
13291329
}
13301330

13311331
if (ifmp && tbp[IFLA_IFNAME]) {
1332-
nla_strlcpy(ifname, tbp[IFLA_IFNAME], IFNAMSIZ);
1332+
nla_strscpy(ifname, tbp[IFLA_IFNAME], IFNAMSIZ);
13331333
name_assign_type = NET_NAME_USER;
13341334
} else {
13351335
snprintf(ifname, IFNAMSIZ, DRV_NAME "%%d");
@@ -1379,7 +1379,7 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
13791379
eth_hw_addr_random(dev);
13801380

13811381
if (tb[IFLA_IFNAME])
1382-
nla_strlcpy(dev->name, tb[IFLA_IFNAME], IFNAMSIZ);
1382+
nla_strscpy(dev->name, tb[IFLA_IFNAME], IFNAMSIZ);
13831383
else
13841384
snprintf(dev->name, IFNAMSIZ, DRV_NAME "%%d");
13851385

include/linux/genl_magic_struct.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ static inline int nla_put_u64_0pad(struct sk_buff *skb, int attrtype, u64 value)
8989
nla_get_u64, nla_put_u64_0pad, false)
9090
#define __str_field(attr_nr, attr_flag, name, maxlen) \
9191
__array(attr_nr, attr_flag, name, NLA_NUL_STRING, char, maxlen, \
92-
nla_strlcpy, nla_put, false)
92+
nla_strscpy, nla_put, false)
9393
#define __bin_field(attr_nr, attr_flag, name, maxlen) \
9494
__array(attr_nr, attr_flag, name, NLA_BINARY, char, maxlen, \
9595
nla_memcpy, nla_put, false)

include/net/netlink.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@
142142
* Attribute Misc:
143143
* nla_memcpy(dest, nla, count) copy attribute into memory
144144
* nla_memcmp(nla, data, size) compare attribute with memory area
145-
* nla_strlcpy(dst, nla, size) copy attribute to a sized string
145+
* nla_strscpy(dst, nla, size) copy attribute to a sized string
146146
* nla_strcmp(nla, str) compare attribute with string
147147
*
148148
* Attribute Parsing:
@@ -506,7 +506,7 @@ int __nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
506506
struct netlink_ext_ack *extack);
507507
int nla_policy_len(const struct nla_policy *, int);
508508
struct nlattr *nla_find(const struct nlattr *head, int len, int attrtype);
509-
size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize);
509+
ssize_t nla_strscpy(char *dst, const struct nlattr *nla, size_t dstsize);
510510
char *nla_strdup(const struct nlattr *nla, gfp_t flags);
511511
int nla_memcpy(void *dest, const struct nlattr *src, int count);
512512
int nla_memcmp(const struct nlattr *nla, const void *data, size_t size);

include/net/pkt_cls.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,7 @@ tcf_change_indev(struct net *net, struct nlattr *indev_tlv,
512512
char indev[IFNAMSIZ];
513513
struct net_device *dev;
514514

515-
if (nla_strlcpy(indev, indev_tlv, IFNAMSIZ) >= IFNAMSIZ) {
515+
if (nla_strscpy(indev, indev_tlv, IFNAMSIZ) < 0) {
516516
NL_SET_ERR_MSG_ATTR(extack, indev_tlv,
517517
"Interface name too long");
518518
return -EINVAL;

kernel/taskstats.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ static int parse(struct nlattr *na, struct cpumask *mask)
346346
data = kmalloc(len, GFP_KERNEL);
347347
if (!data)
348348
return -ENOMEM;
349-
nla_strlcpy(data, na, len);
349+
nla_strscpy(data, na, len);
350350
ret = cpulist_parse(data, mask);
351351
kfree(data);
352352
return ret;

lib/nlattr.c

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -709,35 +709,47 @@ struct nlattr *nla_find(const struct nlattr *head, int len, int attrtype)
709709
EXPORT_SYMBOL(nla_find);
710710

711711
/**
712-
* nla_strlcpy - Copy string attribute payload into a sized buffer
713-
* @dst: where to copy the string to
714-
* @nla: attribute to copy the string from
715-
* @dstsize: size of destination buffer
712+
* nla_strscpy - Copy string attribute payload into a sized buffer
713+
* @dst: Where to copy the string to.
714+
* @nla: Attribute to copy the string from.
715+
* @dstsize: Size of destination buffer.
716716
*
717717
* Copies at most dstsize - 1 bytes into the destination buffer.
718-
* The result is always a valid NUL-terminated string. Unlike
719-
* strlcpy the destination buffer is always padded out.
718+
* Unlike strlcpy the destination buffer is always padded out.
720719
*
721-
* Returns the length of the source buffer.
720+
* Return:
721+
* * srclen - Returns @nla length (not including the trailing %NUL).
722+
* * -E2BIG - If @dstsize is 0 or greater than U16_MAX or @nla length greater
723+
* than @dstsize.
722724
*/
723-
size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
725+
ssize_t nla_strscpy(char *dst, const struct nlattr *nla, size_t dstsize)
724726
{
725727
size_t srclen = nla_len(nla);
726728
char *src = nla_data(nla);
729+
ssize_t ret;
730+
size_t len;
731+
732+
if (dstsize == 0 || WARN_ON_ONCE(dstsize > U16_MAX))
733+
return -E2BIG;
727734

728735
if (srclen > 0 && src[srclen - 1] == '\0')
729736
srclen--;
730737

731-
if (dstsize > 0) {
732-
size_t len = (srclen >= dstsize) ? dstsize - 1 : srclen;
733-
734-
memset(dst, 0, dstsize);
735-
memcpy(dst, src, len);
738+
if (srclen >= dstsize) {
739+
len = dstsize - 1;
740+
ret = -E2BIG;
741+
} else {
742+
len = srclen;
743+
ret = len;
736744
}
737745

738-
return srclen;
746+
memcpy(dst, src, len);
747+
/* Zero pad end of dst. */
748+
memset(dst + len, 0, dstsize - len);
749+
750+
return ret;
739751
}
740-
EXPORT_SYMBOL(nla_strlcpy);
752+
EXPORT_SYMBOL(nla_strscpy);
741753

742754
/**
743755
* nla_strdup - Copy string attribute payload into a newly allocated buffer

net/core/fib_rules.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,7 @@ static int fib_nl2rule(struct sk_buff *skb, struct nlmsghdr *nlh,
563563
struct net_device *dev;
564564

565565
nlrule->iifindex = -1;
566-
nla_strlcpy(nlrule->iifname, tb[FRA_IIFNAME], IFNAMSIZ);
566+
nla_strscpy(nlrule->iifname, tb[FRA_IIFNAME], IFNAMSIZ);
567567
dev = __dev_get_by_name(net, nlrule->iifname);
568568
if (dev)
569569
nlrule->iifindex = dev->ifindex;
@@ -573,7 +573,7 @@ static int fib_nl2rule(struct sk_buff *skb, struct nlmsghdr *nlh,
573573
struct net_device *dev;
574574

575575
nlrule->oifindex = -1;
576-
nla_strlcpy(nlrule->oifname, tb[FRA_OIFNAME], IFNAMSIZ);
576+
nla_strscpy(nlrule->oifname, tb[FRA_OIFNAME], IFNAMSIZ);
577577
dev = __dev_get_by_name(net, nlrule->oifname);
578578
if (dev)
579579
nlrule->oifindex = dev->ifindex;

net/core/rtnetlink.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1939,7 +1939,7 @@ static const struct rtnl_link_ops *linkinfo_to_kind_ops(const struct nlattr *nla
19391939
if (linfo[IFLA_INFO_KIND]) {
19401940
char kind[MODULE_NAME_LEN];
19411941

1942-
nla_strlcpy(kind, linfo[IFLA_INFO_KIND], sizeof(kind));
1942+
nla_strscpy(kind, linfo[IFLA_INFO_KIND], sizeof(kind));
19431943
ops = rtnl_link_ops_get(kind);
19441944
}
19451945

@@ -2953,9 +2953,9 @@ static struct net_device *rtnl_dev_get(struct net *net,
29532953
if (!ifname) {
29542954
ifname = buffer;
29552955
if (ifname_attr)
2956-
nla_strlcpy(ifname, ifname_attr, IFNAMSIZ);
2956+
nla_strscpy(ifname, ifname_attr, IFNAMSIZ);
29572957
else if (altifname_attr)
2958-
nla_strlcpy(ifname, altifname_attr, ALTIFNAMSIZ);
2958+
nla_strscpy(ifname, altifname_attr, ALTIFNAMSIZ);
29592959
else
29602960
return NULL;
29612961
}
@@ -2983,7 +2983,7 @@ static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh,
29832983
goto errout;
29842984

29852985
if (tb[IFLA_IFNAME])
2986-
nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
2986+
nla_strscpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
29872987
else
29882988
ifname[0] = '\0';
29892989

@@ -3264,7 +3264,7 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
32643264
return err;
32653265

32663266
if (tb[IFLA_IFNAME])
3267-
nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
3267+
nla_strscpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
32683268
else
32693269
ifname[0] = '\0';
32703270

@@ -3296,7 +3296,7 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
32963296
memset(linkinfo, 0, sizeof(linkinfo));
32973297

32983298
if (linkinfo[IFLA_INFO_KIND]) {
3299-
nla_strlcpy(kind, linkinfo[IFLA_INFO_KIND], sizeof(kind));
3299+
nla_strscpy(kind, linkinfo[IFLA_INFO_KIND], sizeof(kind));
33003300
ops = rtnl_link_ops_get(kind);
33013301
} else {
33023302
kind[0] = '\0';

0 commit comments

Comments
 (0)