devlink: refactor end checks in devlink_nl_cmd_region_read_dumpit

Clean up after recent fixes, move address calculations
around and change the variable init, so that we can have
just one start_offset == end_offset check.

Make the check a little stricter to preserve the -EINVAL
error if requested start offset is larger than the region
itself.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 20f935fa29f5167a0293a228c297793714287b3b..7b76e5fffc108eda0c7d531e69ed449910f1a498 100644 (file)
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -4215,7 +4215,6 @@ static int devlink_nl_region_read_snapshot_fill(struct sk_buff *skb,
                                                struct nlattr **attrs,
                                                u64 start_offset,
                                                u64 end_offset,
-                                               bool dump,
                                                u64 *new_offset)
 {
        struct devlink_snapshot *snapshot;
@@ -4230,9 +4229,6 @@ static int devlink_nl_region_read_snapshot_fill(struct sk_buff *skb,
        if (!snapshot)
                return -EINVAL;
 
-       if (end_offset > region->size || dump)
-               end_offset = region->size;
-
        while (curr_offset < end_offset) {
                u32 data_size;
                u8 *data;
@@ -4260,13 +4256,12 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
                                             struct netlink_callback *cb)
 {
        const struct genl_dumpit_info *info = genl_dumpit_info(cb);
-       u64 ret_offset, start_offset, end_offset = 0;
+       u64 ret_offset, start_offset, end_offset = U64_MAX;
        struct nlattr **attrs = info->attrs;
        struct devlink_region *region;
        struct nlattr *chunks_attr;
        const char *region_name;
        struct devlink *devlink;
-       bool dump = true;
        void *hdr;
        int err;
 
@@ -4294,8 +4289,21 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
                goto out_unlock;
        }
 
+       if (attrs[DEVLINK_ATTR_REGION_CHUNK_ADDR] &&
+           attrs[DEVLINK_ATTR_REGION_CHUNK_LEN]) {
+               if (!start_offset)
+                       start_offset =
+                               nla_get_u64(attrs[DEVLINK_ATTR_REGION_CHUNK_ADDR]);
+
+               end_offset = nla_get_u64(attrs[DEVLINK_ATTR_REGION_CHUNK_ADDR]);
+               end_offset += nla_get_u64(attrs[DEVLINK_ATTR_REGION_CHUNK_LEN]);
+       }
+
+       if (end_offset > region->size)
+               end_offset = region->size;
+
        /* return 0 if there is no further data to read */
-       if (start_offset >= region->size) {
+       if (start_offset == end_offset) {
                err = 0;
                goto out_unlock;
        }
@@ -4322,27 +4330,10 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
                goto nla_put_failure;
        }
 
-       if (attrs[DEVLINK_ATTR_REGION_CHUNK_ADDR] &&
-           attrs[DEVLINK_ATTR_REGION_CHUNK_LEN]) {
-               if (!start_offset)
-                       start_offset =
-                               nla_get_u64(attrs[DEVLINK_ATTR_REGION_CHUNK_ADDR]);
-
-               end_offset = nla_get_u64(attrs[DEVLINK_ATTR_REGION_CHUNK_ADDR]);
-               end_offset += nla_get_u64(attrs[DEVLINK_ATTR_REGION_CHUNK_LEN]);
-               dump = false;
-
-               if (start_offset == end_offset) {
-                       err = 0;
-                       goto nla_put_failure;
-               }
-       }
-
        err = devlink_nl_region_read_snapshot_fill(skb, devlink,
                                                   region, attrs,
                                                   start_offset,
-                                                  end_offset, dump,
-                                                  &ret_offset);
+                                                  end_offset, &ret_offset);
 
        if (err && err != -EMSGSIZE)
                goto nla_put_failure;

diff --git a/tools/testing/selftests/drivers/net/netdevsim/devlink.sh b/tools/testing/selftests/drivers/net/netdevsim/devlink.sh
index ad539eccddcb3cec2789a2fdc9ebbfd711e54ce6..de4b32fc422336789c207069c0467b5cf233787d 100755 (executable)
--- a/tools/testing/selftests/drivers/net/netdevsim/devlink.sh
+++ b/tools/testing/selftests/drivers/net/netdevsim/devlink.sh
@@ -146,6 +146,21 @@ regions_test()
 
        check_region_snapshot_count dummy post-first-request 3
 
+       devlink region dump $DL_HANDLE/dummy snapshot 25 >> /dev/null
+       check_err $? "Failed to dump snapshot with id 25"
+
+       devlink region read $DL_HANDLE/dummy snapshot 25 addr 0 len 1 >> /dev/null
+       check_err $? "Failed to read snapshot with id 25 (1 byte)"
+
+       devlink region read $DL_HANDLE/dummy snapshot 25 addr 128 len 128 >> /dev/null
+       check_err $? "Failed to read snapshot with id 25 (128 bytes)"
+
+       devlink region read $DL_HANDLE/dummy snapshot 25 addr 128 len $((1<<32)) >> /dev/null
+       check_err $? "Failed to read snapshot with id 25 (oversized)"
+
+       devlink region read $DL_HANDLE/dummy snapshot 25 addr $((1<<32)) len 128 >> /dev/null 2>&1
+       check_fail $? "Bad read of snapshot with id 25 did not fail"
+
        devlink region del $DL_HANDLE/dummy snapshot 25
        check_err $? "Failed to delete snapshot with id 25"