* [PATCH] net: fix checking boundary of valid vlan id
@ 2012-12-30 6:28 akong
2012-12-30 6:39 ` Amos Kong
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: akong @ 2012-12-30 6:28 UTC (permalink / raw)
To: netdev; +Cc: davem
From: Amos Kong <akong@redhat.com>
4096 is not a valid vlan id.
Signed-off-by: Amos Kong <akong@redhat.com>
---
net/bridge/netfilter/ebt_vlan.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/bridge/netfilter/ebt_vlan.c b/net/bridge/netfilter/ebt_vlan.c
index eae67bf..b279ec0 100644
--- a/net/bridge/netfilter/ebt_vlan.c
+++ b/net/bridge/netfilter/ebt_vlan.c
@@ -121,8 +121,8 @@ static int ebt_vlan_mt_check(const struct xt_mtchk_param *par)
* if_vlan.h: VLAN_N_VID 4096. */
if (GET_BITMASK(EBT_VLAN_ID)) {
if (!!info->id) { /* if id!=0 => check vid range */
- if (info->id > VLAN_N_VID) {
- pr_debug("id %d is out of range (1-4096)\n",
+ if (info->id >= VLAN_N_VID) {
+ pr_debug("id %d is out of range (1-4095)\n",
info->id);
return -EINVAL;
}
--
1.7.11.7
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] net: fix checking boundary of valid vlan id 2012-12-30 6:28 [PATCH] net: fix checking boundary of valid vlan id akong @ 2012-12-30 6:39 ` Amos Kong 2012-12-30 7:40 ` David Miller ` (2 subsequent siblings) 3 siblings, 0 replies; 9+ messages in thread From: Amos Kong @ 2012-12-30 6:39 UTC (permalink / raw) To: netdev; +Cc: davem On Sun, Dec 30, 2012 at 02:28:51PM +0800, akong@redhat.com wrote: > From: Amos Kong <akong@redhat.com> > > 4096 is not a valid vlan id. > > Signed-off-by: Amos Kong <akong@redhat.com> > --- > net/bridge/netfilter/ebt_vlan.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/bridge/netfilter/ebt_vlan.c b/net/bridge/netfilter/ebt_vlan.c > index eae67bf..b279ec0 100644 > --- a/net/bridge/netfilter/ebt_vlan.c > +++ b/net/bridge/netfilter/ebt_vlan.c > @@ -121,8 +121,8 @@ static int ebt_vlan_mt_check(const struct xt_mtchk_param *par) > * if_vlan.h: VLAN_N_VID 4096. */ > if (GET_BITMASK(EBT_VLAN_ID)) { > if (!!info->id) { /* if id!=0 => check vid range */ > - if (info->id > VLAN_N_VID) { > - pr_debug("id %d is out of range (1-4096)\n", > + if (info->id >= VLAN_N_VID) { > + pr_debug("id %d is out of range (1-4095)\n", Hi David, 4095 is reserved, treat it as invalid here? if (info->id >= VLAN_N_VID - 1) { pr_debug("id %d is out of range (1-4094)\n", > info->id); > return -EINVAL; > } > -- > 1.7.11.7 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] net: fix checking boundary of valid vlan id 2012-12-30 6:28 [PATCH] net: fix checking boundary of valid vlan id akong 2012-12-30 6:39 ` Amos Kong @ 2012-12-30 7:40 ` David Miller 2012-12-30 12:25 ` Florian Westphal 2013-01-03 0:39 ` Pablo Neira Ayuso 3 siblings, 0 replies; 9+ messages in thread From: David Miller @ 2012-12-30 7:40 UTC (permalink / raw) To: akong; +Cc: netdev Please send netfilter patches to the netfilter developers at netfilter-devel@vger.kernel.org Thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] net: fix checking boundary of valid vlan id 2012-12-30 6:28 [PATCH] net: fix checking boundary of valid vlan id akong 2012-12-30 6:39 ` Amos Kong 2012-12-30 7:40 ` David Miller @ 2012-12-30 12:25 ` Florian Westphal 2012-12-31 11:36 ` Glen Turner 2013-01-03 0:39 ` Pablo Neira Ayuso 3 siblings, 1 reply; 9+ messages in thread From: Florian Westphal @ 2012-12-30 12:25 UTC (permalink / raw) To: akong; +Cc: netdev, davem akong@redhat.com <akong@redhat.com> wrote: > From: Amos Kong <akong@redhat.com> > > 4096 is not a valid vlan id. True. But why shouldn't users be allowed to check for frames with reserved value? ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] net: fix checking boundary of valid vlan id 2012-12-30 12:25 ` Florian Westphal @ 2012-12-31 11:36 ` Glen Turner 2012-12-31 17:12 ` Benny Amorsen 0 siblings, 1 reply; 9+ messages in thread From: Glen Turner @ 2012-12-31 11:36 UTC (permalink / raw) To: Florian Westphal; +Cc: akong, netdev, davem On 30/12/2012, at 10:55 PM, Florian Westphal wrote: > akong@redhat.com <akong@redhat.com> wrote: >> From: Amos Kong <akong@redhat.com> >> >> 4096 is not a valid vlan id. > > True. > > But why shouldn't users be allowed to check for frames > with reserved value It may be a valid VLAN ID, or it may not. The meaning of FFF is reserved for vendor use, which doesn't preclude a vendor using it as a (non-interoperable) VLAN identifier. Many vendor's products treat 4096 as they do any other VID. It's up to Linux if it cares to treat 4096 as a VLAN or as something else. In any case, Florian's point is good: an ethernet-layer firewall should be able to trigger on any value of VID, and particularly one which should not be seen from some vendor's gear. -glen ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] net: fix checking boundary of valid vlan id 2012-12-31 11:36 ` Glen Turner @ 2012-12-31 17:12 ` Benny Amorsen 2012-12-31 22:30 ` Glen Turner 0 siblings, 1 reply; 9+ messages in thread From: Benny Amorsen @ 2012-12-31 17:12 UTC (permalink / raw) To: Glen Turner; +Cc: Florian Westphal, akong, netdev, davem Glen Turner <gdt@gdt.id.au> writes: > It may be a valid VLAN ID, or it may not. The meaning of FFF is > reserved for vendor use, which doesn't preclude a vendor using it as a > (non-interoperable) VLAN identifier. Many vendor's products treat 4096 > as they do any other VID. I may be missing something vital, but 4096 is 0x1000 not 0xFFF? 4095 is reserved and 0 means "treat as if the packet was untagged". 4096 is impossible, there are only 12 bit and the encoding is AFAIK bog standard binary. /Benny ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] net: fix checking boundary of valid vlan id 2012-12-31 17:12 ` Benny Amorsen @ 2012-12-31 22:30 ` Glen Turner 0 siblings, 0 replies; 9+ messages in thread From: Glen Turner @ 2012-12-31 22:30 UTC (permalink / raw) To: Benny Amorsen; +Cc: Florian Westphal, akong, netdev, davem On 01/01/2013, at 3:42 AM, Benny Amorsen wrote: > Glen Turner <gdt@gdt.id.au> writes: > >> It may be a valid VLAN ID, or it may not. The meaning of FFF is >> reserved for vendor use, which doesn't preclude a vendor using it as a >> (non-interoperable) VLAN identifier. Many vendor's products treat 4096 >> as they do any other VID. > > I may be missing something vital, but 4096 is 0x1000 not 0xFFF? 4095 is > reserved and 0 means "treat as if the packet was untagged". 4096 is > impossible, there are only 12 bit and the encoding is AFAIK bog standard > binary. Yep, I've failed at hex math. 0xfff = 4095 is the maximal VID value, and is the one reserved for vendor use. Which means that the patch checking values 1 to 4095 is correct. My apologies, glen ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] net: fix checking boundary of valid vlan id 2012-12-30 6:28 [PATCH] net: fix checking boundary of valid vlan id akong ` (2 preceding siblings ...) 2012-12-30 12:25 ` Florian Westphal @ 2013-01-03 0:39 ` Pablo Neira Ayuso 2013-01-03 20:06 ` Ben Hutchings 3 siblings, 1 reply; 9+ messages in thread From: Pablo Neira Ayuso @ 2013-01-03 0:39 UTC (permalink / raw) To: akong; +Cc: netdev, davem, Netfilter Development Mailing list On Sun, Dec 30, 2012 at 02:28:51PM +0800, akong@redhat.com wrote: > From: Amos Kong <akong@redhat.com> > > 4096 is not a valid vlan id. > > Signed-off-by: Amos Kong <akong@redhat.com> > --- > net/bridge/netfilter/ebt_vlan.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/bridge/netfilter/ebt_vlan.c b/net/bridge/netfilter/ebt_vlan.c > index eae67bf..b279ec0 100644 > --- a/net/bridge/netfilter/ebt_vlan.c > +++ b/net/bridge/netfilter/ebt_vlan.c > @@ -121,8 +121,8 @@ static int ebt_vlan_mt_check(const struct xt_mtchk_param *par) > * if_vlan.h: VLAN_N_VID 4096. */ > if (GET_BITMASK(EBT_VLAN_ID)) { > if (!!info->id) { /* if id!=0 => check vid range */ > - if (info->id > VLAN_N_VID) { > - pr_debug("id %d is out of range (1-4096)\n", > + if (info->id >= VLAN_N_VID) { > + pr_debug("id %d is out of range (1-4095)\n", Someone may forge frames including reserved VLAN ids. People can use ebtables to drop invalid frames, this is a firewalling utility after all ;-). I'm not taking these, sorry. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] net: fix checking boundary of valid vlan id 2013-01-03 0:39 ` Pablo Neira Ayuso @ 2013-01-03 20:06 ` Ben Hutchings 0 siblings, 0 replies; 9+ messages in thread From: Ben Hutchings @ 2013-01-03 20:06 UTC (permalink / raw) To: Pablo Neira Ayuso Cc: akong, netdev, davem, Netfilter Development Mailing list On Thu, 2013-01-03 at 01:39 +0100, Pablo Neira Ayuso wrote: > On Sun, Dec 30, 2012 at 02:28:51PM +0800, akong@redhat.com wrote: > > From: Amos Kong <akong@redhat.com> > > > > 4096 is not a valid vlan id. > > > > Signed-off-by: Amos Kong <akong@redhat.com> > > --- > > net/bridge/netfilter/ebt_vlan.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/net/bridge/netfilter/ebt_vlan.c b/net/bridge/netfilter/ebt_vlan.c > > index eae67bf..b279ec0 100644 > > --- a/net/bridge/netfilter/ebt_vlan.c > > +++ b/net/bridge/netfilter/ebt_vlan.c > > @@ -121,8 +121,8 @@ static int ebt_vlan_mt_check(const struct xt_mtchk_param *par) > > * if_vlan.h: VLAN_N_VID 4096. */ > > if (GET_BITMASK(EBT_VLAN_ID)) { > > if (!!info->id) { /* if id!=0 => check vid range */ > > - if (info->id > VLAN_N_VID) { > > - pr_debug("id %d is out of range (1-4096)\n", > > + if (info->id >= VLAN_N_VID) { > > + pr_debug("id %d is out of range (1-4095)\n", > > Someone may forge frames including reserved VLAN ids. They may find it difficult to fit the value 4096 into a 12-bit field, though. Ben. > People can use ebtables to drop invalid frames, this is a firewalling > utility after all ;-). I'm not taking these, sorry. -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-01-03 20:06 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-12-30 6:28 [PATCH] net: fix checking boundary of valid vlan id akong 2012-12-30 6:39 ` Amos Kong 2012-12-30 7:40 ` David Miller 2012-12-30 12:25 ` Florian Westphal 2012-12-31 11:36 ` Glen Turner 2012-12-31 17:12 ` Benny Amorsen 2012-12-31 22:30 ` Glen Turner 2013-01-03 0:39 ` Pablo Neira Ayuso 2013-01-03 20:06 ` Ben Hutchings
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).