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 netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
