On Wed, Jan 17, 2024 at 03:08:47PM -0500, Gabriel Ryan wrote: > We found a race in the mm subsystem in kernel version v5.18-rc5 that What an odd kernel version to be analysing. It's simultaneously almost two years old, and yet not an actual release. You'd be better off choosing an LTS kernel as your version to analyse, something like v6.6 or v6.1. Or staying right on the bleeding edge and using something more recent like v6.7. > appears to be potentially harmful using a race testing tool we are > developing. The race occurs between: > > mm/readahead.c:140 file_ra_state_init > > ra->ra_pages = inode_to_bdi(mapping->host)->ra_pages; > > block/ioctl.c:497 blkdev_common_ioctl > > bdev->bd_disk->bdi->ra_pages = (arg * 512) / PAGE_SIZE; > > > which both set the ra->ra_pages value. It appears this race could lead > to undefined behavior, if multiple threads set ra->ra_pages to > different values simultaneously for a single file inode. Undefined behaviour from the C spec point of view, sure. But from a realistic hardware/compiler point of view, not really. These are going to be simple loads & stores. since bdi->ra_pages is an unsigned long. And if it does happen to be garbage, how much harm is really done? We'll end up with the wrong readahead value for a single open file. Performance might not be so great, but it's not like we're going to grant root access as a result. We should probably add READ_ONCE / WRITE_ONCE annotations to be certain the compiler doesn't get all clever, but that brings me to the question about why you're developing this tool. We already have tooling to annoy people with these kinds of nitpicks (KCSAN).
