| Message ID | 20171115120932.A6A87A0CA8@unicorn.suse.cz |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] genetlink: fix genlmsg_nlhdr() | expand |
On Wed, 2017-11-15 at 13:09 +0100, Michal Kubecek wrote: > According to the description, first argument of genlmsg_nlhdr() points to > what genlmsg_put() returns, i.e. beginning of user header. Therefore we > should only subtract size of genetlink header and netlink message header, > not user header. > > This also means we don't need to pass the pointer to genetlink family and > the same is true for genl_dump_check_consistent() which is the only caller > of genlmsg_nlhdr(). (Note that at the moment, these functions are only > used for families which do not have user header so that they are not > affected.) > > Fixes: 670dc2833d14 ("netlink: advertise incomplete dumps") > Signed-off-by: Michal Kubecek <mkubecek@suse.cz> Looks sensible, though I don't think it's really needed in net since it really has no effect - as you note, family->hdrsize is 0 for all the families calling this right now. Reviewed-by: Johannes Berg <johannes@sipsolutions.net> johannes
On Wed, Nov 15, 2017 at 01:20:10PM +0100, Johannes Berg wrote: > On Wed, 2017-11-15 at 13:09 +0100, Michal Kubecek wrote: > > According to the description, first argument of genlmsg_nlhdr() points to > > what genlmsg_put() returns, i.e. beginning of user header. Therefore we > > should only subtract size of genetlink header and netlink message header, > > not user header. > > > > This also means we don't need to pass the pointer to genetlink family and > > the same is true for genl_dump_check_consistent() which is the only caller > > of genlmsg_nlhdr(). (Note that at the moment, these functions are only > > used for families which do not have user header so that they are not > > affected.) > > > > Fixes: 670dc2833d14 ("netlink: advertise incomplete dumps") > > Signed-off-by: Michal Kubecek <mkubecek@suse.cz> > > Looks sensible, though I don't think it's really needed in net since it > really has no effect - as you note, family->hdrsize is 0 for all the > families calling this right now. > > Reviewed-by: Johannes Berg <johannes@sipsolutions.net> Well, I'm currently working on one with hdrsize > 0 which is how I found this. :-) But it will still need some time before it can get even into net-next. Michal Kubecek
From: Michal Kubecek <mkubecek@suse.cz> Date: Wed, 15 Nov 2017 13:09:32 +0100 (CET) > According to the description, first argument of genlmsg_nlhdr() points to > what genlmsg_put() returns, i.e. beginning of user header. Therefore we > should only subtract size of genetlink header and netlink message header, > not user header. > > This also means we don't need to pass the pointer to genetlink family and > the same is true for genl_dump_check_consistent() which is the only caller > of genlmsg_nlhdr(). (Note that at the moment, these functions are only > used for families which do not have user header so that they are not > affected.) > > Fixes: 670dc2833d14 ("netlink: advertise incomplete dumps") > Signed-off-by: Michal Kubecek <mkubecek@suse.cz> Applied.
diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c index 5ab1b8849c30..da3975c5636d 100644 --- a/drivers/net/macsec.c +++ b/drivers/net/macsec.c @@ -2410,7 +2410,7 @@ static int dump_secy(struct macsec_secy *secy, struct net_device *dev, if (!hdr) return -EMSGSIZE; - genl_dump_check_consistent(cb, hdr, &macsec_fam); + genl_dump_check_consistent(cb, hdr); if (nla_put_u32(skb, MACSEC_ATTR_IFINDEX, dev->ifindex)) goto nla_put_failure; diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c index 6467ffac9811..3244d957454b 100644 --- a/drivers/net/wireless/mac80211_hwsim.c +++ b/drivers/net/wireless/mac80211_hwsim.c @@ -2805,7 +2805,7 @@ static int mac80211_hwsim_get_radio(struct sk_buff *skb, return -EMSGSIZE; if (cb) - genl_dump_check_consistent(cb, hdr, &hwsim_genl_family); + genl_dump_check_consistent(cb, hdr); if (data->alpha2[0] && data->alpha2[1]) param.reg_alpha2 = data->alpha2; diff --git a/include/net/genetlink.h b/include/net/genetlink.h index 5ac169a735f4..decf6012a401 100644 --- a/include/net/genetlink.h +++ b/include/net/genetlink.h @@ -154,15 +154,12 @@ void *genlmsg_put(struct sk_buff *skb, u32 portid, u32 seq, /** * genlmsg_nlhdr - Obtain netlink header from user specified header * @user_hdr: user header as returned from genlmsg_put() - * @family: generic netlink family * * Returns pointer to netlink header. */ -static inline struct nlmsghdr * -genlmsg_nlhdr(void *user_hdr, const struct genl_family *family) +static inline struct nlmsghdr *genlmsg_nlhdr(void *user_hdr) { return (struct nlmsghdr *)((char *)user_hdr - - family->hdrsize - GENL_HDRLEN - NLMSG_HDRLEN); } @@ -190,16 +187,14 @@ static inline int genlmsg_parse(const struct nlmsghdr *nlh, * genl_dump_check_consistent - check if sequence is consistent and advertise if not * @cb: netlink callback structure that stores the sequence number * @user_hdr: user header as returned from genlmsg_put() - * @family: generic netlink family * * Cf. nl_dump_check_consistent(), this just provides a wrapper to make it * simpler to use with generic netlink. */ static inline void genl_dump_check_consistent(struct netlink_callback *cb, - void *user_hdr, - const struct genl_family *family) + void *user_hdr) { - nl_dump_check_consistent(cb, genlmsg_nlhdr(user_hdr, family)); + nl_dump_check_consistent(cb, genlmsg_nlhdr(user_hdr)); } /** diff --git a/net/nfc/netlink.c b/net/nfc/netlink.c index b251fb936a27..371352f4dd60 100644 --- a/net/nfc/netlink.c +++ b/net/nfc/netlink.c @@ -75,7 +75,7 @@ static int nfc_genl_send_target(struct sk_buff *msg, struct nfc_target *target, if (!hdr) return -EMSGSIZE; - genl_dump_check_consistent(cb, hdr, &nfc_genl_family); + genl_dump_check_consistent(cb, hdr); if (nla_put_u32(msg, NFC_ATTR_TARGET_INDEX, target->idx) || nla_put_u32(msg, NFC_ATTR_PROTOCOLS, target->supported_protocols) || @@ -603,7 +603,7 @@ static int nfc_genl_send_device(struct sk_buff *msg, struct nfc_dev *dev, return -EMSGSIZE; if (cb) - genl_dump_check_consistent(cb, hdr, &nfc_genl_family); + genl_dump_check_consistent(cb, hdr); if (nfc_genl_setup_device_added(dev, msg)) goto nla_put_failure; @@ -1332,7 +1332,7 @@ static int nfc_genl_send_se(struct sk_buff *msg, struct nfc_dev *dev, goto nla_put_failure; if (cb) - genl_dump_check_consistent(cb, hdr, &nfc_genl_family); + genl_dump_check_consistent(cb, hdr); if (nla_put_u32(msg, NFC_ATTR_DEVICE_INDEX, dev->idx) || nla_put_u32(msg, NFC_ATTR_SE_INDEX, se->idx) || diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index d396cb61a280..0488d714224a 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -6277,7 +6277,7 @@ static int nl80211_send_regdom(struct sk_buff *msg, struct netlink_callback *cb, if (!hdr) return -1; - genl_dump_check_consistent(cb, hdr, &nl80211_fam); + genl_dump_check_consistent(cb, hdr); if (nl80211_put_regdom(regdom, msg)) goto nla_put_failure; @@ -7689,7 +7689,7 @@ static int nl80211_send_bss(struct sk_buff *msg, struct netlink_callback *cb, if (!hdr) return -1; - genl_dump_check_consistent(cb, hdr, &nl80211_fam); + genl_dump_check_consistent(cb, hdr); if (nla_put_u32(msg, NL80211_ATTR_GENERATION, rdev->bss_generation)) goto nla_put_failure;
According to the description, first argument of genlmsg_nlhdr() points to what genlmsg_put() returns, i.e. beginning of user header. Therefore we should only subtract size of genetlink header and netlink message header, not user header. This also means we don't need to pass the pointer to genetlink family and the same is true for genl_dump_check_consistent() which is the only caller of genlmsg_nlhdr(). (Note that at the moment, these functions are only used for families which do not have user header so that they are not affected.) Fixes: 670dc2833d14 ("netlink: advertise incomplete dumps") Signed-off-by: Michal Kubecek <mkubecek@suse.cz> --- drivers/net/macsec.c | 2 +- drivers/net/wireless/mac80211_hwsim.c | 2 +- include/net/genetlink.h | 11 +++-------- net/nfc/netlink.c | 6 +++--- net/wireless/nl80211.c | 4 ++-- 5 files changed, 10 insertions(+), 15 deletions(-)