From nobody Mon Jun 8 22:52:29 2026 Received: from mail-pj1-f45.google.com (mail-pj1-f45.google.com [209.85.216.45]) (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 E83A1335568 for ; Mon, 25 May 2026 16:32:59 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.45 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779726781; cv=none; b=YLqdaRYps8IarKjIZAvKa2lHLxUlxpqfGzVugJoiMlvpNJ+jnXrAt4/45hMsO7s85DSUthDAe2cbb/KYGz2KMvSo96QhkRFssnvlWYByRr1LymedgPha8NUx+AG/QaPMR2nGER03GACz0oClpuOPITxCDuFdWGDN+IjzP1jTNJ4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779726781; c=relaxed/simple; bh=9chFCJVkQfUv9IOHg6aT2DEbMxsdxlpxk/YTOkn74Es=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=RzawB+wmhmPF6K0voAhwGqpyjE5nYY5UYVqWif5LLDw72/CDV4Gy9QAd/8IouPtaxxli0c4/Vk3XxTz4Y6knNilE9xmudnzBSr4plSLTOJ8TLAQ2bsE2SuaWq09xbpULBznF1xhumclHSO3KXnpB848G14GOGS8O+UuWCbHQtqo= 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=iVB3XBJU; arc=none smtp.client-ip=209.85.216.45 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="iVB3XBJU" Received: by mail-pj1-f45.google.com with SMTP id 98e67ed59e1d1-36add801e24so251951a91.1 for ; Mon, 25 May 2026 09:32:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1779726779; x=1780331579; 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=IXYIjjNTf7p0VT2d/kxQo1Swgf2FpjR3wFuBOuUxxkI=; b=iVB3XBJU0tS4+hoIJ187qEuw/6kodOlNVT2UtrprpMP/zoHjgT2TIRn88fbrPnJDVt qC6FKPATei0WluxjMSmid4ZSjCvkr/EWmstDhb/k98t0PMNONiKtUCoMkCc9mP7iT9Gf DR7ofC/Wc8Prn0MOtaexe+l5rYl19+jBsx6jig7CMvPWpqsixjLl9syccJAmXAVe9h7U NkL81HBzg6klWxfjsU7LrZIJFlyzenEmfUdUJ6+mSRewGFsA7hHRqYYij34Oapj4ixKJ 0XdY539AF0QCdUo4J2ORTxtdZxyM55a4P86+zvhEnCXPjWiADf8MiCKTMlqSppuvwril xP2w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779726779; x=1780331579; 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=IXYIjjNTf7p0VT2d/kxQo1Swgf2FpjR3wFuBOuUxxkI=; b=XG3xI0NNzdJ8Dv3a4kBKP0MgsJm0MWsMwICL5awzuLO2HQe0GGhyOOnNmq7utS5ZGk 0K3sRBSA1gjgLRK+MRavdYMZCwvmDb6gQP+vc/yBcYzgzXWUJrvY/wTp+iHrAjBr1Mxs FeuMaZmdKQjoCIcuTiJ9EfA8vjLirANh6sI/NA63t2khAgY3sitPybq/mQx/S+w4syFR FkWrmvAirBrrSQ6UpihjVZklv0Ce66Lw7uMHemp9J7rYKwzevF+Dpdm7ozw40iX2x3qz ppvrAsaDJLER/cF+QLbslAWaXip9qwQCSeBZ32D8fooRw+xoFl9FBNC6r+aczipuJzoy QlGw== X-Forwarded-Encrypted: i=1; AFNElJ/PeAPwjt7f1loOQwwhDeil4FJD4d+2IRXwegME3cTQUcCuDw3rt1SJRlWgZ0VaJkB5D4k8klkejgeQ68c=@vger.kernel.org X-Gm-Message-State: AOJu0YwKgVdYwJVkgE7LCyJQGrayhZmVoMdpSyFyBhyq3r5XxlGdI7xt mss8Wj87dwo1lCwgfzqBXrqE/2ai2w6AbtBSWMAEMjSAjayJBpX65c9L X-Gm-Gg: Acq92OGs94Qb9Zt++dHroLxG+VWdIsuORoaF+k/M7cGzdqMNoAHLYy6u+rnZJWA2xlA QTF8tXSYz1a7oTw70RGOFR4dJuBzj2NbIzn5toa63o8Hw2vddFvpxnOdxHRR8havl1XY2BjHS+F WS8sQ3H/N4Ai2F9XGKWx5xX9x+cWLmH6beFyEwGA3CyEIuCrH6RMXZP/UsYaDY3NiXWzJNaSzAO UExewUw7l0jM3rF7IOzOj3ocLWbsU/htyh/4A4vJC59H1rbjbTVCgwtfCgKpwDS3G6upnVaH3Hs oj0z+DHvDg8/sqki2/N0L0RaHnY+0Np03oV3xj9F+FWUU/ngkEvaBtroC2o9Azpf5oLvnrqchgB gaGYHUhgxQBG4JdcKJVLAKySfBy9dwk0L14HNS1jmXsgpHVNBxWtTSQZy+YG2U+OrbE1el60SHu etc6/BXgM7btkPg4l4Texx6/CqASo= X-Received: by 2002:a17:90b:4b91:b0:36a:5b60:553b with SMTP id 98e67ed59e1d1-36a678c789amr8073092a91.7.1779726779062; Mon, 25 May 2026 09:32:59 -0700 (PDT) Received: from ser8.. ([221.156.231.192]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-36a72c4ca35sm10369609a91.9.2026.05.25.09.32.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 25 May 2026 09:32:58 -0700 (PDT) From: DaeMyung Kang To: Namjae Jeon , Hyunchul Lee Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v2] ntfs: validate resident attribute values on lookup Date: Tue, 26 May 2026 01:32:54 +0900 Message-ID: <20260525163254.240238-1-charsyam@gmail.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260524054238.3129288-3-charsyam@gmail.com> References: <20260524054238.3129288-3-charsyam@gmail.com> 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 variable-length $FILE_NAME and $INDEX_ROOT fields, 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. 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 v2 contains only the remaining patch, with the resident attribute validation reworked per review feedback. 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. - Extend type-specific validation beyond $FILE_NAME to $INDEX_ROOT (entries_offset / index_length / allocated_size bounds and 8-byte alignment); add $INDEX_ROOT to ntfs_resident_attr_min_value_length(). - 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 | 182 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 151 insertions(+), 31 deletions(-) diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c index 421c6cdcbb53..63b3b0d13c45 100644 --- a/fs/ntfs/attrib.c +++ b/fs/ntfs/attrib.c @@ -588,6 +588,8 @@ static u32 ntfs_resident_attr_min_value_length(const __= le32 type) sizeof(__le16) * 1; case AT_VOLUME_INFORMATION: return sizeof(struct volume_information); + case AT_INDEX_ROOT: + return sizeof(struct index_root); case AT_EA_INFORMATION: return sizeof(struct ea_information); default: @@ -595,6 +597,144 @@ 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); +} + +static bool ntfs_index_root_attr_value_is_valid(const u8 *value, const u32= value_length) +{ + const struct index_root *ir; + u32 index_size; + u32 entries_offset; + u32 index_length; + u32 allocated_size; + + if (value_length < ntfs_resident_attr_min_value_length(AT_INDEX_ROOT)) + return false; + + ir =3D (const struct index_root *)value; + index_size =3D value_length - offsetof(struct index_root, index); + entries_offset =3D le32_to_cpu(ir->index.entries_offset); + index_length =3D le32_to_cpu(ir->index.index_length); + allocated_size =3D le32_to_cpu(ir->index.allocated_size); + + if ((entries_offset | index_length | allocated_size) & 7 || + entries_offset < sizeof(struct index_header) || + entries_offset > index_length || + index_length > allocated_size || + allocated_size > index_size || + index_length - entries_offset < sizeof(struct index_entry_header)) + return false; + + return true; +} + +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; + case AT_INDEX_ROOT: + if (!ntfs_index_root_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 +845,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 +890,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 +1382,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 +1412,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