From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 37ACE19CC14; Tue, 10 Feb 2026 05:01:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770699685; cv=none; b=OB4SzEvYFahrgMAi2CUj251gttqAWhJ1QVaWpqslzeN5PajEnpZi8BLjI6QH3fDW1iU0OIZ4L5H0Ufs43/r0QJOKMMSCwfTBOcAhutZVZbh0ZMQ5V3psI+SQV1AWNzcumnpHz4ZNelHDwOJr4wrfaS4nys6RjFbY5AtlOwaJPe4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770699685; c=relaxed/simple; bh=rM6QjJtKoKs0itasWEpHjPkPV3940T+T4PgqfmckEtM=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=KeVBOu/w4DG7rN2RV2M08NqFV/x6MX5H/fuGjGdRTTmehdFc2R9Kgtke8hsNyaOqB5O7tplBmjktHKesTnoqTed+U+FmcJ1aSPPaQGpkNbMtCOKK2jtDbjWD9S3qp76uRDFOAq12fhhjFuy1+KF0N9y2asy21FWY/yvu/IZt2cA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=b7IJxP28; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="b7IJxP28" Received: by smtp.kernel.org (Postfix) with ESMTPSA id CC284C116C6; Tue, 10 Feb 2026 05:01:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1770699684; bh=rM6QjJtKoKs0itasWEpHjPkPV3940T+T4PgqfmckEtM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=b7IJxP28lbjIAUp5jP7mXFUxw56TGA2+FfMUB2ISerOb3jPXTlHDQdRvwFmf6O0nM ekmB0BhFaMuRGKIU2Uj+yqhJRaA1wWHucPkRbNqqNTs/ki3wd2esmOhXVXGfRNhKLF Xp3Na7+CeXUfrwJ4y8giOjg2nsSsGxEX6wpOloE6Kz3THJgtVxA/rIb235M+IIoILM PLP0D3A2xo10dVLF3eCLd2Qaa7wD2+Q+8hMJH4li7GvTH7iWXGRKbjyzT1kQyQf4Ym RWrntbLSJuatw/N3DN/iSBKBY3azGGgch8/yqZtHMkjP6jJmeX8bR2dnrc3MlMQap/ 01xCDYyphucPg== Date: Mon, 9 Feb 2026 21:01:24 -0800 From: "Darrick J. Wong" To: Kees Cook Cc: Fedor Pchelkin , Theodore Ts'o , =?utf-8?B?5p2O6b6Z5YW0?= , Andreas Dilger , Andy Shevchenko , linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org, Jan Kara Subject: Re: [PATCH] ext4: Reject on-disk mount options with missing NUL-terminator Message-ID: <20260210050124.GR7686@frogsfrogsfrogs> References: <20260206212654.work.035-kees@kernel.org> <20260209193945-80d9bfc8aa82b0eb1b764c7f-pchelkin@ispras> <202602091148.EDBFECE686@keescook> Precedence: bulk X-Mailing-List: linux-ext4@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <202602091148.EDBFECE686@keescook> On Mon, Feb 09, 2026 at 12:00:36PM -0800, Kees Cook wrote: > On Mon, Feb 09, 2026 at 08:27:50PM +0300, Fedor Pchelkin wrote: > > Kees Cook wrote: > > > ee5a977b4e77 ("ext4: fix string copying in parse_apply_sb_mount_options()") > > > Notices the loud failures of strscpy_pad() introduced by 8ecb790ea8c3, > > > and attempted to silence them by making the destination 64 and rejecting > > > too-long strings from the on-disk copy of s_mount_opts, but didn't > > > actually solve it at all, since the problem was always the over-read > > > of the source seen by strnlen(). (Note that the report quoted in this > > > commit exactly matches the report today.) > > > > > > > [...] > > > > > Reported-by: 李龙兴 > > > Closes: https://lore.kernel.org/lkml/CAHPqNmzBb2LruMA6jymoHXQRsoiAKMFZ1wVEz8JcYKg4U6TBbw@mail.gmail.com/ > > > Fixes: ee5a977b4e77 ("ext4: fix string copying in parse_apply_sb_mount_options()") > > > > Hi there, > > > > [ I'd better be Cc'ed as the author of the commit in Fixes ] > > Agreed! Sorry I missed adding you to Cc. > > > The mentioned reports are for v6.18.2 kernel while ee5a977b4e77 ("ext4: > > fix string copying in parse_apply_sb_mount_options()") landed in v6.18.3. > > Back at the time I've tested the patch with different bogus s_mount_opts > > values and the fortify warnings should have been gone. > > Ah-ha! Okay, thank you for catching this versioning issue. I had been > scratching my head over how it could have been the same warning. This > report is effectively a duplicate of the report you fixed with > ee5a977b4e77. > > > I don't think there is an error in ee5a977b4e77 unless these warnings > > actually appear on the latest kernels with ee5a977b4e77 applied. > > > > > @@ -2485,6 +2485,13 @@ static int parse_apply_sb_mount_options(struct super_block *sb, > > > if (!sbi->s_es->s_mount_opts[0]) > > > return 0; > > > > > > + if (strnlen(sbi->s_es->s_mount_opts, sizeof(sbi->s_es->s_mount_opts)) == > > > + sizeof(sbi->s_es->s_mount_opts)) { > > > + ext4_msg(sb, KERN_ERR, > > > + "Mount options in superblock are not NUL-terminated"); > > > + return -EINVAL; > > > + } > > > > strscpy_pad() returns -E2BIG if the source string was truncated. This > > happens for the above condition as well - the last byte is truncated and > > replaced with a NUL-terminator. > > Yeah, I've double-checked this now. The second half of the overflow > check in the fortified strnlen eluded by eyes when I went through this > originally. Thanks for sanity checking this! > > > The check at 3db63d2c2d1d ("ext4: check if mount_opts is NUL-terminated in > > ext4_ioctl_set_tune_sb()") was done in that manner as there is currently > > no way to propagate strscpy_pad() return value up from ext4_sb_setparams(). > > So the string is independently checked inside ext4_ioctl_set_tune_sb() > > directly. > > > > > > As for the 64/65 byte length part, now the rationale of the checks works > > as Darrick Wong described at the other part of this thread and corresponds > > to how relevant userspace stuff treats the s_mount_opts field: the buffer > > is at most 63 payload characters long + NUL-terminator. Jan Kara also > > shared similar thoughts during the discussion of ee5a977b4e77 [1]. > > > > [1]: https://lore.kernel.org/linux-ext4/yq6rbx54jt4btntsh37urd6u63wwcd3lyhovbrm6w7occaveea@riljfkx5jmhi/ > > Okay, great. I figure I can do two things: > > 1) rework this patch with adjusted commit log to reflect the notes > raised so far, so that we reject mounts that lack a NUL-terminated > s_mount_opts (as silent truncation may induce an unintended option > string, e.g. "...,journal_path=/dev/sda2" into "...,journal_path=/dev/sda" > or something weird like that). > > 2) Leave everything as-is, live with above corner case since it should > be unreachable with userspace tooling as they have always existed. > > I'm fine either way! :) I'd pick #1, unless someone knows of a userspace program that could have set a 64-byte s_mount_ops string with no null terminator. I didn't find any, but there are many implementations of ext4 out there. :/ (and yes, it's better to reject an unterminated s_mount_opts than accidentally point the kernel at the wrong block device) --D > -Kees > > -- > Kees Cook >