On Tue, 8 Nov 2011, Witold Baryluk wrote:
> On 11-08 08:56, Jozsef Kadlecsik wrote:
> >
> > On Mon, 7 Nov 2011, Witold Baryluk wrote:
> >
> > > in file et/netfilter/ipset/ip_set_hash_ipportnet.c
> > > in functionhash_ipportnet4_uadt(...)
> > > variable u32 ip_to, can be used uninitialized
> > > in line 283
> > > for (; !before(ip_to, ip); ip++) {
> > >
> > > because, if both tb[IPSET_ATTR_IP_TO] and tb[IPSET_ATTR_CIDR]
> > > are false, then there is no initialization of ip_to.
> > >
> > > CC [M] net/netfilter/ipset/ip_set_hash_ipportnet.o
> > > net/netfilter/ipset/ip_set_hash_ipportnet.c: In function ?hash_ipportnet4_uadt?:
> > > include/net/tcp.h:266:28: warning: ?ip_to? may be used uninitialized in this function [-Wuninitialized]
> > > net/netfilter/ipset/ip_set_hash_ipportnet.c:187:10: note: ?ip_to? was declared here
> >
> > Which ipset version you are looking at? What gcc version are you running?
>
> sorry to not mention it.
>
> I'm compiling on 32-bit Intel x86, using gcc 4.6.2-4 with -march=pentium-m.
> I'm using current/today mainline Linus' tree.
> Last significant ipset change in this tree is from Jul 21, 2011.
Oops, that patch was not submitted to kernel inclusion. The merge window
has just opened up, I'm going to send it in the next day.
> > ip_to is explicitly initialized to zero in line 187 in the current ipset
> > version.
> >
> > > I think this should be fixed be properly initializing / using ip_to,
> > > or by telling compiler that some of paths will end in initialization.
> > >
> > > Also why on earth a before(a,b) function is used
> > > to compare IP addresses? Shouldn't before(seq1,seq2) function
> > > be used only to compare TCP sequnce numbers?!
> >
> > That's a quick way to avoid wraparound in the for loop.
>
> It's somehow ugly. I agree that same method should be used,
> but this just leaks a abstraction and who knows in the future
> if this will not make problems.
The net/tcp.h header file says
/*
* The next routines deal with comparing 32 bit unsigned ints
* and worry about wraparound (automatic with unsigned arithmetic).
*/
static inline int before(__u32 seq1, __u32 seq2)
...
Neither the comment, nor the function names imply that the routines should
be used only for TCP sequence numbers.
Maybe the routines should be relocated into a more generic header file.
Best regards,
Jozsef
-
E-mail : kadlec@xxxxxxxxxxxxxxxxx, kadlec@xxxxxxxxxxxx
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
