lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 23 Aug 2022 22:56:21 -0700 From: Linus Torvalds <torvalds@...ux-foundation.org> To: Al Viro <viro@...iv.linux.org.uk> Cc: Bart Van Assche <bvanassche@....org>, Rasmus Villemoes <linux@...musvillemoes.dk>, Steven Rostedt <rostedt@...dmis.org>, linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...nel.org>, Andrew Morton <akpm@...ux-foundation.org>, Christoph Hellwig <hch@....de>, Luc Van Oostenryck <luc.vanoostenryck@...il.com>, Jens Axboe <axboe@...nel.dk> Subject: Re: [for-linus][PATCH 01/10] tracing: Suppress sparse warnings triggered by is_signed_type() On Tue, Aug 23, 2022 at 8:10 PM Al Viro <viro@...iv.linux.org.uk> wrote: > > Any operations like ordered comparisons would trigger unrestrict() on > these suckers, which would warn and convert to underlying type. > > Your addition of "ordered comparison with 0 or -1" evades unrestrict(). No. Look. Try this modification to that test, and use './test-linearize' to see what sparse turns it into without my patch to keep the signedness. static long test(void) { return (le32) 0xffffffff; } yes, yes, it warns (twice, actually), but it also then generates ret.64 $-1 for that return. Why? Because it thinks that 'le32' is a signed 32-bit thing due to the clearing of the MOD_UNSIGNED bit, so when it casts it to 'long' it will sign-extend it. So the sign confusion exists and is visible regardless of the added ordered comparison. Now, we normally don't *notice* any of this, because we obviously don't rely on sparse generating any code. And we _do_ cast those bitwise things in many places, although we use "__force" to show that it's intentional. Including, very much, those kinds of widening casts where the signedness matters. See for example very much the csum code: __wsum csum_partial(const void *buff, int len, __wsum wsum) { unsigned int sum = (__force unsigned int)wsum; which is *exactly* that kind of code where it's fundamentally important that 'wsum' is an unsigned type, and casting it to 'unsigned int' does not sign-extend it. So no. This has absolutely nothing to do with the new ordered comparisons. Those bitwise types have always been integers, just with special rules for warning about mis-using them. And the sign handling has always been wrong. It just so happens that me using 'test-linearize' to double-check what sparse does for that signedness check *uncovered* that pre-existing bug. It was not introduced by the new code, and the ordered comparisons are not AT ALL different from the equality comparisons, except for the fact that they actually care about the signedness being right. Linus
Powered by blists - more mailing lists