Re: [PATCH nf-next] netfilter: xt_osf: Use explicit member assignment to avoid implicit no padding rule
Hi Liping, On Tue, Sep 27, 2016 at 2:05 PM, Liping Zhang wrote: > Hi Feng, > > 2016-09-27 14:00 GMT+08:00 Gao Feng : >> Hi Liping, >> >>> >>> This xt_osf_user_finger{} is carefully designed, no padding now, and >>> will not be changed in the future, otherwise backward compatibility will >>> be broken. >> >> Yes, there is no padding now. So it is ok to use memcmp now. >> I am afraid the struct would be modified for other requirements. > > This is structure was passed by netlink attribute, so modify it will > break backward compatibility. Reasonable. Thanks Liping. Regards Feng > >> >> If it is never changed forever, it is ok certainly. >> >>> >>> I don't think this convert is necessary, actually it is a little ugly, and >>> will >>> increase the maintenance burden. >> >> I just want the codes don't depend any implicit rule. >> >> It is a tradeoff. If never change, needn't do any convert. >> If may change, the memcmp is a little dangerous. >> >> Regards >> Feng -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH nf-next] netfilter: xt_osf: Use explicit member assignment to avoid implicit no padding rule
Hi Feng, 2016-09-27 14:00 GMT+08:00 Gao Feng : > Hi Liping, > >> >> This xt_osf_user_finger{} is carefully designed, no padding now, and >> will not be changed in the future, otherwise backward compatibility will >> be broken. > > Yes, there is no padding now. So it is ok to use memcmp now. > I am afraid the struct would be modified for other requirements. This is structure was passed by netlink attribute, so modify it will break backward compatibility. > > If it is never changed forever, it is ok certainly. > >> >> I don't think this convert is necessary, actually it is a little ugly, and >> will >> increase the maintenance burden. > > I just want the codes don't depend any implicit rule. > > It is a tradeoff. If never change, needn't do any convert. > If may change, the memcmp is a little dangerous. > > Regards > Feng -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH nf-next] netfilter: xt_osf: Use explicit member assignment to avoid implicit no padding rule
Hi Liping, On Tue, Sep 27, 2016 at 1:49 PM, Liping Zhang wrote: > Hi Feng, > > 2016-09-27 12:39 GMT+08:00 : >> From: Gao Feng >> >> Current xt_osf codes use memcmp to check if two user fingers are same, >> so it depends on that the struct xt_osf_user_finger is no padding. >> It is one implicit rule, and is not good to maintain. >> >> Now use zero memory and assign the members explicitly. >> >> Signed-off-by: Gao Feng >> --- >> net/netfilter/xt_osf.c | 32 ++-- >> 1 file changed, 30 insertions(+), 2 deletions(-) >> >> diff --git a/net/netfilter/xt_osf.c b/net/netfilter/xt_osf.c >> index 2455b69..9793670 100644 >> --- a/net/netfilter/xt_osf.c >> +++ b/net/netfilter/xt_osf.c >> @@ -61,6 +61,34 @@ static const struct nla_policy xt_osf_policy[OSF_ATTR_MAX >> + 1] = { >> [OSF_ATTR_FINGER] = { .len = sizeof(struct xt_osf_user_finger) >> }, >> }; >> >> +static void copy_user_finger(struct xt_osf_user_finger *dst, >> +const struct xt_osf_user_finger *src) >> +{ >> +#define OSF_COPY_MEMBER(mem) dst->mem = src->mem >> + >> + int i; >> + >> + OSF_COPY_MEMBER(wss.wc); >> + OSF_COPY_MEMBER(wss.val); >> + >> + OSF_COPY_MEMBER(ttl); >> + OSF_COPY_MEMBER(df); >> + OSF_COPY_MEMBER(ss); >> + OSF_COPY_MEMBER(mss); >> + OSF_COPY_MEMBER(opt_num); >> + >> + memcpy(dst->genre, src->genre, sizeof(dst->genre)); >> + memcpy(dst->version, src->version, sizeof(dst->version)); >> + memcpy(dst->subtype, src->subtype, sizeof(dst->subtype)); >> + >> + for (i = 0; i < MAX_IPOPTLEN; ++i) { >> + OSF_COPY_MEMBER(opt[i].kind); >> + OSF_COPY_MEMBER(opt[i].length); >> + OSF_COPY_MEMBER(opt[i].wc.wc); >> + OSF_COPY_MEMBER(opt[i].wc.val); >> + } >> +} >> + > > This xt_osf_user_finger{} is carefully designed, no padding now, and > will not be changed in the future, otherwise backward compatibility will > be broken. Yes, there is no padding now. So it is ok to use memcmp now. I am afraid the struct would be modified for other requirements. If it is never changed forever, it is ok certainly. > > I don't think this convert is necessary, actually it is a little ugly, and > will > increase the maintenance burden. I just want the codes don't depend any implicit rule. It is a tradeoff. If never change, needn't do any convert. If may change, the memcmp is a little dangerous. Regards Feng -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH nf-next] netfilter: xt_osf: Use explicit member assignment to avoid implicit no padding rule
Hi Feng, 2016-09-27 12:39 GMT+08:00 : > From: Gao Feng > > Current xt_osf codes use memcmp to check if two user fingers are same, > so it depends on that the struct xt_osf_user_finger is no padding. > It is one implicit rule, and is not good to maintain. > > Now use zero memory and assign the members explicitly. > > Signed-off-by: Gao Feng > --- > net/netfilter/xt_osf.c | 32 ++-- > 1 file changed, 30 insertions(+), 2 deletions(-) > > diff --git a/net/netfilter/xt_osf.c b/net/netfilter/xt_osf.c > index 2455b69..9793670 100644 > --- a/net/netfilter/xt_osf.c > +++ b/net/netfilter/xt_osf.c > @@ -61,6 +61,34 @@ static const struct nla_policy xt_osf_policy[OSF_ATTR_MAX > + 1] = { > [OSF_ATTR_FINGER] = { .len = sizeof(struct xt_osf_user_finger) > }, > }; > > +static void copy_user_finger(struct xt_osf_user_finger *dst, > +const struct xt_osf_user_finger *src) > +{ > +#define OSF_COPY_MEMBER(mem) dst->mem = src->mem > + > + int i; > + > + OSF_COPY_MEMBER(wss.wc); > + OSF_COPY_MEMBER(wss.val); > + > + OSF_COPY_MEMBER(ttl); > + OSF_COPY_MEMBER(df); > + OSF_COPY_MEMBER(ss); > + OSF_COPY_MEMBER(mss); > + OSF_COPY_MEMBER(opt_num); > + > + memcpy(dst->genre, src->genre, sizeof(dst->genre)); > + memcpy(dst->version, src->version, sizeof(dst->version)); > + memcpy(dst->subtype, src->subtype, sizeof(dst->subtype)); > + > + for (i = 0; i < MAX_IPOPTLEN; ++i) { > + OSF_COPY_MEMBER(opt[i].kind); > + OSF_COPY_MEMBER(opt[i].length); > + OSF_COPY_MEMBER(opt[i].wc.wc); > + OSF_COPY_MEMBER(opt[i].wc.val); > + } > +} > + This xt_osf_user_finger{} is carefully designed, no padding now, and will not be changed in the future, otherwise backward compatibility will be broken. I don't think this convert is necessary, actually it is a little ugly, and will increase the maintenance burden. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH nf-next] netfilter: xt_osf: Use explicit member assignment to avoid implicit no padding rule
From: Gao Feng Current xt_osf codes use memcmp to check if two user fingers are same, so it depends on that the struct xt_osf_user_finger is no padding. It is one implicit rule, and is not good to maintain. Now use zero memory and assign the members explicitly. Signed-off-by: Gao Feng --- net/netfilter/xt_osf.c | 32 ++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/net/netfilter/xt_osf.c b/net/netfilter/xt_osf.c index 2455b69..9793670 100644 --- a/net/netfilter/xt_osf.c +++ b/net/netfilter/xt_osf.c @@ -61,6 +61,34 @@ static const struct nla_policy xt_osf_policy[OSF_ATTR_MAX + 1] = { [OSF_ATTR_FINGER] = { .len = sizeof(struct xt_osf_user_finger) }, }; +static void copy_user_finger(struct xt_osf_user_finger *dst, +const struct xt_osf_user_finger *src) +{ +#define OSF_COPY_MEMBER(mem) dst->mem = src->mem + + int i; + + OSF_COPY_MEMBER(wss.wc); + OSF_COPY_MEMBER(wss.val); + + OSF_COPY_MEMBER(ttl); + OSF_COPY_MEMBER(df); + OSF_COPY_MEMBER(ss); + OSF_COPY_MEMBER(mss); + OSF_COPY_MEMBER(opt_num); + + memcpy(dst->genre, src->genre, sizeof(dst->genre)); + memcpy(dst->version, src->version, sizeof(dst->version)); + memcpy(dst->subtype, src->subtype, sizeof(dst->subtype)); + + for (i = 0; i < MAX_IPOPTLEN; ++i) { + OSF_COPY_MEMBER(opt[i].kind); + OSF_COPY_MEMBER(opt[i].length); + OSF_COPY_MEMBER(opt[i].wc.wc); + OSF_COPY_MEMBER(opt[i].wc.val); + } +} + static int xt_osf_add_callback(struct net *net, struct sock *ctnl, struct sk_buff *skb, const struct nlmsghdr *nlh, const struct nlattr * const osf_attrs[]) @@ -77,11 +105,11 @@ static int xt_osf_add_callback(struct net *net, struct sock *ctnl, f = nla_data(osf_attrs[OSF_ATTR_FINGER]); - kf = kmalloc(sizeof(struct xt_osf_finger), GFP_KERNEL); + kf = kzalloc(sizeof(*kf), GFP_KERNEL); if (!kf) return -ENOMEM; - memcpy(&kf->finger, f, sizeof(struct xt_osf_user_finger)); + copy_user_finger(&kf->finger, f); list_for_each_entry(sf, &xt_osf_fingers[!!f->df], finger_entry) { if (memcmp(&sf->finger, f, sizeof(struct xt_osf_user_finger))) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html