Received-SPF: pass (google.com: domain of linux-kernel+bounces-211759-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Wed, 12 Jun 2024 08:07:39 -0700
From: Eric Biggers <ebiggers@kernel.org>
To: Jan Kara <jack@suse.cz>
Cc: Hugh Dickins <hughd@google.com>, Yosry Ahmed <yosryahmed@google.com>,
	syzbot <syzbot+9c1fe13fcb51574b249b@syzkaller.appspotmail.com>,
	akpm@linux-foundation.org, chengming.zhou@linux.dev,
	hannes@cmpxchg.org, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, nphamcs@gmail.com,
	syzkaller-bugs@googlegroups.com, linux-ext4@vger.kernel.org
Subject: Re: [syzbot] [mm?] KMSAN: uninit-value in zswap_store
Message-ID: <20240612150739.GA1170@sol.localdomain>
References: <000000000000d0f165061a6754c3@google.com>
 <CAJD7tkaTQU1Kxt935fmq+_BJd-VT6vKFj58o7Aq+QhoyrOtCmw@mail.gmail.com>
 <02a21a55-8fe3-a9eb-f54b-051d75ae8335@google.com>
 <20240612075027.5pzkkovaewtx63n5@quack3>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240612075027.5pzkkovaewtx63n5@quack3>

On Wed, Jun 12, 2024 at 09:50:27AM +0200, Jan Kara wrote:
> 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.

Writing uninitialized memory to disk is definitely a bug.

- Eric