On Tue, Feb 10, 2015 at 6:26 AM, Tobias Stöckmann <tobias@xxxxxxxxxxxxxx> wrote:
> Hi,
>
>> On February 10, 2015 at 3:56 AM Lucas De Marchi <lucas.de.marchi@xxxxxxxxx>
>> wrote:
>> > - if (min_size > elf->size) {
>> > + if (ULLONG_MAX - *offset < *size || min_size > elf->size) {
>
>> - min_size = *offset + *size;
>> - if (min_size > elf->size) {
>> + if (__builtin_uaddl_overflow(*offset, *size, &min_size)
>> + || min_size > elf->size) {
>
> If at all, it would have to be __builtin_uaddll_overflow due to uint64_t even on
> 32 bit systems. But even then the prototype looks a bit off to me:
sizeof(uint64_t) == sizeof(unsigned long) both on 32 and 64 bits... no
need to use long long.
> from https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html
> __builtin_uaddll_overflow (unsigned long long int a, unsigned long long int b,
> unsigned long int *res)
>
> I hope it's a typo and they meant "unsigned long long int *res", otherwise that
> built-in function by itself is prone to truncation.
yep, it seems to be a typo
> If it works, i.e. the poc.ko module doesn't trigger a segmentation fault, I am
> fine with that solution. When it's in the tree, I will create fixes for the
> other occurrences in that style as well.
are you going to submit the patch or you're waiting on me?
> Please keep in mind that these libkmod issues are not limited to just modinfo.
> The tool modinfo is just the easiest way to trigger them, because it doesn't
> need any advanced permissions.
yep, it'd be good to have the other fixes, even more than this one. If
this is pointed out by any static analysis tool, feel free to add that
in the commit message.
thanks
--
Lucas De Marchi
--
To unsubscribe from this list: send the line "unsubscribe linux-modules" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
