* [PATCH nf-next] netfilter: xt_osf: Use explicit member assignment to avoid implicit no padding rule
@ 2016-09-27 4:39 fgao
2016-09-27 5:49 ` Liping Zhang
0 siblings, 1 reply; 5+ messages in thread
From: fgao @ 2016-09-27 4:39 UTC (permalink / raw)
To: pablo, kaber, netfilter-devel, netdev; +Cc: gfree.wind, Gao Feng
From: Gao Feng <fgao@ikuai8.com>
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 <fgao@ikuai8.com>
---
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH nf-next] netfilter: xt_osf: Use explicit member assignment to avoid implicit no padding rule
2016-09-27 4:39 [PATCH nf-next] netfilter: xt_osf: Use explicit member assignment to avoid implicit no padding rule fgao
@ 2016-09-27 5:49 ` Liping Zhang
2016-09-27 6:00 ` Gao Feng
0 siblings, 1 reply; 5+ messages in thread
From: Liping Zhang @ 2016-09-27 5:49 UTC (permalink / raw)
To: fgao; +Cc: Pablo Neira Ayuso, Patrick McHardy, netfilter-devel, netdev,
Feng Gao
Hi Feng,
2016-09-27 12:39 GMT+08:00 <fgao@ikuai8.com>:
> From: Gao Feng <fgao@ikuai8.com>
>
> 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 <fgao@ikuai8.com>
> ---
> 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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH nf-next] netfilter: xt_osf: Use explicit member assignment to avoid implicit no padding rule
2016-09-27 5:49 ` Liping Zhang
@ 2016-09-27 6:00 ` Gao Feng
2016-09-27 6:05 ` Liping Zhang
0 siblings, 1 reply; 5+ messages in thread
From: Gao Feng @ 2016-09-27 6:00 UTC (permalink / raw)
To: Liping Zhang
Cc: Pablo Neira Ayuso, Patrick McHardy,
Netfilter Developer Mailing List, Linux Kernel Network Developers
Hi Liping,
On Tue, Sep 27, 2016 at 1:49 PM, Liping Zhang <zlpnobody@gmail.com> wrote:
> Hi Feng,
>
> 2016-09-27 12:39 GMT+08:00 <fgao@ikuai8.com>:
>> From: Gao Feng <fgao@ikuai8.com>
>>
>> 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 <fgao@ikuai8.com>
>> ---
>> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH nf-next] netfilter: xt_osf: Use explicit member assignment to avoid implicit no padding rule
2016-09-27 6:00 ` Gao Feng
@ 2016-09-27 6:05 ` Liping Zhang
2016-09-27 6:10 ` Gao Feng
0 siblings, 1 reply; 5+ messages in thread
From: Liping Zhang @ 2016-09-27 6:05 UTC (permalink / raw)
To: Gao Feng
Cc: Pablo Neira Ayuso, Patrick McHardy,
Netfilter Developer Mailing List, Linux Kernel Network Developers
Hi Feng,
2016-09-27 14:00 GMT+08:00 Gao Feng <fgao@ikuai8.com>:
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH nf-next] netfilter: xt_osf: Use explicit member assignment to avoid implicit no padding rule
2016-09-27 6:05 ` Liping Zhang
@ 2016-09-27 6:10 ` Gao Feng
0 siblings, 0 replies; 5+ messages in thread
From: Gao Feng @ 2016-09-27 6:10 UTC (permalink / raw)
To: Liping Zhang
Cc: Pablo Neira Ayuso, Patrick McHardy,
Netfilter Developer Mailing List, Linux Kernel Network Developers
Hi Liping,
On Tue, Sep 27, 2016 at 2:05 PM, Liping Zhang <zlpnobody@gmail.com> wrote:
> Hi Feng,
>
> 2016-09-27 14:00 GMT+08:00 Gao Feng <fgao@ikuai8.com>:
>> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-09-27 6:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-27 4:39 [PATCH nf-next] netfilter: xt_osf: Use explicit member assignment to avoid implicit no padding rule fgao
2016-09-27 5:49 ` Liping Zhang
2016-09-27 6:00 ` Gao Feng
2016-09-27 6:05 ` Liping Zhang
2016-09-27 6:10 ` Gao Feng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).