* [patch] netfilter: potential NULL dereference in get_inner_hdr()
@ 2012-05-12 11:00 Dan Carpenter
2012-05-14 7:36 ` Hans Schillstrom
2012-05-14 8:24 ` Pablo Neira Ayuso
0 siblings, 2 replies; 8+ messages in thread
From: Dan Carpenter @ 2012-05-12 11:00 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Patrick McHardy, David S. Miller, netfilter-devel, netfilter,
coreteam, netdev, kernel-janitors
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.
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? */
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [patch] netfilter: potential NULL dereference in get_inner_hdr()
2012-05-12 11:00 [patch] netfilter: potential NULL dereference in get_inner_hdr() Dan Carpenter
@ 2012-05-14 7:36 ` Hans Schillstrom
2012-05-14 7:38 ` David Miller
2012-05-14 8:39 ` Dan Carpenter
2012-05-14 8:24 ` Pablo Neira Ayuso
1 sibling, 2 replies; 8+ messages in thread
From: Hans Schillstrom @ 2012-05-14 7:36 UTC (permalink / raw)
To: Dan Carpenter
Cc: Pablo Neira Ayuso, Patrick McHardy, David S. Miller,
netfilter-devel@vger.kernel.org, netfilter@vger.kernel.org,
coreteam@netfilter.org, netdev@vger.kernel.org,
kernel-janitors@vger.kernel.org
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
>
--
Regards
Hans Schillstrom <hans.schillstrom@ericsson.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [patch] netfilter: potential NULL dereference in get_inner_hdr()
2012-05-14 7:36 ` Hans Schillstrom
@ 2012-05-14 7:38 ` David Miller
2012-05-14 7:53 ` Eric Dumazet
2012-05-14 8:39 ` Dan Carpenter
1 sibling, 1 reply; 8+ messages in thread
From: David Miller @ 2012-05-14 7:38 UTC (permalink / raw)
To: hans.schillstrom
Cc: dan.carpenter, pablo, kaber, netfilter-devel, netfilter, coreteam,
netdev, kernel-janitors
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [patch] netfilter: potential NULL dereference in get_inner_hdr()
2012-05-14 7:38 ` David Miller
@ 2012-05-14 7:53 ` Eric Dumazet
2012-05-14 8:00 ` Hans Schillstrom
0 siblings, 1 reply; 8+ messages in thread
From: Eric Dumazet @ 2012-05-14 7:53 UTC (permalink / raw)
To: David Miller
Cc: hans.schillstrom, dan.carpenter, pablo, kaber, netfilter-devel,
netfilter, coreteam, netdev, kernel-janitors
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [patch] netfilter: potential NULL dereference in get_inner_hdr()
2012-05-14 7:53 ` Eric Dumazet
@ 2012-05-14 8:00 ` Hans Schillstrom
2012-05-14 8:22 ` Pablo Neira Ayuso
0 siblings, 1 reply; 8+ messages in thread
From: Hans Schillstrom @ 2012-05-14 8:00 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, dan.carpenter@oracle.com, pablo@netfilter.org,
kaber@trash.net, netfilter-devel@vger.kernel.org,
netfilter@vger.kernel.org, coreteam@netfilter.org,
netdev@vger.kernel.org, kernel-janitors@vger.kernel.org
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
--
Regards
Hans Schillstrom <hans.schillstrom@ericsson.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [patch] netfilter: potential NULL dereference in get_inner_hdr()
2012-05-14 8:00 ` Hans Schillstrom
@ 2012-05-14 8:22 ` Pablo Neira Ayuso
0 siblings, 0 replies; 8+ messages in thread
From: Pablo Neira Ayuso @ 2012-05-14 8:22 UTC (permalink / raw)
To: Hans Schillstrom
Cc: Eric Dumazet, David Miller, dan.carpenter@oracle.com,
kaber@trash.net, netfilter-devel@vger.kernel.org,
netfilter@vger.kernel.org, coreteam@netfilter.org,
netdev@vger.kernel.org, kernel-janitors@vger.kernel.org
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [patch] netfilter: potential NULL dereference in get_inner_hdr()
2012-05-14 7:36 ` Hans Schillstrom
2012-05-14 7:38 ` David Miller
@ 2012-05-14 8:39 ` Dan Carpenter
1 sibling, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2012-05-14 8:39 UTC (permalink / raw)
To: Hans Schillstrom
Cc: Pablo Neira Ayuso, Patrick McHardy, David S. Miller,
netfilter-devel@vger.kernel.org, netfilter@vger.kernel.org,
coreteam@netfilter.org, netdev@vger.kernel.org,
kernel-janitors@vger.kernel.org
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [patch] netfilter: potential NULL dereference in get_inner_hdr()
2012-05-12 11:00 [patch] netfilter: potential NULL dereference in get_inner_hdr() Dan Carpenter
2012-05-14 7:36 ` Hans Schillstrom
@ 2012-05-14 8:24 ` Pablo Neira Ayuso
1 sibling, 0 replies; 8+ messages in thread
From: Pablo Neira Ayuso @ 2012-05-14 8:24 UTC (permalink / raw)
To: Dan Carpenter
Cc: Patrick McHardy, David S. Miller, netfilter-devel, netfilter,
coreteam, netdev, kernel-janitors
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-05-14 8:35 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-12 11:00 [patch] netfilter: potential NULL dereference in get_inner_hdr() Dan Carpenter
2012-05-14 7:36 ` Hans Schillstrom
2012-05-14 7:38 ` David Miller
2012-05-14 7:53 ` Eric Dumazet
2012-05-14 8:00 ` Hans Schillstrom
2012-05-14 8:22 ` Pablo Neira Ayuso
2012-05-14 8:39 ` Dan Carpenter
2012-05-14 8:24 ` Pablo Neira Ayuso
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).