| Message ID | 1356848931-22193-1-git-send-email-akong@redhat.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Please send netfilter patches to the netfilter developers at netfilter-devel@vger.kernel.org Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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; }