<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
</head>
<body style="overflow-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;">
Hi,<br>
<div><br>
<blockquote type="cite">
<div>On 5 May 2023, at 07:20, Namjae Jeon <linkinjeon@kernel.org> wrote:</div>
<div>
<div>2023-04-08 4:44 GMT+09:00, Danila Chernetsov <listdansp@mail.ru>:<br>
<blockquote type="cite">In ntfs_mft_data_extend_allocation_nolock(), if an error condition occurs<br>
prior to 'ctx' being set to a non-NULL value, avoid dereferencing the NULL<br>
'ctx' pointer in error handling.<br>
<br>
Found by Linux Verification Center (linuxtesting.org) with SVACE.<br>
<br>
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")<br>
Signed-off-by: Danila Chernetsov <listdansp@mail.ru><br>
---<br>
fs/ntfs/mft.c | 38 +++++++++++++++++++++-----------------<br>
1 file changed, 21 insertions(+), 17 deletions(-)<br>
<br>
diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c<br>
index 48030899dc6e..e1126ce6f8ec 100644<br>
--- a/fs/ntfs/mft.c<br>
+++ b/fs/ntfs/mft.c<br>
@@ -1955,36 +1955,40 @@ static int<br>
ntfs_mft_data_extend_allocation_nolock(ntfs_volume *vol)<br>
<span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>"attribute.%s", es);<br>
<span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>NVolSetErrors(vol);<br>
<span class="Apple-tab-span" style="white-space:pre"></span>}<br>
-<span class="Apple-tab-span" style="white-space:pre"> </span>a = ctx->attr;<br>
+<span class="Apple-tab-span" style="white-space:pre"> </span><br>
<span class="Apple-tab-span" style="white-space:pre"></span>if (ntfs_rl_truncate_nolock(vol, &mft_ni->runlist, old_last_vcn)) {<br>
<span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>ntfs_error(vol->sb, "Failed to truncate mft data attribute "<br>
<span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>"runlist.%s", es);<br>
<span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>NVolSetErrors(vol);<br>
<span class="Apple-tab-span" style="white-space:pre"></span>}<br>
-<span class="Apple-tab-span" style="white-space:pre"> </span>if (mp_rebuilt && !IS_ERR(ctx->mrec)) {<br>
-<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>if (ntfs_mapping_pairs_build(vol, (u8*)a + le16_to_cpu(<br>
+<span class="Apple-tab-span" style="white-space:pre"> </span>if (ctx) {<br>
+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>a = ctx->attr;<br>
+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>if (mp_rebuilt && !IS_ERR(ctx->mrec)) {<br>
+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>if (ntfs_mapping_pairs_build(vol, (u8*)a + le16_to_cpu(<br>
<span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>a->data.non_resident.mapping_pairs_offset),<br>
<span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>old_alen - le16_to_cpu(<br>
-<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>a->data.non_resident.mapping_pairs_offset),<br>
+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>a->data.non_resident.mapping_pairs_offset),<br>
<span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>rl2, ll, -1, NULL))
{<br>
-<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>ntfs_error(vol->sb, "Failed to restore mapping pairs "<br>
+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>ntfs_error(vol->sb,
"Failed to restore mapping pairs "<br>
<span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>"array.%s",
es);<br>
-<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>NVolSetErrors(vol);<br>
-<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>}<br>
-<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>if (ntfs_attr_record_resize(ctx->mrec, a, old_alen)) {<br>
-<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>ntfs_error(vol->sb, "Failed to restore attribute "<br>
+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>NVolSetErrors(vol);<br>
+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>}<br>
+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>if (ntfs_attr_record_resize(ctx->mrec, a, old_alen)) {<br>
+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>ntfs_error(vol->sb,
"Failed to restore attribute "<br>
<span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>"record.%s",
es);<br>
-<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>NVolSetErrors(vol);<br>
+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>NVolSetErrors(vol);<br>
+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>}<br>
+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>flush_dcache_mft_record_page(ctx->ntfs_ino);<br>
+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>mark_mft_record_dirty(ctx->ntfs_ino);<br>
<span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>}<br>
-<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>flush_dcache_mft_record_page(ctx->ntfs_ino);<br>
-<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>mark_mft_record_dirty(ctx->ntfs_ino);<br>
-<span class="Apple-tab-span" style="white-space:pre"> </span>} else if (IS_ERR(ctx->mrec)) {<br>
-<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>ntfs_error(vol->sb, "Failed to restore attribute search "<br>
+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>else if (IS_ERR(ctx->mrec)) {<br>
+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>ntfs_error(vol->sb, "Failed to restore attribute search "<br>
<span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>"context.%s", es);<br>
-<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>NVolSetErrors(vol);<br>
+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>NVolSetErrors(vol);<br>
+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>}<br>
+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>if (ctx)<br>
</blockquote>
I think that this check is not needed.<br>
</div>
</div>
</blockquote>
<div><br>
</div>
<div>That is correct. It doesn't do any harm but yes ctx cannot be NULL or it would have crashed above when it was dereferenced.</div>
<div><br>
</div>
<div>Best regards,</div>
<div><br>
</div>
<div><span class="Apple-tab-span" style="white-space:pre"></span>Anton</div>
<br>
<blockquote type="cite">
<div>
<div>
<blockquote type="cite">+<span class="Apple-tab-span" style="white-space:pre"> </span>
<span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>ntfs_attr_put_search_ctx(ctx);<br>
<span class="Apple-tab-span" style="white-space:pre"></span>}<br>
-<span class="Apple-tab-span" style="white-space:pre"> </span>if (ctx)<br>
-<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"></span>ntfs_attr_put_search_ctx(ctx);<br>
<span class="Apple-tab-span" style="white-space:pre"></span>if (!IS_ERR(mrec))<br>
<span class="Apple-tab-span" style="white-space:pre"></span><span class="Apple-tab-span" style="white-space:pre"></span>unmap_mft_record(mft_ni);<br>
<span class="Apple-tab-span" style="white-space:pre"></span>up_write(&mft_ni->runlist.lock);<br>
--<br>
2.25.1<br>
<br>
<br>
</blockquote>
</div>
</div>
</blockquote>
</div>
<br>
<div>
<div>-- <br>
Anton Altaparmakov <anton at tuxera.com> (replace at with @)<br>
Lead in File System Development, Tuxera Inc., http://www.tuxera.com/<br>
Linux NTFS maintainer</div>
</div>
<br>
</body>
</html>