| Message ID | alpine.OSX.2.00.1212111432430.78903@animac.local |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, 2012-12-11 at 14:36 -0800, Ani Sinha wrote: > > > > It is possible to test for the presence of support of the new vlan bpf > > extensions by attempting to load a filter that uses them. As only valid > > filters can be loaded, old kernels that do not support filtering of vlan > > tags will fail to load the a test filter with uses them. > > Unfortunately I do not see this. The sk_chk_filter() does not have a > default in the case statement and the check will not detect an unknown > instruction. It will fail when the filter is run and as far as I can see, > the packet will be dropped. Something like this might help? > > diff --git a/net/core/filter.c b/net/core/filter.c > index c23543c..96338aa 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -548,6 +548,8 @@ int sk_chk_filter(struct sock_filter *filter, unsigned int flen) > return -EINVAL; > /* Some instructions need special checks */ > switch (code) { > + /* for unknown instruction, return EINVAL */ > + default : return -EINVAL; > case BPF_S_ALU_DIV_K: > /* check for division by zero */ > if (ftest->k == 0) This patch is wrong. Check lines 546, 547, 548 where we do the check for unknown instructions code = codes[code]; if (!code) return -EINVAL; If you want to test ANCILLARY possible values, its already too late, as old kernels wont use any patch anyway. -- 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 Tue, 11 Dec 2012 14:36:33 -0800 (PST) Ani Sinha <ani@aristanetworks.com> wrote: > > > > It is possible to test for the presence of support of the new vlan bpf > > extensions by attempting to load a filter that uses them. As only valid > > filters can be loaded, old kernels that do not support filtering of vlan > > tags will fail to load the a test filter with uses them. > > Unfortunately I do not see this. The sk_chk_filter() does not have a > default in the case statement and the check will not detect an unknown > instruction. It will fail when the filter is run and as far as I can see, > the packet will be dropped. Something like this might help? > > diff --git a/net/core/filter.c b/net/core/filter.c > index c23543c..96338aa 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -548,6 +548,8 @@ int sk_chk_filter(struct sock_filter *filter, unsigned int flen) > return -EINVAL; > /* Some instructions need special checks */ > switch (code) { > + /* for unknown instruction, return EINVAL */ > + default : return -EINVAL; > case BPF_S_ALU_DIV_K: > /* check for division by zero */ > if (ftest->k == 0) Did you test this? I think it will blow up for some existing instructions like BPF_S_ALU_XOR_X or any of the other non-special instructions. Also it is not formatted correctly for the kernel programming style. ERROR: space prohibited before that ':' (ctx:WxW) #86: FILE: net/core/filter.c:552: + default : return -EINVAL; ^ -- 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 Tue, Dec 11, 2012 at 3:04 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > On Tue, 2012-12-11 at 14:36 -0800, Ani Sinha wrote: >> > >> > It is possible to test for the presence of support of the new vlan bpf >> > extensions by attempting to load a filter that uses them. As only valid >> > filters can be loaded, old kernels that do not support filtering of vlan >> > tags will fail to load the a test filter with uses them. >> >> Unfortunately I do not see this. The sk_chk_filter() does not have a >> default in the case statement and the check will not detect an unknown >> instruction. It will fail when the filter is run and as far as I can see, >> the packet will be dropped. Something like this might help? >> >> diff --git a/net/core/filter.c b/net/core/filter.c >> index c23543c..96338aa 100644 >> --- a/net/core/filter.c >> +++ b/net/core/filter.c >> @@ -548,6 +548,8 @@ int sk_chk_filter(struct sock_filter *filter, unsigned int flen) >> return -EINVAL; >> /* Some instructions need special checks */ >> switch (code) { >> + /* for unknown instruction, return EINVAL */ >> + default : return -EINVAL; >> case BPF_S_ALU_DIV_K: >> /* check for division by zero */ >> if (ftest->k == 0) > > This patch is wrong. yes I generated this patch wrong. > > Check lines 546, 547, 548 where we do the check for unknown instructions > > code = codes[code]; > if (!code) > return -EINVAL; yepph it's OK here. > > If you want to test ANCILLARY possible values, its already too late, as > old kernels wont use any patch anyway. yepph, I was looking at possible ancilliary values. Basically this case statement : #define ANCILLARY(CODE) case SKF_AD_OFF + SKF_AD_##CODE: \ code = BPF_S_ANC_##CODE; \ break switch (ftest->k) { ANCILLARY(PROTOCOL); ANCILLARY(PKTTYPE); ANCILLARY(IFINDEX); ANCILLARY(NLATTR); ANCILLARY(NLATTR_NEST); ANCILLARY(MARK); ANCILLARY(QUEUE); ANCILLARY(HATYPE); ANCILLARY(RXHASH); ANCILLARY(CPU); ANCILLARY(ALU_XOR_X); ANCILLARY(VLAN_TAG); ANCILLARY(VLAN_TAG_PRESENT); } -- 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 Tue, Dec 11, 2012 at 3:04 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > On Tue, 2012-12-11 at 14:36 -0800, Ani Sinha wrote: >> > >> > It is possible to test for the presence of support of the new vlan bpf > If you want to test ANCILLARY possible values, its already too late, as > old kernels wont use any patch anyway. > So basically this means that if we generate a filter with these special negative offset values and expect that the kernel will complain if it does not recognize the newer values then we would be wrong. And you are right. Old kernels never knew about them and the code wasn't written in a way to return EINVAL if it didn't recognize a special negative anciliary offset 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
diff --git a/net/core/filter.c b/net/core/filter.c index c23543c..96338aa 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -548,6 +548,8 @@ int sk_chk_filter(struct sock_filter *filter, unsigned int flen) return -EINVAL; /* Some instructions need special checks */ switch (code) { + /* for unknown instruction, return EINVAL */ + default : return -EINVAL; case BPF_S_ALU_DIV_K: /* check for division by zero */ if (ftest->k == 0)