From nobody Mon Jun 8 21:46:10 2026 Received: from mail-pl1-f174.google.com (mail-pl1-f174.google.com [209.85.214.174]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6688A3E0C59 for ; Tue, 26 May 2026 13:32:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.174 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779802325; cv=none; b=UbQvBwhVRdydjJ3MK8U0LSH0Zt8cSv/e1FrseqEGYl53Uxn4ctrhrnbcwccSxVPtA1zEWcZwm8UyfmiNPA27NZiJv/d+gSoW9I7McHKipeLevcjbjWztRp0/vgIDGm8Aa/m+nw9ErkKxnsvjgOA00muw35bt5o/GK1ODakDaDJU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779802325; c=relaxed/simple; bh=HwhzAtM+XNwwG9kqZSabVMEeDxCqRAqk3cTMua2G0xQ=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=B0zqizsIcCX1MttKub0aH8YK3EF5QcuGxY0emjmImMoWv/GK2NC3Oogu8ViMGrJZ1ScitpIykXCC2WpYVt5Ud8zqvvbgw+tnIg4VRv5fPidXvwMpLMxL2UBfNYrrfULvi1DXxZLaZ2C7DweXYSzuAiERrybjhSmmMllCMUJ6kDk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=V4g5a0Vb; arc=none smtp.client-ip=209.85.214.174 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="V4g5a0Vb" Received: by mail-pl1-f174.google.com with SMTP id d9443c01a7336-2bad6226d1cso14592195ad.2 for ; Tue, 26 May 2026 06:32:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1779802318; x=1780407118; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=OqeVPJKaMhyjbC6h9N+4+fsMpUnV3HwawjE7ULuFdT0=; b=V4g5a0VbenPvgfTSlBf9bQ5QjM+SGPVZgED1ixsbHvfcv8HL7JVyB8O0fI4PRbUtu3 6OkzutEWACcZrQQ63Hf+igwsqXjoiwCnIus9bBhse7H7MNoLj4VcEfUxcVBfVUpFgI2W 7L5I1dxPQSCg/8npSAiuWMgcL/1zoygNaQveGDWuzoHu6NNAfk5dgoclbRrvIDjAjDwd fcSg7E6jIfJRiMr1bTx2R5ov/o9EonNc3ov1L5tQPCre0PHwPkBBWme2tSw/vaFT7PUL WZtfzFOmnQm7tkVWYU+ADGAPJZc2ftbkXqi6C5HouvMA0vxL5B91frtUKJQyvjfPhoJa /UNA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779802318; x=1780407118; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=OqeVPJKaMhyjbC6h9N+4+fsMpUnV3HwawjE7ULuFdT0=; b=CJ6+/zOFmxJHC7TR9s1LAVcj0Dmk1mvInWRd2XnBNosJSLPjh82oT9rJDjv5y0T+G2 x0qEKhHAr2G0b7E1rv3sevRKyJU+YGHQbzZuKmQvy9kxRr9t/2LcCdm/+OuPUQ5BUbLI VTsoGOfxxznN+UoLw9rDgJnyiRbQuo2Qtg+dLELnq4/djyNOCQSa3E1as/NKZdvxlACz +vFKg3dELFyg8jh0AGrlsZ5EbRfC1o7R14XWG/TnYhOf0UGno43okDDZCFiSVHm3Tk7A fdhkknR8feVEB6co6onADRYz4wMyg28b5bJO5w7KkiiHmLAPFVdogQwful4r/kQEHQgC PaiA== X-Forwarded-Encrypted: i=1; AFNElJ9u49AkC0QbvvEUZGm3UNVdqw6mqArA5/qXrYIbVTSNj5PniVh7RDsuAn6L0V8r1b3qmqoVPFoIoGVvIhM=@vger.kernel.org X-Gm-Message-State: AOJu0YwQqIrIOuqllQmwgBpTY94dbWJ/xBVMnhSe4KbaTECWAzOxgHJA 7aXlNYQjl6/7RXYYBwENIIAHtmGPdpWYCELu5TifR5nT0pG4VY9ndgQM X-Gm-Gg: Acq92OH8mIapPfUOqcmbNfwsu906GuBkvfcMBgAIHIdIBdpOk6HbwIR6f3UJpaqHaVD rW1yqD3Qjv5HHhYYgLlDywPI7Zx+hAtWRk+3TeTlKE6w9YANJyiVFqm4qDF9q4qgAzt3anda1M6 LGmDEvh+pAq2SMfGzIYEieJOeoSOMOYDrdW2IHlMbx2w0kU83u2Lbi0ainB2TO2qo/Obv+xS2cd aIdupPP0RIh9bOf4L8qTlAEndoSTxFdjiUfDpSZ+Op3eV/De9PF3HkXoG9dbDTRFo8XZkhtlICE a3EEmx6jo+DuQiCrPjtSG2unngByzMfP4vDNDD8MYpVG6QF0E0QUDpWjsEEvkaVIBczlWTWkOZo gNrJBlDzBKVHEtNPAbAKQLX9s9mYVmyR4RpjPRUJDVeNg9hXbGAUAf9oKAoQpicp1NIyg5nQcnL kY3yIouJIFiEhZs38I X-Received: by 2002:a17:902:ced1:b0:2bc:cb8f:c286 with SMTP id d9443c01a7336-2beb07a16a7mr109198715ad.7.1779802317555; Tue, 26 May 2026 06:31:57 -0700 (PDT) Received: from ser8.. ([221.156.231.192]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-2beb591d102sm122455505ad.81.2026.05.26.06.31.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 26 May 2026 06:31:57 -0700 (PDT) From: DaeMyung Kang To: Namjae Jeon , Hyunchul Lee Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, DaeMyung Kang Subject: [PATCH v3] ntfs: validate resident attribute values on lookup Date: Tue, 26 May 2026 22:31:52 +0900 Message-ID: <20260526133152.1205929-1-charsyam@gmail.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: References: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" ntfs_attr_find() and ntfs_external_attr_find() check that generic resident attribute values fit in their attribute records and that fixed-size resident values are large enough. For variable-length resident formats, however, the fixed part is not enough: embedded length fields can still point callers past the resident value. A crafted image can set a small resident $FILE_NAME value_length while leaving file_name_length large. Callers then trust file_name_length and read past the resident value when converting or comparing the name. This was reproduced with a crafted image under KASAN as a slab-out-of-bounds read from the kmalloc-1k MFT record copy. The stack included ntfs_lookup(), ntfs_iget(), ntfs_read_locked_inode(), ntfs_attr_name_get(), ntfs_ucstonls(), and utf16s_to_utf8s(). Add a shared resident attribute value validator and use it before a lookup path can return an attribute, including the AT_UNUSED enumeration case where callers inspect returned attributes directly. The helper validates resident value bounds, minimum value sizes, and the variable-length $FILE_NAME field, while preserving the fixed-size checks for attributes such as $VOLUME_INFORMATION and $EA_INFORMATION. Log corruption at the rejection point with the attribute name where known. Reject non-resident $FILE_NAME records too: the format requires $FILE_NAME to be resident and callers treat returned records as resident. Type-specific format validation for $VOLUME_NAME is intentionally not added to this helper, even though $VOLUME_NAME is a variable-length resident attribute. Existing callers such as ntfs_write_volume_label() treat a failed AT_VOLUME_NAME lookup as an absent label and unconditionally add a new record afterwards. If the helper rejected odd-length labels at lookup time, the corrupt record would stay in place and the caller would append a new record next to it, producing two $VOLUME_NAME attributes on disk. Caller-side cleanup must precede any lookup-time rejection of malformed $VOLUME_NAME payloads, so the type-specific check is left for a follow-up patch. The generic resident bounds checks (value_offset / value_length / attr_len) still apply to $VOLUME_NAME, and its entry in the type-name table is kept so that those failures log with the attribute name. Do not add $INDEX_ROOT-specific value validation in this change. Testing with stricter checks showed an existing ntfs_ir_truncate() shrink ordering bug: the resident value_length is shrunk before the root index allocated_size is updated, so a relookup can observe allocated_size beyond the now-smaller resident value and fail. Fix that ordering before adding the $INDEX_ROOT-specific minimum-size and structural checks. Fixes: 6ceb4cc81ef3 ("ntfs: add bound checking to ntfs_attr_find") Signed-off-by: DaeMyung Kang --- Patch 1/2 of the previous series ("ntfs: free link name from ntfs_name_cache") has been applied to ntfs-next; this v3 contains only the remaining patch, with the resident attribute validation reworked per review feedback. Changes since v2: - Drop $INDEX_ROOT-specific validation from this patch, including both the minimum-size table entry and the structural checks. QEMU testing with xfstests generic/001 showed that those checks expose an existing ntfs_ir_truncate() shrink ordering bug, where value_length is reduced before allocated_size is synchronized. The same test passes when allocated_size is lowered before shrinking value_length, with the $INDEX_ROOT-specific validation enabled. That ordering issue will be fixed in a separate patch, after which the $INDEX_ROOT-specific validation can be reintroduced in a follow-up. Changes since v1: - Refactor resident attribute validation into a shared helper ntfs_attr_value_is_valid() used by both ntfs_attr_find() and ntfs_external_attr_find(), and remove the duplicated inline check from ntfs_external_attr_find(). - Split out resident header/value bounds extraction into ntfs_resident_attr_value_get(), returning a small view struct so the top-level orchestration reads as: get view -> common min check -> per-type format check. - Validate before the AT_UNUSED enumeration path in ntfs_attr_find() can return the attribute to callers that inspect it directly. - Reject non-resident $FILE_NAME records: the format requires $FILE_NAME to be resident and callers treat returned records as resident. - Type-specific format validation for $VOLUME_NAME is intentionally not added; see the commit message for the caller-side reason. Generic resident bounds checks still apply, and the $VOLUME_NAME entry in the type-name table is kept so that those failures log with a readable attribute name. - Log corruption at the rejection point with the attribute type name via a new ntfs_attr_type_name() helper, matching the existing attribute-name corruption message in ntfs_attr_find(). fs/ntfs/attrib.c | 148 +++++++++++++++++++++++++++++++++++++++++++--------= ---- 1 file changed, 117 insertions(+), 31 deletions(-) diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c index 421c6cdcbb53..382d1b6e877e 100644 --- a/fs/ntfs/attrib.c +++ b/fs/ntfs/attrib.c @@ -595,6 +595,112 @@ static u32 ntfs_resident_attr_min_value_length(const = __le32 type) } } =20 +static const char *ntfs_attr_type_name(const __le32 type) +{ + switch (type) { + case AT_STANDARD_INFORMATION: + return "$STANDARD_INFORMATION"; + case AT_FILE_NAME: + return "$FILE_NAME"; + case AT_VOLUME_NAME: + return "$VOLUME_NAME"; + case AT_VOLUME_INFORMATION: + return "$VOLUME_INFORMATION"; + case AT_INDEX_ROOT: + return "$INDEX_ROOT"; + case AT_EA_INFORMATION: + return "$EA_INFORMATION"; + default: + return NULL; + } +} + +static bool ntfs_file_name_attr_value_is_valid(const u8 *value, const u32 = value_length) +{ + const struct file_name_attr *fn; + u32 file_name_size; + + if (value_length < ntfs_resident_attr_min_value_length(AT_FILE_NAME)) + return false; + + fn =3D (const struct file_name_attr *)value; + file_name_size =3D fn->file_name_length * sizeof(__le16); + + return file_name_size <=3D + value_length - offsetof(struct file_name_attr, file_name); +} + +struct ntfs_resident_attr_value { + const u8 *data; + u32 len; +}; + +static bool ntfs_resident_attr_value_get(const struct attr_record *a, + struct ntfs_resident_attr_value *value) +{ + u32 attr_len; + u16 value_offset; + + attr_len =3D le32_to_cpu(a->length); + if (attr_len < offsetof(struct attr_record, data.resident.reserved) + + sizeof(a->data.resident.reserved)) + return false; + + value->len =3D le32_to_cpu(a->data.resident.value_length); + value_offset =3D le16_to_cpu(a->data.resident.value_offset); + + if (value->len > attr_len || value_offset > attr_len - value->len) + return false; + + value->data =3D (const u8 *)a + value_offset; + return true; +} + +static bool ntfs_attr_value_is_valid(struct ntfs_volume *vol, + const struct attr_record *a, + const u64 mft_no) +{ + struct ntfs_resident_attr_value value; + const char *type_name; + u32 min_len; + + if (a->non_resident) { + if (a->type !=3D AT_FILE_NAME) + return true; + ntfs_error(vol->sb, + "Corrupt non-resident $FILE_NAME attribute in MFT record %llu\n", + mft_no); + return false; + } + + if (!ntfs_resident_attr_value_get(a, &value)) + goto corrupt; + + min_len =3D ntfs_resident_attr_min_value_length(a->type); + if (min_len && value.len < min_len) + goto corrupt; + + switch (a->type) { + case AT_FILE_NAME: + if (!ntfs_file_name_attr_value_is_valid(value.data, value.len)) + goto corrupt; + break; + } + return true; + +corrupt: + type_name =3D ntfs_attr_type_name(a->type); + if (type_name) + ntfs_error(vol->sb, + "Corrupt resident %s attribute in MFT record %llu\n", + type_name, mft_no); + else + ntfs_error(vol->sb, + "Corrupt resident %#x attribute in MFT record %llu\n", + le32_to_cpu(a->type), mft_no); + return false; +} + /* * ntfs_attr_find - find (next) attribute in mft record * @type: attribute type to find @@ -705,8 +811,11 @@ static int ntfs_attr_find(const __le32 type, const __l= e16 *name, } } =20 - if (type =3D=3D AT_UNUSED) + if (type =3D=3D AT_UNUSED) { + if (!ntfs_attr_value_is_valid(vol, a, ctx->ntfs_ino->mft_no)) + break; return 0; + } if (a->type !=3D type) continue; /* @@ -747,24 +856,11 @@ static int ntfs_attr_find(const __le32 type, const __= le16 *name, } } =20 - /* Validate attribute's value offset/length */ - if (!a->non_resident) { - u32 min_len; - u32 value_length =3D le32_to_cpu(a->data.resident.value_length); - u16 value_offset =3D le16_to_cpu(a->data.resident.value_offset); - - if (value_length > le32_to_cpu(a->length) || - value_offset > le32_to_cpu(a->length) - value_length) - break; + if (!ntfs_attr_value_is_valid(vol, a, ctx->ntfs_ino->mft_no)) + break; =20 - min_len =3D ntfs_resident_attr_min_value_length(a->type); - if (min_len && value_length < min_len) { - ntfs_error(vol->sb, - "Too small %#x resident attribute value in MFT record %lld\n", - le32_to_cpu(a->type), (long long)ctx->ntfs_ino->mft_no); - break; - } - } else { + /* Validate non-resident mapping-pairs fields. */ + if (a->non_resident) { u32 min_len; u16 mp_offset; =20 @@ -1252,6 +1348,9 @@ static int ntfs_external_attr_find(const __le32 type, =20 ctx->attr =3D a; =20 + if (!ntfs_attr_value_is_valid(vol, a, ctx->ntfs_ino->mft_no)) + break; + if (a->non_resident) { u32 min_len; u16 mp_offset; @@ -1279,19 +1378,6 @@ static int ntfs_external_attr_find(const __le32 type, u32 value_length =3D le32_to_cpu(a->data.resident.value_length); u16 value_offset =3D le16_to_cpu(a->data.resident.value_offset); =20 - if (attr_len < offsetof(struct attr_record, data.resident.reserved) + - sizeof(a->data.resident.reserved)) - break; - if (value_length > attr_len || value_offset > attr_len - value_length) - break; - - value_length =3D ntfs_resident_attr_min_value_length(a->type); - if (value_length && le32_to_cpu(a->data.resident.value_length) < - value_length) { - pr_err("Too small resident attribute value in MFT record %lld, type %#= x\n", - (long long)ctx->ntfs_ino->mft_no, a->type); - break; - } if (value_length =3D=3D val_len && !memcmp((u8 *)a + value_offset, val, val_len)) { attr_found: --=20 2.43.0