On Fri, 14 Sep 2007, Linus Torvalds wrote: > > ... and we also make sure that we don't overflow when doing the matrix > size calculation. Side note: by "make sure", I don't really mean a total guarantee. We could be even more careful here. In particular: - we later do end up allocating the matrix with sizeof(*mx) * num_create * num_src and I didn't actually fix the overflow that is possible due to the "sizeof(*mx)" multiplication. - even after we've checked that not *both* of the source and destination counts are larger than the rename_limit, we could still overflow the multiplication in just the limit check. .. but with the rename_limit being set to 100, in practice neither of these are really even close to realistic (ie you'd need to have less than 100 new files, and deleted over twenty million files to overflow, or vice versa). So with a rename_limit of 100, it's all good (I'm pretty sure you'd have *other* issues long before you'd hit the integer overflows on renames ;) But if somebody sets the rename_limit to something bigger, it gets increasingly easier to screw it up. If somebody wants to be *really* careful, they'd need to do something like unsigned long max; /* This isn't going to overflow, since we limited 'rename_limit' */ max = rename_limit * rename_limit; /* * But we should also check that multiplying by "sizeof(*mx)" * won't make it overlof either.. */ while ((sizeof(*mx) * max) / sizeof(*mx) != max) max >>= 1; /* * And then avoid multiplying "rename_dst_nr" and "rename_src_nr" * together by turning it into a division instead */ if (max / rename_dst_nr > rename_src_nr) goto cleanup; but the patch I sent out was the "obvious" first one that at least avoided the overflow for the triggerable case that Dmitry had, and as per above likely in all reasonable cases... Linus - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
