From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8DA78C02193 for ; Tue, 4 Feb 2025 21:25:46 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E99036B007B; Tue, 4 Feb 2025 16:25:45 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id E49856B0082; Tue, 4 Feb 2025 16:25:45 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D119E6B0083; Tue, 4 Feb 2025 16:25:45 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id B6A816B007B for ; Tue, 4 Feb 2025 16:25:45 -0500 (EST) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 41C4E160386 for ; Tue, 4 Feb 2025 21:25:45 +0000 (UTC) X-FDA: 83083544250.26.616F8FE Received: from mail-ed1-f52.google.com (mail-ed1-f52.google.com [209.85.208.52]) by imf16.hostedemail.com (Postfix) with ESMTP id 3B5E1180014 for ; Tue, 4 Feb 2025 21:25:42 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=KwedqdOt; spf=pass (imf16.hostedemail.com: domain of mjguzik@gmail.com designates 209.85.208.52 as permitted sender) smtp.mailfrom=mjguzik@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1738704343; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=F4C3nF0MrFU3lYanCxKKbK2G+JW9NeWKWWVNPsiXZNQ=; b=LS1Xn04HLsygZZpbZTXfncqOW1kBLZ2hAKf5uzb8FtwUy5+kwwl4SbKxn5iqhChe1+cYcy 3fTdX9R6EOPB+QZgxDUcRSc7EVi1zTbimkTK4vCsxgrBA9pGaMdV+VEmHdueA7CMBjhEqb q2L7j83Rz3V5zcCW4FQye0jnAKGK+Jc= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1738704343; a=rsa-sha256; cv=none; b=sOGxWb84S/iN31ocEvUYddLRy/KCTiQDYk9he9kZbyCyfFJceJxS4naR9pagxhoGf4Gc+F Scj2l9t3U6cP97vF6JzLYS43eqRIQlxJ47jZ5Kw95/eUoIUsn3r4/uFI9DpcIDrjjkgEFj 7J9wT1ajSW8FofpUaWYB427T8uZ4TVg= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=KwedqdOt; spf=pass (imf16.hostedemail.com: domain of mjguzik@gmail.com designates 209.85.208.52 as permitted sender) smtp.mailfrom=mjguzik@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-ed1-f52.google.com with SMTP id 4fb4d7f45d1cf-5dcd09af4f9so1723714a12.0 for ; Tue, 04 Feb 2025 13:25:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738704341; x=1739309141; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=F4C3nF0MrFU3lYanCxKKbK2G+JW9NeWKWWVNPsiXZNQ=; b=KwedqdOtDDM1MW8ndLflzF2lmeBo1jD1QPG/LqxqMk1aQDx822tHeKDQRsT9rA8mHL QCv8As8pWtX1Yoe2oe3OFPRAbHBi1dUOzOXAKlydi5J7QSTgbiBzxgXu49rVyryp6he4 HxDMAolm2U3me0u5RhqVK6zB+vYcjFtbJ0RXoVzpVxl3rVJ97hon8QS1u2vYo5OeRCTe PYA7ocF1/K7V7rpMHlNf0lUkQM9HRtJaiaz2MU0/Es/dS1XrlZFhPin+uwZQsImLw3D/ c+B5AbOwEeM6gpmGuqTW3Xbu4g5LtoH9kP36GWpMv1XAcfPBBTcWDe434wU4Ec6dtOtf 0f4A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738704341; x=1739309141; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=F4C3nF0MrFU3lYanCxKKbK2G+JW9NeWKWWVNPsiXZNQ=; b=vpoXzukyXqiDr7kHsxJuPc5FKSfwMu+DVgkwwb4ZHLvOQGAtdBVLYgRkQRoTKBw24A 8jtH/HS/tDSR3zB5ftC8YDadGPadsPFA/H2Hta9nObsLGcWoRXup+ZbSP/aXUNEa2rN4 /CNL354qxV0PTlIyc1+2/uV6MiWFLSEn0gIl4GNbFPMXrQaoDc+UuK792CkYHisQi2bp Q+br6dxuwOFGMGWjO+1/B3FRLExPSDKtPrTfzPDi0y3Rf4Qfzmh2wJkL2P73NH5ialZl QlDPeMG72hbHe8BNrDJFuicrPlHCoODTKjtEz1fkSGCrMsfNmcqRbqh0EmKC/Ogl8X0R F/dA== X-Forwarded-Encrypted: i=1; AJvYcCUXVvyim+Wq5hF3uqByaI3m6oNvP8te1zYKic3QnsN0QqZb9VPPexaOthaFszbwLGUJpFuBacs1+A==@kvack.org X-Gm-Message-State: AOJu0YzlKMBhei5cwH5VnVCLJW1pE0U6bL7uxP4iwq4Gb2NMMlX9/359 y9la/r7zfWYrGT4Lb6rppA7JleunAXvLydQO3AtHSWJex17Mbdzrw5FiDIoV6QI1TvWQIWDV0an 07Pj2853IPskgGG34Wp3YZE5o3eo= X-Gm-Gg: ASbGncuDjFwpckky5HD/XWJqyyoQf3+wH4wgI0fWuuTg0sFBLhnGmBW+EadWlBhO7Bx OKdE0Z5GD5isrN4U/tDeN3bp+7Ohi6TqZWc9jzvyZdPsSWAsnlOHlouFfSXKg2GwI1SCRZH0= X-Google-Smtp-Source: AGHT+IGjcSWjs+ThZeODFo1Ch/yAVmyGtwLMyZHY93carg6mxOuz8K/lwuEqDLo0g3BJ/62afzxS2kQCol8JaUeguSE= X-Received: by 2002:a05:6402:358d:b0:5dc:cc02:5d25 with SMTP id 4fb4d7f45d1cf-5dcdb73336emr654488a12.11.1738704341310; Tue, 04 Feb 2025 13:25:41 -0800 (PST) MIME-Version: 1.0 References: <67a1e1f4.050a0220.163cdc.0063.GAE@google.com> <202502040717.FCEFDB7E0@keescook> <20250204203059.GA909029@mit.edu> In-Reply-To: From: Mateusz Guzik Date: Tue, 4 Feb 2025 22:25:29 +0100 X-Gm-Features: AWEUYZn5Bnu3knhWyzIOw0Z5zwfvbIPBa4gVp-XE170mO4LdzTpwkbg3B8Bab6M Message-ID: Subject: Re: [syzbot] [hardening?] [mm?] BUG: bad usercopy in vfs_readlink To: "Theodore Ts'o" Cc: Kees Cook , syzbot , akpm@linux-foundation.org, brauner@kernel.org, gustavoars@kernel.org, linux-hardening@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, syzkaller-bugs@googlegroups.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: 31yj1jbocpwuqeowgfd114b6t14chokk X-Rspam-User: X-Rspamd-Queue-Id: 3B5E1180014 X-Rspamd-Server: rspam03 X-HE-Tag: 1738704342-112200 X-HE-Meta: U2FsdGVkX18YSHCAm3JmvKL2eDbOFb1IZSsnIv1NXXuWDXoSEUbG5ZU7mEA4sH6DiyIfulabgzIWnBVZBRBU1UMra4fuFMRXSuaK5y/OJp1fXKYwjY/OQfQcD2nnUZFVcDsXTzo5X9egdSA5CbhM7bXLbrgHWUYG3pvjLofEcG5cx4eTj7s8f4BiBpvuvU/W5TjWXiSQXDJ4wUk8dJgd5cl49Hu0u6zpQvjBgGSivzj88SqUx/kNZUknISAYhuSwI/n7FuZXv6Zbg90WSRHjxFoAKOjwBCImblx3thHxJazL4x7OpWrg/BlcmdKsndgtHLO0NDrHx4ADaDstIZ92h9fxHrK+vZwM+vE5OXM1PNL7OebnSDHw166HeBrf4K1jWSSmtohnuoy5k3kJZkfzwvFbWM5VbSma1K3pRk92nl+vnC8to/+IgdNqS+mdzULX3sDVuB9Y7+S7CiUtqtYV6iaXimqwx10NI7RthfF/80qWo0YceQkJYaNdFuX7AMpTOrZKVisfMt9ecRAgLb/M4Sit6a65XEE4MqDQEz8CyLg5QYmJz6EIeuecsljISkX4pTXtttcXuGS5/ql7772Zz2taCKX9LfEe1sLSnF5wPOXGGWMEjdlHSNSi8E0SOg3kvgpAFqIAw+7vR1/Jf6QZJIItK4bKlDmICQ10scGvXFi5xhc5vu2O1eVb/hlewIudblz3WnUAXzvdBd1uJTt55GFzpQUOMC6j/r+6c3TTS+YWz54r4yxu4fLxJelRC3XZ+C1TPa+1mXcU5u0wNUbhDEQ/sff33Ws6l4RG+BIsV3rBwvsIHKJuMUEtBdVmRb/WNh+jU3ehBR4Jpg081L0703F24hdXWh2+DGmzrWgo7VVZGXdt+l25IEX2CzJqSXFu+pK/fogyT8106pYMGSSec9VfZru+3Bpx6zax3l4dDE0t3BkSHZ/GBIK88BgSPFR8rQORSg543D1XkocJCIj xpjwivY1 UGCVV6EbF91Bhq1vHObNNul7m9UiwbxPrKaLAJh34lJfqSnCwjE3xotqyRbEH2E609AxdyTKqzb+Lk7MBiYYP/OGqDnImTeaDOC4G48ID8H+eE5awkp1DqIGhGSOVitP21DMylFN/Ne3NJUEuFkmhtPds4DI0NU1ocIiGmaVCF9c2mNyKe61QuDky5491Mu6vCEoeOj44HNtuby5Ur+UrjkzLbJjs5Mta6yGdTJ/TzIMZ6S9F7GRRHT6RinvW9Bz79EHKAnk8PjhkaAG/GtsJ+7Ftj/Vum2hm84K6ggqkFzkgyZc+9hMQ7SeX9/dXKnUK+uaVv6l72aYU5i70Bazacne3DEAEiCD24MU6Kf8p10xl8FjQKvmRRTxjDklt9TleieH+OGOW3Q5iulWMKW0pRDeNJWCoUEjhTVgiy+EBM+V+2QOZOZDoevO99CV79aUGzA4/xVXOFrsSzmWFAsVe/NagdGjXqeRcLpfgeHZl3I5nszGaKvQGG3CDPbFY0X64jA5yUw0zX7jcWuD8j1IK3h/UVzZzgPjJo9ZeR19neSet1Rcsl4wbAyXzxG9AIAhxotE5 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Tue, Feb 4, 2025 at 9:48=E2=80=AFPM Mateusz Guzik wr= ote: > > On Tue, Feb 4, 2025 at 9:31=E2=80=AFPM Theodore Ts'o wrot= e: > > > > On Tue, Feb 04, 2025 at 05:49:48PM +0100, Mateusz Guzik wrote: > > > I'm going to restate: the original behavior can be restored by > > > replacing i_size usage with a strlen call. However, as is I have no > > > basis to think that the disparity between the two is legitimate. If a= n > > > ext4 person (Ted cc'ed) tells me the disparity can legally happen and > > > is the expected way, I'm going to patch it up no problem. > > > > There are two kinds of symlinks in ext4. "Fast symlinks", where the > > symlink target can fit in i_block[]. And normal, sometimes called > > "slow" symlinks, where i_block[] contains a pointer to data block > > (either i_blocks[0] for a traditional inode, or the root of an extent > > tree that will fit in i_block[] if EXTENTS_FL is set). In the case > > of a fast symlink, the maximum size of the file system is > > sizeof(i_block[])-1. For a normal/slow symlink, the maximum size is > > super->s_blocksize. > > > > We determine whether a symlink is "fast" or not by checking whether > > i_size is less than sizeof(i_blocks[]). In other words. if i_size > > less than 60 (and it's not an encrypted inode), it's a fast symlink > > and we set i_link to point to the i_block array. From __ext4_iget() > > in fs/ext4/inode.c: > > > > if (IS_ENCRYPTED(inode)) { > > inode->i_op =3D &ext4_encrypted_symlink_inode_o= perations; > > } else if (ext4_inode_is_fast_symlink(inode)) { > > inode->i_link =3D (char *)ei->i_data; > > inode->i_op =3D &ext4_fast_symlink_inode_operat= ions; > > nd_terminate_link(ei->i_data, inode->i_size, > > sizeof(ei->i_data) - 1); > > } else { > > inode->i_op =3D &ext4_symlink_inode_operations; > > } > > > > > > The call to nd_terminate_link() guarantees that inode->i_link is > > NUL-terminated, although the on-disk formatshould have already been > > NUL-terminated when the symlink is set. > > > > It is nul-terminated, but inode->i_size does not line up with it -- as > in the inode size pulled from the disk image is different than what > strlen would suggest. > > My question is if that's legitimate, I'm guessing not. If not, then > ext4 should complain about it. > > On stock kernel this happens to work because strlen finds the "right" siz= e. > So it occurred to me to check what fsck thinks about it. I ran it twice in a row, it *removed* the problematic symlink. e2fsck 1.47.0 (5-Feb-2023) One or more block group descriptor checksums are invalid. Fix? yes Group descriptor 0 checksum is 0xd7c5, should be 0xa2fe. FIXED. Pass 1: Checking inodes, blocks, and sizes Pass 2: Checking directory structure Symlink /file0/file1 (inode #14) is invalid. Clear? yes Pass 3: Checking directory connectivity Pass 4: Checking reference counts Pass 5: Checking group summary information [ERROR] ../../../../lib/support/quotaio_tree.c:546:check_reference: Illegal reference (4294967295 >=3D 6) in user quota file Update quota info for quota type 0? yes [QUOTA WARNING] Usage inconsistent for ID 4294967295:actual (0, 0) !=3D expected (458752, 8) [QUOTA WARNING] Usage inconsistent for ID 0:actual (458752, 8) !=3D expected (0, 0) [ERROR] ../../../../lib/support/quotaio_tree.c:546:check_reference: Illegal reference (256 >=3D 6) in group quota file Update quota info for quota type 1? yes syzkaller: ***** FILE SYSTEM WAS MODIFIED ***** syzkaller: 16/32 files (0.0% non-contiguous), 160/512 blocks So I stand by my assessment that the symlink fetching code should check for the problem of size vs strlen disparity. > > Also note that there really is no point to caching inodes where > > inode->i_link is not a NUL pointer, since in those cases we never call > > file system's get_link() function; the VFS will just fetch it straight > > from i_link. And in those cases, it's because it's a "fast symlink" > > (many file systems have this concept; not just ext[234]) and the > > symlink target was read in as part of the on-disk inode, and so there > > is no separate disk block read read the symlink. And so if you are > > attempting to cache symlinks where inode->i_link is non-NULL, you're > > just wasting a small amount of memory, and wasting the CPU time to do > > the strcpy. > > > > My code does not allocate any extra memory or do any extra copies. > > The code prior to my change would grab i_link, strlen on it and pass > that to copy_to_user every single time readlink is issued. > > My code stores the size in the inode (unioned with a field not used > for symlinks, so no again no increase in memory usage) so that the > strlen call can be avoided. Past that it's the same thing. > > -- > Mateusz Guzik --=20 Mateusz Guzik