[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.LSU.2.01.1008061044270.8895@obet.zrqbmnf.qr>
Date: Fri, 6 Aug 2010 10:52:23 +0200 (CEST)
From: Jan Engelhardt <jengelh@...ozas.de>
To: Luciano Coelho <luciano.coelho@...ia.com>
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 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.
>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".
>> 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.
--
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
Powered by blists - more mailing lists