netdev - Re: [PATCH 2/2] netfilter: xt_condition: change the value from boolean to u32

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Fri, 06 Aug 2010 13:34:32 +0300
From:	Luciano Coelho <luciano.coelho@...ia.com>
To:	ext Jan Engelhardt <jengelh@...ozas.de>
Cc:	"netfilter-devel@...r.kernel.org" <netfilter-devel@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"kaber@...sh.net" <kaber@...sh.net>,
	"sameo@...ux.intel.com" <sameo@...ux.intel.com>
Subject: Re: [PATCH 2/2] netfilter: xt_condition: change the value from
 boolean to u32

On Fri, 2010-08-06 at 10:52 +0200, ext Jan Engelhardt wrote:
> On Friday 2010-08-06 10:00, Luciano Coelho wrote:
> >> >+	buf[length - 1] = '\0';
> >> >+
> >> >+	if (strict_strtoull(buf, 0, &value) != 0)
> >> >+		return -EINVAL;
> >> >+
> >> >+	if (value > (u32) value)
> >> >+		return -EINVAL;
> >> 
> >> Is it possible to use just strict_strtoul?
> >
> >Not easily.  I found that there is a bug in strtoul (and strtoull for
> >that matter) that causes the long to overflow if there are valid digits
> >after the maximum possible digits for the base.  For example if you try
> >to strtoul 0xfffffffff (with 9 f's) the strtoul will overflow and come
> >up with a bogus result.
> 
> I see. Strange that no one has adressed this yet - I mean, writing
> a just-too-large value into a procfs/sysfs file and thus effectively
> causing a bogus value to be actually written isn't quite so thrilling
> as things go haywire.

Yes, I was really surprised to see this happening when I was testing the
limits.  And I was even more surprised when I checked the strtoull code
and saw that it is broken.


> >I can't easily truncate the string to avoid
> >this problem, because with decimal or octal, the same valid value would
> >take more spaces.  I could do some magic here, checking whether it's a
> >hex, dec or oct and truncate appropriately, but that would be very ugly.
> >
> >So the simplest way I came up with was to use strtoull and return
> >-EINVAL if the value exceeds 32 bits. ;)
> 
> If I read strtoul(3) right, ERANGE is used for "out of range".

Yes, libc's strtoul returns ERANGE in that case.  strict_strtoul() in
the kernel code doesn't.  I'll change my code to return -ERANGE here
too, for consistency.


> >> Since the condition value (cdmark) was thought of an nfmark-style thing, 
> >> would it perhaps make sense to model it after it
> >> 
> >> 	return (var->value & ~info->mask) ^ info->value;
> >> 
> >> Other opinions?
> >
> >I think it's nicer to have it as a normal equals here for now and then
> >extend the match with more operations.  We can later add, for example,
> >an --and option to the condition match in order to do other kinds of
> >binary operations.  It would be more flexible this way because we could
> >use several different types of comparisons, wouldn't it? And in the
> >target we could have several different types of operations.
> 
> Indeed.


-- 
Cheers,
Luca.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html