Am Donnerstag, dem 05.10.2023 um 15:35 -0700 schrieb Kees Cook: > On Thu, Oct 05, 2023 at 04:08:52PM -0400, Siddhesh Poyarekar wrote: > > 2. How would you handle signedness of the size field? The size gets > > converted to sizetype everywhere it is used and overflows/underflows may > > produce interesting results. Do you want to limit the types to unsigned or > > do you want to add a disclaimer in the docs? The former seems like the > > *right* thing to do given that it is a new feature; best to enforce the > > cleaner habit at the outset. > > The Linux kernel has a lot of "int" counters, so the goal is to catch > negative offsets just like too-large offsets at runtime with the sanitizer > and report 0 for __bdos. Refactoring all these to be unsigned is going > to take time since at least some of them use the negative values as > special values unrelated to array indexing. :( > > So, perhaps if unsigned counters are worth enforcing, can this be a > separate warning the kernel can turn off initially? >
I think unsigned counters are much more problematic than signed ones because wraparound errors are more difficult to find. With unsigned you could potentially diagnose wraparound, but only if we add -fsanitize=unsigned-overflow *and* add mechanism to mark intentional wraparound *and* everybody adds this annotation after carefully screening their code *and* rewriting all operations such as (counter - 3) + 5 where the wraparound in the intermediate expression is harmless. For this reason, I do not think we should ever enforce some rule that the counter has to be unsigned. What we could do, is detect *storing* negative values into the counter at run-time using UBSan. (but if negative values are used for special cases, one also should be able to turn this off). Martin
