From 4ee007bdd8b893fb4f0c9a8c1d5053546ec0fcc2 Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Sun, 21 Jul 2019 15:50:05 +0300 Subject: genl: Always call subdissector Commit 61c5e8e76d21 ("genl: make subdissectors responsible for header") changed the generic netlink dissector to only call a sub-dissector if there is a payload after the generic netlink header. However, there are commands in certain generic netlink families that do not have any payload. For example, 'NET_DM_CMD_START' in the 'NET_DM' family. This means that the command will not be dissected by the subdissector, as it will never be invoked. Change the generic netlink dissector to always call a subdissector, if it is present. Prevent the subdissectors from trying to dissect past the end of the packet by adding checks in the two existing subdissectors, for the 'nlctrl' and 'nl80211' families. Change-Id: I4d2f48531dee92b11dc45000081a8d2d3dd875c6 Signed-off-by: Ido Schimmel Reviewed-on: https://code.wireshark.org/review/34350 Reviewed-by: Peter Wu Petri-Dish: Peter Wu Tested-by: Petri Dish Buildbot Reviewed-by: Anders Broman --- epan/dissectors/packet-netlink-generic.c | 19 ++++++++++--------- epan/dissectors/packet-netlink-nl80211.c | 4 ++++ 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/epan/dissectors/packet-netlink-generic.c b/epan/dissectors/packet-netlink-generic.c index b7f29313d0..4676a80f2a 100644 --- a/epan/dissectors/packet-netlink-generic.c +++ b/epan/dissectors/packet-netlink-generic.c @@ -366,6 +366,10 @@ dissect_genl_ctrl(tvbuff_t *tvb, packet_info *pinfo _U_, proto_tree *tree _U_, v offset = dissect_genl_header(tvb, genl_info, &hfi_genl_ctrl_cmd); + /* Return if command has no payload */ + if (!tvb_reported_length_remaining(tvb, offset)) + return offset; + dissect_netlink_attributes(tvb, &hfi_genl_ctrl_attr, ett_genl_ctrl_attr, &info, info.data, genl_info->genl_tree, offset, -1, dissect_genl_ctrl_attrs); /* @@ -445,15 +449,12 @@ dissect_netlink_generic(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, voi /* Optional user-specific message header and optional message payload. */ next_tvb = tvb_new_subset_remaining(tvb, offset); - /* Try subdissector if there is a payload. */ - if (tvb_reported_length_remaining(tvb, offset + 4)) { - if (family_name) { - int ret; - /* Invoke subdissector with genlmsghdr present. */ - ret = dissector_try_string(genl_dissector_table, family_name, next_tvb, pinfo, tree, &info); - if (ret) { - return ret; - } + if (family_name) { + int ret; + /* Invoke subdissector with genlmsghdr present. */ + ret = dissector_try_string(genl_dissector_table, family_name, next_tvb, pinfo, tree, &info); + if (ret) { + return ret; } } diff --git a/epan/dissectors/packet-netlink-nl80211.c b/epan/dissectors/packet-netlink-nl80211.c index 007e88b8e5..07ffaaf40d 100644 --- a/epan/dissectors/packet-netlink-nl80211.c +++ b/epan/dissectors/packet-netlink-nl80211.c @@ -2901,6 +2901,10 @@ dissect_netlink_nl80211(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, voi offset = dissect_genl_header(tvb, genl_info, &hfi_nl80211_commands); + /* Return if command has no payload */ + if (!tvb_reported_length_remaining(tvb, offset)) + return offset; + pi = proto_tree_add_item(tree, proto_registrar_get_nth(proto_netlink_nl80211), tvb, offset, -1, ENC_NA); nlmsg_tree = proto_item_add_subtree(pi, ett_nl80211); -- cgit v1.2.3