| Message ID | 20120512110003.GB19472@elgon.mountain |
|---|---|
| State | Accepted |
| Headers | show |
On Saturday 12 May 2012 13:00:03 Dan Carpenter wrote: > There is a typo in the error checking and "&&" was used instead of "||". > If skb_header_pointer() returns NULL then it leads to a NULL > dereference. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Ack-by: Hans Schillstrom <hans.schillstrom@ericsson.com> > --- > Btw, this is new code and Sparse complains about endian bugs. Can you give me some hints here, arch , compiler version etc. I guess it was input to hmark_addr_mask() that complains ? > > diff --git a/net/netfilter/xt_HMARK.c b/net/netfilter/xt_HMARK.c > index 32fbd73..5817d03 100644 > --- a/net/netfilter/xt_HMARK.c > +++ b/net/netfilter/xt_HMARK.c > @@ -223,7 +223,7 @@ static int get_inner_hdr(const struct sk_buff *skb, int iphsz, int *nhoff) > > /* Not enough header? */ > icmph = skb_header_pointer(skb, *nhoff + iphsz, sizeof(_ih), &_ih); > - if (icmph == NULL && icmph->type > NR_ICMP_TYPES) > + if (icmph == NULL || icmph->type > NR_ICMP_TYPES) > return 0; > > /* Error message? */ > -- > 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 >
From: Hans Schillstrom <hans.schillstrom@ericsson.com> Date: Mon, 14 May 2012 09:36:55 +0200 > On Saturday 12 May 2012 13:00:03 Dan Carpenter wrote: >> There is a typo in the error checking and "&&" was used instead of "||". >> If skb_header_pointer() returns NULL then it leads to a NULL >> dereference. >> >> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > Ack-by: Hans Schillstrom <hans.schillstrom@ericsson.com> > >> --- >> Btw, this is new code and Sparse complains about endian bugs. > > Can you give me some hints here, arch , compiler version etc. > I guess it was input to hmark_addr_mask() that complains ? He said what he's using, "sparse", the semantic parser, which is largely arch agnostic. I guarantee you will see the warnings if you run it on your system on this code. -- 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
On Mon, 2012-05-14 at 03:38 -0400, David Miller wrote: > He said what he's using, "sparse", the semantic parser, which > is largely arch agnostic. > > I guarantee you will see the warnings if you run it on your > system on this code. Some more info : http://lwn.net/Articles/205624/ make C=2 CF=-D__CHECK_ENDIAN__ net/netfilter/xt_HMARK.o CHECK net/netfilter/xt_HMARK.c net/netfilter/xt_HMARK.c:87:21: warning: incorrect type in assignment (different base types) net/netfilter/xt_HMARK.c:87:21: expected unsigned short [unsigned] [usertype] src net/netfilter/xt_HMARK.c:87:21: got restricted __be16 [usertype] all net/netfilter/xt_HMARK.c:88:21: warning: incorrect type in assignment (different base types) net/netfilter/xt_HMARK.c:88:21: expected unsigned short [unsigned] [usertype] dst net/netfilter/xt_HMARK.c:88:21: got restricted __be16 [usertype] all net/netfilter/xt_HMARK.c:170:27: warning: incorrect type in argument 2 (different signedness) net/netfilter/xt_HMARK.c:170:27: expected int *offset net/netfilter/xt_HMARK.c:170:27: got unsigned int *<noident> net/netfilter/xt_HMARK.c:181:28: warning: incorrect type in argument 1 (different base types) net/netfilter/xt_HMARK.c:181:28: expected unsigned int const [usertype] *addr32 net/netfilter/xt_HMARK.c:181:28: got restricted __be32 *<noident> net/netfilter/xt_HMARK.c:182:28: warning: incorrect type in argument 1 (different base types) net/netfilter/xt_HMARK.c:182:28: expected unsigned int const [usertype] *addr32 net/netfilter/xt_HMARK.c:182:28: got restricted __be32 *<noident> net/netfilter/xt_HMARK.c:261:9: warning: invalid assignment: &= net/netfilter/xt_HMARK.c:261:9: left side has type unsigned int net/netfilter/xt_HMARK.c:261:9: right side has type restricted __be32 net/netfilter/xt_HMARK.c:262:9: warning: invalid assignment: &= net/netfilter/xt_HMARK.c:262:9: left side has type unsigned int net/netfilter/xt_HMARK.c:262:9: right side has type restricted __be32 -- 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
On Monday 14 May 2012 09:53:15 Eric Dumazet wrote: > On Mon, 2012-05-14 at 03:38 -0400, David Miller wrote: > > > He said what he's using, "sparse", the semantic parser, which > > is largely arch agnostic. > > > > I guarantee you will see the warnings if you run it on your > > system on this code. > > Some more info : > > http://lwn.net/Articles/205624/ > > make C=2 CF=-D__CHECK_ENDIAN__ net/netfilter/xt_HMARK.o > CHECK net/netfilter/xt_HMARK.c > net/netfilter/xt_HMARK.c:87:21: warning: incorrect type in assignment (different base types) > net/netfilter/xt_HMARK.c:87:21: expected unsigned short [unsigned] [usertype] src > net/netfilter/xt_HMARK.c:87:21: got restricted __be16 [usertype] all > net/netfilter/xt_HMARK.c:88:21: warning: incorrect type in assignment (different base types) > net/netfilter/xt_HMARK.c:88:21: expected unsigned short [unsigned] [usertype] dst > net/netfilter/xt_HMARK.c:88:21: got restricted __be16 [usertype] all > net/netfilter/xt_HMARK.c:170:27: warning: incorrect type in argument 2 (different signedness) > net/netfilter/xt_HMARK.c:170:27: expected int *offset > net/netfilter/xt_HMARK.c:170:27: got unsigned int *<noident> > net/netfilter/xt_HMARK.c:181:28: warning: incorrect type in argument 1 (different base types) > net/netfilter/xt_HMARK.c:181:28: expected unsigned int const [usertype] *addr32 > net/netfilter/xt_HMARK.c:181:28: got restricted __be32 *<noident> > net/netfilter/xt_HMARK.c:182:28: warning: incorrect type in argument 1 (different base types) > net/netfilter/xt_HMARK.c:182:28: expected unsigned int const [usertype] *addr32 > net/netfilter/xt_HMARK.c:182:28: got restricted __be32 *<noident> > net/netfilter/xt_HMARK.c:261:9: warning: invalid assignment: &= > net/netfilter/xt_HMARK.c:261:9: left side has type unsigned int > net/netfilter/xt_HMARK.c:261:9: right side has type restricted __be32 > net/netfilter/xt_HMARK.c:262:9: warning: invalid assignment: &= > net/netfilter/xt_HMARK.c:262:9: left side has type unsigned int > net/netfilter/xt_HMARK.c:262:9: right side has type restricted __be32 > Thanks I'll try to fix the warnings
On Mon, May 14, 2012 at 10:00:00AM +0200, Hans Schillstrom wrote: > > > > http://lwn.net/Articles/205624/ > > > > make C=2 CF=-D__CHECK_ENDIAN__ net/netfilter/xt_HMARK.o > > CHECK net/netfilter/xt_HMARK.c > > net/netfilter/xt_HMARK.c:87:21: warning: incorrect type in assignment (different base types) > > net/netfilter/xt_HMARK.c:87:21: expected unsigned short [unsigned] [usertype] src > > net/netfilter/xt_HMARK.c:87:21: got restricted __be16 [usertype] all [...] > > Thanks > I'll try to fix the warnings Ok, wait for that patch, thanks. -- 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
On Sat, May 12, 2012 at 02:00:03PM +0300, Dan Carpenter wrote: > There is a typo in the error checking and "&&" was used instead of "||". > If skb_header_pointer() returns NULL then it leads to a NULL > dereference. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Applied, thanks Dan. -- 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
On Mon, May 14, 2012 at 09:36:55AM +0200, Hans Schillstrom wrote: > On Saturday 12 May 2012 13:00:03 Dan Carpenter wrote: > > There is a typo in the error checking and "&&" was used instead of "||". > > If skb_header_pointer() returns NULL then it leads to a NULL > > dereference. > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > Ack-by: Hans Schillstrom <hans.schillstrom@ericsson.com> > > > --- > > Btw, this is new code and Sparse complains about endian bugs. > > Can you give me some hints here, arch , compiler version etc. > I guess it was input to hmark_addr_mask() that complains ? > Yes. That was one of the warnings. http://lwn.net/Articles/205624/ net/netfilter/xt_HMARK.c:87:35: warning: incorrect type in assignment (different base types) net/netfilter/xt_HMARK.c:87:35: expected unsigned short [unsigned] [usertype] src net/netfilter/xt_HMARK.c:87:35: got restricted __be16 [usertype] all net/netfilter/xt_HMARK.c:88:35: warning: incorrect type in assignment (different base types) net/netfilter/xt_HMARK.c:88:35: expected unsigned short [unsigned] [usertype] dst net/netfilter/xt_HMARK.c:88:35: got restricted __be16 [usertype] all net/netfilter/xt_HMARK.c:181:35: warning: incorrect type in argument 1 (different base types) net/netfilter/xt_HMARK.c:181:35: expected unsigned int const [usertype] *addr32 net/netfilter/xt_HMARK.c:181:35: got restricted __be32 *<noident> net/netfilter/xt_HMARK.c:182:35: warning: incorrect type in argument 1 (different base types) net/netfilter/xt_HMARK.c:182:35: expected unsigned int const [usertype] *addr32 net/netfilter/xt_HMARK.c:182:35: got restricted __be32 *<noident> net/netfilter/xt_HMARK.c:261:16: warning: invalid assignment: &= net/netfilter/xt_HMARK.c:261:16: left side has type unsigned int net/netfilter/xt_HMARK.c:261:16: right side has type restricted __be32 net/netfilter/xt_HMARK.c:262:16: warning: invalid assignment: &= net/netfilter/xt_HMARK.c:262:16: left side has type unsigned int net/netfilter/xt_HMARK.c:262:16: right side has type restricted __be32 regards, dan carpenter -- 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
diff --git a/net/netfilter/xt_HMARK.c b/net/netfilter/xt_HMARK.c index 32fbd73..5817d03 100644 --- a/net/netfilter/xt_HMARK.c +++ b/net/netfilter/xt_HMARK.c @@ -223,7 +223,7 @@ static int get_inner_hdr(const struct sk_buff *skb, int iphsz, int *nhoff) /* Not enough header? */ icmph = skb_header_pointer(skb, *nhoff + iphsz, sizeof(_ih), &_ih); - if (icmph == NULL && icmph->type > NR_ICMP_TYPES) + if (icmph == NULL || icmph->type > NR_ICMP_TYPES) return 0; /* Error message? */
There is a typo in the error checking and "&&" was used instead of "||". If skb_header_pointer() returns NULL then it leads to a NULL dereference. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- Btw, this is new code and Sparse complains about endian bugs. -- 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