On Tue 11-06-24 02:13:59, Hugh Dickins wrote: > On Mon, 10 Jun 2024, Yosry Ahmed wrote: > > Hugh, do you mind taking a quick look at whether this is a real bug? > > > > If this is expected behavior, perhaps there is some annotation we can > > use in zswap that it is fine to encounter uninitialized data when > > reading the folio. > > I've not been faced with a KMSAN report before, so I might well be > misunderstanding its language: but this looks like an ext4 "bug" to me. > > I think the story that the three KMSAN stacks are telling is this: > syzbot has an ext4 filesystem on a loop device on a tmpfs file (I do > exactly that too); ext4 is doing some ext4_xattr_inode_write() business, > in the course of which it writes back a not-fully-initialized block to > the loop device; shmem faithfully copies that data into its pagecache, > then later when under memory pressure that page gets "written" out to > zswap: where zswap_is_page_same_filled() takes an interest in the data, > and KMSAN objects because some of it was not originally initialized. > > If that's a correct interpretation, then it's probably not a big deal: > it's probably the uninitialized end of a buffer that's written out, > not any part of the "disk" which ext4 would ever show to a user; but > I do agree with KMSAN that ext4 would do better to clear that area, > rather than accidentally storing someone else's super-secret info. Yes, that seems to be accurate. ext4_xattr_inode_write() stores large extended attribute in the inode and we don't bother to zero out the tail of the block we use since we never access bytes beyond xattr size. Frankly I don't consider this a bug since the uninitialized bytes are never exposed to (unpriviledged) userspace. But I agree that out of pure precaution and because it doesn't cost much in terms of performance we could zero out the block tail. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
