On Mon, Sep 9, 2024 at 5:57 PM Jason A. Donenfeld Jason@zx2c4.com wrote:
On Mon, Sep 09, 2024 at 09:53:43AM +0200, Uros Bizjak wrote:
a) Substitutes the inclusion of <linux/random.h> with the inclusion of <linux/prandom.h> where needed (patches 1 - 17).
b) Removes legacy inclusion of <linux/prandom.h> from <linux/random.h> (patch 18).
c) Includes <linux/percpu.h> in <linux/prandom.h> (patch 19).
Thanks for doing this. That seems like a fine initiative to me. (I'm also curious about the future percpu changes you've got planned.)
As explained in the cover letter, recent GCCs are able to track address space of variables in percpu address space from the declaration to its usage site. There are certain rules regarding casts of variables and their pointers (when this named address space is not considered a subspace of the generic address space), so it is possible to create much more effective checks for cast-from-as type casts than what sparse can achieve.
Besides GCC, clang can define various named address space via address_space attribute:
--cut here-- #define __as(N) __attribute__((address_space(N)))
void *foo(void __as(1) *x) { return x; } // error
void *bar(void __as(1) *x) { return (void *)x; } // fine --cut here--
When compiling this, the compiler returns:
clang-as.c:3:37: error: returning '__as(1) void *' from a function with result type 'void *' changes address space of pointer
Although clang currently errors out when __seg_gs and __seg_fs named address space designators are used, we can explore its named address spaces functionality to implement percpu checks for all targets. The percpu address space checks patchset, referred to in the cover letter, also supports this functionality when per_cpu_qual is defined to __attribute__((address_space(N))).
Perhaps we can use different address spaces to also handle __user, __iomem and __rcu address spaces. This way the compiler will be able to handle address space checks instead of sparse.
Tree-wise, were you expecting me to take this through random.git? And if so, what timeframe did you have in mind? For 6.12 next week (can you poke folks for acks in time?), or punt it for 6.13? Or did you have a different tree in mind for treewide changes (in which case, I'll send you an ack for the [p]random.h changes).
I think that the best approach is to target this patchset for linux 6.13 via random.git tree. I will prepare a v3 after 6.12rc1, so when committed to random.git, the patchset will be able to spend some time in linux-next. This way, there will be plenty of time for CI robots to do additional checks also for some less popular targets (although individual patches are dead simple, removing these kinds of "legacy" includes can be tricky), and I will also be able to collect Acked-by:s in the meantime.
While the patchset is an improvement by itself, its inclusion is not time sensitive. The follow up percpu named address checking functionality requires a very recent feature (__typeof_unqual__ keyword), which is only supported in recent compilers (gcc-14 and clang-20). Besides compiler support, sparse doesn't know about __typeof_unqual__, resulting in broken type tracing and hundreds of sparse errors with C=1 due to unknown keyword.
So, I think we are not in a hurry and can take the slow and safe path.
Thanks and best regards, Uros.