On Sun, Mar 08, 2020 at 08:34:29AM +0100, Maxime Villard wrote: > Le 08/03/2020 ? 01:31, Andrew Doran a ?crit?: > > Module Name: src > > Committed By: ad > > Date: Sun Mar 8 00:31:19 UTC 2020 > > > > Modified Files: > > src/sys/kern: subr_kmem.c > > > > Log Message: > > KMEM_SIZE: append the size_t to the allocated buffer, rather than > > prepending, so it doesn't screw up the alignment of the buffer. > > > > Reported-by: syzbot+c024c50570cccac51...@syzkaller.appspotmail.com > > > > > > To generate a diff of this commit: > > cvs rdiff -u -r1.78 -r1.79 src/sys/kern/subr_kmem.c > > > > Please note that diffs are not public domain; they are subject to the > > copyright notices on the relevant files. > > This is wrong. The point of KMEM_SIZE is to store at a reliable location > the size of the buffer, in order to then verify that the 'size' argument > given to kmem_free() is correct. > > Here, you are using that size argument to compute the location, which > breaks the whole point of the check.
Sure I understand the frustration, but it's still correct according to the parameters I set for it 10+ years ago, which were for it to be a quick-n-dirty diagnostic aid. > Also it probably collides with the KASAN shadow. Hmm, is that purely an alignment issue then, because it works in 8 byte cells? Otherwise it sounds like this should not be enabled with KASAN at all. Andrew > Please revert this change. > > As said previously, if cacheline alignment is expected this way, then it > should clearly be part of the kmem contract, and documented to be so.