* [PATCH] netfilter: nf_tables: check if payload offset is a power of 2
@ 2014-02-16 11:54 Nikolay Aleksandrov
2014-02-16 12:07 ` Patrick McHardy
0 siblings, 1 reply; 6+ messages in thread
From: Nikolay Aleksandrov @ 2014-02-16 11:54 UTC (permalink / raw)
To: netfilter-devel; +Cc: pablo, kaber, Nikolay Aleksandrov
Add a check if payload's offset is a power of 2 when selecting ops.
The fast ops were meant for well aligned offsets, also this fixes a
small bug when using unaligned offset and length of 3 which causes
only 1 byte to be loaded because the fast ops are chosen.
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
I broke the line since it was >80 chars.
This patch applies to Dave's -net tree.
net/netfilter/nft_payload.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/nft_payload.c b/net/netfilter/nft_payload.c
index a2aeb31..15a3dcb 100644
--- a/net/netfilter/nft_payload.c
+++ b/net/netfilter/nft_payload.c
@@ -135,7 +135,9 @@ nft_payload_select_ops(const struct nft_ctx *ctx,
if (len == 0 || len > FIELD_SIZEOF(struct nft_data, data))
return ERR_PTR(-EINVAL);
- if (len <= 4 && IS_ALIGNED(offset, len) && base != NFT_PAYLOAD_LL_HEADER)
+ if (len <= 4 && IS_ALIGNED(offset, len) &&
+ base != NFT_PAYLOAD_LL_HEADER &&
+ !(offset & (offset - 1)))
return &nft_payload_fast_ops;
else
return &nft_payload_ops;
--
1.8.4.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] netfilter: nf_tables: check if payload offset is a power of 2
2014-02-16 11:54 [PATCH] netfilter: nf_tables: check if payload offset is a power of 2 Nikolay Aleksandrov
@ 2014-02-16 12:07 ` Patrick McHardy
2014-02-16 12:09 ` Nikolay Aleksandrov
0 siblings, 1 reply; 6+ messages in thread
From: Patrick McHardy @ 2014-02-16 12:07 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netfilter-devel, pablo
On Sun, Feb 16, 2014 at 12:54:40PM +0100, Nikolay Aleksandrov wrote:
> Add a check if payload's offset is a power of 2 when selecting ops.
> The fast ops were meant for well aligned offsets, also this fixes a
> small bug when using unaligned offset and length of 3 which causes
> only 1 byte to be loaded because the fast ops are chosen.
>
> Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
> ---
> I broke the line since it was >80 chars.
> This patch applies to Dave's -net tree.
>
> net/netfilter/nft_payload.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/netfilter/nft_payload.c b/net/netfilter/nft_payload.c
> index a2aeb31..15a3dcb 100644
> --- a/net/netfilter/nft_payload.c
> +++ b/net/netfilter/nft_payload.c
> @@ -135,7 +135,9 @@ nft_payload_select_ops(const struct nft_ctx *ctx,
> if (len == 0 || len > FIELD_SIZEOF(struct nft_data, data))
> return ERR_PTR(-EINVAL);
>
> - if (len <= 4 && IS_ALIGNED(offset, len) && base != NFT_PAYLOAD_LL_HEADER)
> + if (len <= 4 && IS_ALIGNED(offset, len) &&
> + base != NFT_PAYLOAD_LL_HEADER &&
> + !(offset & (offset - 1)))
Small cosmetic note: we have is_power_of_2(), which does exactly this.
Also I think the more logical order would be:
len <= 4 && is_power_of_2 && IS_ALIGNED && base ...
> return &nft_payload_fast_ops;
> else
> return &nft_payload_ops;
> --
> 1.8.4.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] netfilter: nf_tables: check if payload offset is a power of 2
2014-02-16 12:07 ` Patrick McHardy
@ 2014-02-16 12:09 ` Nikolay Aleksandrov
2014-02-16 12:14 ` Nikolay Aleksandrov
2014-02-16 12:27 ` Patrick McHardy
0 siblings, 2 replies; 6+ messages in thread
From: Nikolay Aleksandrov @ 2014-02-16 12:09 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netfilter-devel, pablo
On 02/16/2014 01:07 PM, Patrick McHardy wrote:
> On Sun, Feb 16, 2014 at 12:54:40PM +0100, Nikolay Aleksandrov wrote:
>> Add a check if payload's offset is a power of 2 when selecting ops.
>> The fast ops were meant for well aligned offsets, also this fixes a
>> small bug when using unaligned offset and length of 3 which causes
>> only 1 byte to be loaded because the fast ops are chosen.
>>
>> Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
>> ---
>> I broke the line since it was >80 chars.
>> This patch applies to Dave's -net tree.
>>
>> net/netfilter/nft_payload.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/netfilter/nft_payload.c b/net/netfilter/nft_payload.c
>> index a2aeb31..15a3dcb 100644
>> --- a/net/netfilter/nft_payload.c
>> +++ b/net/netfilter/nft_payload.c
>> @@ -135,7 +135,9 @@ nft_payload_select_ops(const struct nft_ctx *ctx,
>> if (len == 0 || len > FIELD_SIZEOF(struct nft_data, data))
>> return ERR_PTR(-EINVAL);
>>
>> - if (len <= 4 && IS_ALIGNED(offset, len) && base != NFT_PAYLOAD_LL_HEADER)
>> + if (len <= 4 && IS_ALIGNED(offset, len) &&
>> + base != NFT_PAYLOAD_LL_HEADER &&
>> + !(offset & (offset - 1)))
>
> Small cosmetic note: we have is_power_of_2(), which does exactly this.
>
I didn't choose is_power_of_2 because it also checks if x is != 0, so I
thought we can allow for 0 offset with the fast ops.
> Also I think the more logical order would be:
>
> len <= 4 && is_power_of_2 && IS_ALIGNED && base ...
>
Okay :-)
>> return &nft_payload_fast_ops;
>> else
>> return &nft_payload_ops;
>> --
>> 1.8.4.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] netfilter: nf_tables: check if payload offset is a power of 2
2014-02-16 12:09 ` Nikolay Aleksandrov
@ 2014-02-16 12:14 ` Nikolay Aleksandrov
2014-02-16 12:27 ` Patrick McHardy
1 sibling, 0 replies; 6+ messages in thread
From: Nikolay Aleksandrov @ 2014-02-16 12:14 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netfilter-devel, pablo
On 02/16/2014 01:09 PM, Nikolay Aleksandrov wrote:
> On 02/16/2014 01:07 PM, Patrick McHardy wrote:
>> On Sun, Feb 16, 2014 at 12:54:40PM +0100, Nikolay Aleksandrov wrote:
>>> Add a check if payload's offset is a power of 2 when selecting ops.
>>> The fast ops were meant for well aligned offsets, also this fixes a
>>> small bug when using unaligned offset and length of 3 which causes
>>> only 1 byte to be loaded because the fast ops are chosen.
>>>
>>> Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
>>> ---
>>> I broke the line since it was >80 chars.
>>> This patch applies to Dave's -net tree.
>>>
>>> net/netfilter/nft_payload.c | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/net/netfilter/nft_payload.c b/net/netfilter/nft_payload.c
>>> index a2aeb31..15a3dcb 100644
>>> --- a/net/netfilter/nft_payload.c
>>> +++ b/net/netfilter/nft_payload.c
>>> @@ -135,7 +135,9 @@ nft_payload_select_ops(const struct nft_ctx *ctx,
>>> if (len == 0 || len > FIELD_SIZEOF(struct nft_data, data))
>>> return ERR_PTR(-EINVAL);
>>>
>>> - if (len <= 4 && IS_ALIGNED(offset, len) && base != NFT_PAYLOAD_LL_HEADER)
>>> + if (len <= 4 && IS_ALIGNED(offset, len) &&
>>> + base != NFT_PAYLOAD_LL_HEADER &&
>>> + !(offset & (offset - 1)))
>>
>> Small cosmetic note: we have is_power_of_2(), which does exactly this.
>>
> I didn't choose is_power_of_2 because it also checks if x is != 0, so I
> thought we can allow for 0 offset with the fast ops.
>
ugh, forget this, I'll fix it.
>> Also I think the more logical order would be:
>>
>> len <= 4 && is_power_of_2 && IS_ALIGNED && base ...
>>
> Okay :-)
>
>>> return &nft_payload_fast_ops;
>>> else
>>> return &nft_payload_ops;
>>> --
>>> 1.8.4.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] netfilter: nf_tables: check if payload offset is a power of 2
2014-02-16 12:09 ` Nikolay Aleksandrov
2014-02-16 12:14 ` Nikolay Aleksandrov
@ 2014-02-16 12:27 ` Patrick McHardy
2014-02-16 12:29 ` Nikolay Aleksandrov
1 sibling, 1 reply; 6+ messages in thread
From: Patrick McHardy @ 2014-02-16 12:27 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netfilter-devel, pablo
On Sun, Feb 16, 2014 at 01:09:43PM +0100, Nikolay Aleksandrov wrote:
> On 02/16/2014 01:07 PM, Patrick McHardy wrote:
> > On Sun, Feb 16, 2014 at 12:54:40PM +0100, Nikolay Aleksandrov wrote:
> >> Add a check if payload's offset is a power of 2 when selecting ops.
> >> The fast ops were meant for well aligned offsets, also this fixes a
> >> small bug when using unaligned offset and length of 3 which causes
> >> only 1 byte to be loaded because the fast ops are chosen.
> >>
> >> Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
> >> ---
> >> I broke the line since it was >80 chars.
> >> This patch applies to Dave's -net tree.
> >>
> >> net/netfilter/nft_payload.c | 4 +++-
> >> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/net/netfilter/nft_payload.c b/net/netfilter/nft_payload.c
> >> index a2aeb31..15a3dcb 100644
> >> --- a/net/netfilter/nft_payload.c
> >> +++ b/net/netfilter/nft_payload.c
> >> @@ -135,7 +135,9 @@ nft_payload_select_ops(const struct nft_ctx *ctx,
> >> if (len == 0 || len > FIELD_SIZEOF(struct nft_data, data))
> >> return ERR_PTR(-EINVAL);
> >>
> >> - if (len <= 4 && IS_ALIGNED(offset, len) && base != NFT_PAYLOAD_LL_HEADER)
> >> + if (len <= 4 && IS_ALIGNED(offset, len) &&
> >> + base != NFT_PAYLOAD_LL_HEADER &&
> >> + !(offset & (offset - 1)))
> >
> > Small cosmetic note: we have is_power_of_2(), which does exactly this.
> >
> I didn't choose is_power_of_2 because it also checks if x is != 0, so I
> thought we can allow for 0 offset with the fast ops.
Sorry for not noticing before, but I'd propose to check length for power
of two. We're trying to determine whether the load is well aligned.
>
> > Also I think the more logical order would be:
> >
> > len <= 4 && is_power_of_2 && IS_ALIGNED && base ...
> >
> Okay :-)
>
> >> return &nft_payload_fast_ops;
> >> else
> >> return &nft_payload_ops;
> >> --
> >> 1.8.4.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] netfilter: nf_tables: check if payload offset is a power of 2
2014-02-16 12:27 ` Patrick McHardy
@ 2014-02-16 12:29 ` Nikolay Aleksandrov
0 siblings, 0 replies; 6+ messages in thread
From: Nikolay Aleksandrov @ 2014-02-16 12:29 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netfilter-devel, pablo
On 02/16/2014 01:27 PM, Patrick McHardy wrote:
> On Sun, Feb 16, 2014 at 01:09:43PM +0100, Nikolay Aleksandrov wrote:
>> On 02/16/2014 01:07 PM, Patrick McHardy wrote:
>>> On Sun, Feb 16, 2014 at 12:54:40PM +0100, Nikolay Aleksandrov wrote:
>>>> Add a check if payload's offset is a power of 2 when selecting ops.
>>>> The fast ops were meant for well aligned offsets, also this fixes a
>>>> small bug when using unaligned offset and length of 3 which causes
>>>> only 1 byte to be loaded because the fast ops are chosen.
>>>>
>>>> Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
>>>> ---
>>>> I broke the line since it was >80 chars.
>>>> This patch applies to Dave's -net tree.
>>>>
>>>> net/netfilter/nft_payload.c | 4 +++-
>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/net/netfilter/nft_payload.c b/net/netfilter/nft_payload.c
>>>> index a2aeb31..15a3dcb 100644
>>>> --- a/net/netfilter/nft_payload.c
>>>> +++ b/net/netfilter/nft_payload.c
>>>> @@ -135,7 +135,9 @@ nft_payload_select_ops(const struct nft_ctx *ctx,
>>>> if (len == 0 || len > FIELD_SIZEOF(struct nft_data, data))
>>>> return ERR_PTR(-EINVAL);
>>>>
>>>> - if (len <= 4 && IS_ALIGNED(offset, len) && base != NFT_PAYLOAD_LL_HEADER)
>>>> + if (len <= 4 && IS_ALIGNED(offset, len) &&
>>>> + base != NFT_PAYLOAD_LL_HEADER &&
>>>> + !(offset & (offset - 1)))
>>>
>>> Small cosmetic note: we have is_power_of_2(), which does exactly this.
>>>
>> I didn't choose is_power_of_2 because it also checks if x is != 0, so I
>> thought we can allow for 0 offset with the fast ops.
>
> Sorry for not noticing before, but I'd propose to check length for power
> of two. We're trying to determine whether the load is well aligned.
>
hehe, I was wondering why the offset but decided to just go with it,
should've asked.
Okay, going for v3 I guess.
>>
>>> Also I think the more logical order would be:
>>>
>>> len <= 4 && is_power_of_2 && IS_ALIGNED && base ...
>>>
>> Okay :-)
>>
>>>> return &nft_payload_fast_ops;
>>>> else
>>>> return &nft_payload_ops;
>>>> --
>>>> 1.8.4.2
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-02-16 12:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-16 11:54 [PATCH] netfilter: nf_tables: check if payload offset is a power of 2 Nikolay Aleksandrov
2014-02-16 12:07 ` Patrick McHardy
2014-02-16 12:09 ` Nikolay Aleksandrov
2014-02-16 12:14 ` Nikolay Aleksandrov
2014-02-16 12:27 ` Patrick McHardy
2014-02-16 12:29 ` Nikolay Aleksandrov
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).