For all string attributes for which we don't currently accept the
element as input, we only use it as output, set the string length to
RDMA_NLDEV_ATTR_EMPTY_STRING which is defined as 1. That way we will
only accept a null string for that element. This will prevent someone
from writing a new input routine that uses the element without also
updating the policy to have a valid value.
Also while there, make sure the existing entries that are valid have the
correct policy, if not, correct the policy. Remove unnecessary checks
for nla_strlcpy() overflow once the policy has been set correctly.
Signed-off-by: Doug Ledford <dledford@xxxxxxxxxx>
---
drivers/infiniband/core/nldev.c | 37 ++++++++++++++++++--------------
include/rdma/rdma_netlink.h | 5 +++++
include/uapi/rdma/rdma_netlink.h | 4 ----
3 files changed, 26 insertions(+), 20 deletions(-)
v0->v1: Remove all whitespace change noise from this patch, this patch
is now all functional changes
v1->v2: Instead of ignoring string overruns, return -EINVAL on string
buffer overruns (JGunthorpe requested behavior)
diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 6006d23d0410..68eb7e05104f 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -49,29 +49,29 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
[RDMA_NLDEV_ATTR_CHARDEV] = { .type = NLA_U64 },
[RDMA_NLDEV_ATTR_CHARDEV_ABI] = { .type = NLA_U64 },
[RDMA_NLDEV_ATTR_CHARDEV_NAME] = { .type = NLA_NUL_STRING,
- .len = RDMA_NLDEV_ATTR_ENTRY_STRLEN },
+ .len = RDMA_NLDEV_ATTR_EMPTY_STRING },
[RDMA_NLDEV_ATTR_CHARDEV_TYPE] = { .type = NLA_NUL_STRING,
- .len = 128 },
+ .len = IB_DEVICE_NAME_MAX },
[RDMA_NLDEV_ATTR_DEV_INDEX] = { .type = NLA_U32 },
[RDMA_NLDEV_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING,
- .len = IB_DEVICE_NAME_MAX - 1},
+ .len = IB_DEVICE_NAME_MAX },
[RDMA_NLDEV_ATTR_DEV_NODE_TYPE] = { .type = NLA_U8 },
[RDMA_NLDEV_ATTR_DEV_PROTOCOL] = { .type = NLA_NUL_STRING,
- .len = RDMA_NLDEV_ATTR_ENTRY_STRLEN },
+ .len = RDMA_NLDEV_ATTR_EMPTY_STRING },
[RDMA_NLDEV_ATTR_DRIVER] = { .type = NLA_NESTED },
[RDMA_NLDEV_ATTR_DRIVER_ENTRY] = { .type = NLA_NESTED },
[RDMA_NLDEV_ATTR_DRIVER_PRINT_TYPE] = { .type = NLA_U8 },
[RDMA_NLDEV_ATTR_DRIVER_STRING] = { .type = NLA_NUL_STRING,
- .len = RDMA_NLDEV_ATTR_ENTRY_STRLEN },
+ .len = RDMA_NLDEV_ATTR_EMPTY_STRING },
[RDMA_NLDEV_ATTR_DRIVER_S32] = { .type = NLA_S32 },
[RDMA_NLDEV_ATTR_DRIVER_S64] = { .type = NLA_S64 },
[RDMA_NLDEV_ATTR_DRIVER_U32] = { .type = NLA_U32 },
[RDMA_NLDEV_ATTR_DRIVER_U64] = { .type = NLA_U64 },
[RDMA_NLDEV_ATTR_FW_VERSION] = { .type = NLA_NUL_STRING,
- .len = IB_FW_VERSION_NAME_MAX - 1},
+ .len = RDMA_NLDEV_ATTR_EMPTY_STRING },
[RDMA_NLDEV_ATTR_LID] = { .type = NLA_U32 },
[RDMA_NLDEV_ATTR_LINK_TYPE] = { .type = NLA_NUL_STRING,
- .len = RDMA_NLDEV_ATTR_ENTRY_STRLEN },
+ .len = IFNAMSIZ },
[RDMA_NLDEV_ATTR_LMC] = { .type = NLA_U8 },
[RDMA_NLDEV_ATTR_NDEV_INDEX] = { .type = NLA_U32 },
[RDMA_NLDEV_ATTR_NDEV_NAME] = { .type = NLA_NUL_STRING,
@@ -92,7 +92,7 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
.len = sizeof(struct __kernel_sockaddr_storage) },
[RDMA_NLDEV_ATTR_RES_IOVA] = { .type = NLA_U64 },
[RDMA_NLDEV_ATTR_RES_KERN_NAME] = { .type = NLA_NUL_STRING,
- .len = TASK_COMM_LEN },
+ .len = RDMA_NLDEV_ATTR_EMPTY_STRING },
[RDMA_NLDEV_ATTR_RES_LKEY] = { .type = NLA_U32 },
[RDMA_NLDEV_ATTR_RES_LOCAL_DMA_LKEY] = { .type = NLA_U32 },
[RDMA_NLDEV_ATTR_RES_LQPN] = { .type = NLA_U32 },
@@ -120,7 +120,7 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
[RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY] = { .type = NLA_NESTED },
[RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_CURR]= { .type = NLA_U64 },
[RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_NAME]= { .type = NLA_NUL_STRING,
- .len = 16 },
+ .len = RDMA_NLDEV_ATTR_EMPTY_STRING },
[RDMA_NLDEV_ATTR_RES_TYPE] = { .type = NLA_U8 },
[RDMA_NLDEV_ATTR_RES_UNSAFE_GLOBAL_RKEY]= { .type = NLA_U32 },
[RDMA_NLDEV_ATTR_RES_USECNT] = { .type = NLA_U64 },
@@ -700,8 +700,9 @@ 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],
- IB_DEVICE_NAME_MAX);
+ if (nla_strlcpy(name, tb[RDMA_NLDEV_ATTR_DEV_NAME],
+ sizeof(name)) >= sizeof(name))
+ return -EINVAL;
err = ib_device_rename(device, name);
goto done;
}
@@ -1300,14 +1301,18 @@ 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],
- sizeof(ibdev_name));
+ if (nla_strlcpy(ibdev_name, tb[RDMA_NLDEV_ATTR_DEV_NAME],
+ sizeof(ibdev_name)) >= sizeof(ibdev_name))
+ return -EINVAL;
if (strchr(ibdev_name, '%'))
return -EINVAL;
- nla_strlcpy(type, tb[RDMA_NLDEV_ATTR_LINK_TYPE], sizeof(type));
- nla_strlcpy(ndev_name, tb[RDMA_NLDEV_ATTR_NDEV_NAME],
- sizeof(ndev_name));
+ if (nla_strlcpy(type, tb[RDMA_NLDEV_ATTR_LINK_TYPE],
+ sizeof(type)) >= sizeof(type))
+ return -EINVAL;
+ if (nla_strlcpy(ndev_name, tb[RDMA_NLDEV_ATTR_NDEV_NAME],
+ sizeof(ndev_name)) >= sizeof(ndev_name))
+ return -EINVAL;
ndev = dev_get_by_name(&init_net, ndev_name);
if (!ndev)
diff --git a/include/rdma/rdma_netlink.h b/include/rdma/rdma_netlink.h
index c7acbe083428..2cac22e2f34a 100644
--- a/include/rdma/rdma_netlink.h
+++ b/include/rdma/rdma_netlink.h
@@ -6,6 +6,11 @@
#include <linux/netlink.h>
#include <uapi/rdma/rdma_netlink.h>
+enum {
+ RDMA_NLDEV_ATTR_EMPTY_STRING = 1,
+ RDMA_NLDEV_ATTR_ENTRY_STRLEN = 16,
+};
+
struct rdma_nl_cbs {
int (*doit)(struct sk_buff *skb, struct nlmsghdr *nlh,
struct netlink_ext_ack *extack);
diff --git a/include/uapi/rdma/rdma_netlink.h b/include/uapi/rdma/rdma_netlink.h
index b27c02185dcc..650cee8c4bf1 100644
--- a/include/uapi/rdma/rdma_netlink.h
+++ b/include/uapi/rdma/rdma_netlink.h
@@ -284,10 +284,6 @@ enum rdma_nldev_command {
RDMA_NLDEV_NUM_OPS
};
-enum {
- RDMA_NLDEV_ATTR_ENTRY_STRLEN = 16,
-};
-
enum rdma_nldev_print_type {
RDMA_NLDEV_PRINT_TYPE_UNSPEC,
RDMA_NLDEV_PRINT_TYPE_HEX,
--
2.21.0
