fs/ntfs/mft.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-)
In ntfs_mft_data_extend_allocation_nolock(), if an error condition occurs
prior to 'ctx' being set to a non-NULL value, avoid dereferencing the NULL
'ctx' pointer in error handling.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Danila Chernetsov <listdansp@mail.ru>
---
fs/ntfs/mft.c | 38 +++++++++++++++++++++-----------------
1 file changed, 21 insertions(+), 17 deletions(-)
diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c
index 48030899dc6e..e1126ce6f8ec 100644
--- a/fs/ntfs/mft.c
+++ b/fs/ntfs/mft.c
@@ -1955,36 +1955,40 @@ static int ntfs_mft_data_extend_allocation_nolock(ntfs_volume *vol)
"attribute.%s", es);
NVolSetErrors(vol);
}
- a = ctx->attr;
+
if (ntfs_rl_truncate_nolock(vol, &mft_ni->runlist, old_last_vcn)) {
ntfs_error(vol->sb, "Failed to truncate mft data attribute "
"runlist.%s", es);
NVolSetErrors(vol);
}
- if (mp_rebuilt && !IS_ERR(ctx->mrec)) {
- if (ntfs_mapping_pairs_build(vol, (u8*)a + le16_to_cpu(
+ if (ctx) {
+ a = ctx->attr;
+ if (mp_rebuilt && !IS_ERR(ctx->mrec)) {
+ if (ntfs_mapping_pairs_build(vol, (u8*)a + le16_to_cpu(
a->data.non_resident.mapping_pairs_offset),
old_alen - le16_to_cpu(
- a->data.non_resident.mapping_pairs_offset),
+ a->data.non_resident.mapping_pairs_offset),
rl2, ll, -1, NULL)) {
- ntfs_error(vol->sb, "Failed to restore mapping pairs "
+ ntfs_error(vol->sb, "Failed to restore mapping pairs "
"array.%s", es);
- NVolSetErrors(vol);
- }
- if (ntfs_attr_record_resize(ctx->mrec, a, old_alen)) {
- ntfs_error(vol->sb, "Failed to restore attribute "
+ NVolSetErrors(vol);
+ }
+ if (ntfs_attr_record_resize(ctx->mrec, a, old_alen)) {
+ ntfs_error(vol->sb, "Failed to restore attribute "
"record.%s", es);
- NVolSetErrors(vol);
+ NVolSetErrors(vol);
+ }
+ flush_dcache_mft_record_page(ctx->ntfs_ino);
+ mark_mft_record_dirty(ctx->ntfs_ino);
}
- flush_dcache_mft_record_page(ctx->ntfs_ino);
- mark_mft_record_dirty(ctx->ntfs_ino);
- } else if (IS_ERR(ctx->mrec)) {
- ntfs_error(vol->sb, "Failed to restore attribute search "
+ else if (IS_ERR(ctx->mrec)) {
+ ntfs_error(vol->sb, "Failed to restore attribute search "
"context.%s", es);
- NVolSetErrors(vol);
+ NVolSetErrors(vol);
+ }
+ if (ctx)
+ ntfs_attr_put_search_ctx(ctx);
}
- if (ctx)
- ntfs_attr_put_search_ctx(ctx);
if (!IS_ERR(mrec))
unmap_mft_record(mft_ni);
up_write(&mft_ni->runlist.lock);
--
2.25.12023-04-08 4:44 GMT+09:00, Danila Chernetsov <listdansp@mail.ru>:
> In ntfs_mft_data_extend_allocation_nolock(), if an error condition occurs
> prior to 'ctx' being set to a non-NULL value, avoid dereferencing the NULL
> 'ctx' pointer in error handling.
Please check the warnings from checkpatch.pl.
ERROR: trailing whitespace
#107: FILE: fs/ntfs/mft.c:1958:
+^I$
ERROR: "(foo*)" should be "(foo *)"
#118: FILE: fs/ntfs/mft.c:1967:
+ if (ntfs_mapping_pairs_build(vol, (u8*)a + le16_to_cpu(
ERROR: else should follow close brace '}'
#146: FILE: fs/ntfs/mft.c:1984:
}
+ else if (IS_ERR(ctx->mrec)) {
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Danila Chernetsov <listdansp@mail.ru>
> ---
> fs/ntfs/mft.c | 38 +++++++++++++++++++++-----------------
> 1 file changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c
> index 48030899dc6e..e1126ce6f8ec 100644
> --- a/fs/ntfs/mft.c
> +++ b/fs/ntfs/mft.c
> @@ -1955,36 +1955,40 @@ static int
> ntfs_mft_data_extend_allocation_nolock(ntfs_volume *vol)
> "attribute.%s", es);
> NVolSetErrors(vol);
> }
> - a = ctx->attr;
> +
> if (ntfs_rl_truncate_nolock(vol, &mft_ni->runlist, old_last_vcn)) {
> ntfs_error(vol->sb, "Failed to truncate mft data attribute "
> "runlist.%s", es);
> NVolSetErrors(vol);
> }
> - if (mp_rebuilt && !IS_ERR(ctx->mrec)) {
> - if (ntfs_mapping_pairs_build(vol, (u8*)a + le16_to_cpu(
> + if (ctx) {
> + a = ctx->attr;
> + if (mp_rebuilt && !IS_ERR(ctx->mrec)) {
> + if (ntfs_mapping_pairs_build(vol, (u8*)a + le16_to_cpu(
> a->data.non_resident.mapping_pairs_offset),
> old_alen - le16_to_cpu(
> - a->data.non_resident.mapping_pairs_offset),
> + a->data.non_resident.mapping_pairs_offset),
> rl2, ll, -1, NULL)) {
> - ntfs_error(vol->sb, "Failed to restore mapping pairs "
> + ntfs_error(vol->sb, "Failed to restore mapping pairs "
> "array.%s", es);
> - NVolSetErrors(vol);
> - }
> - if (ntfs_attr_record_resize(ctx->mrec, a, old_alen)) {
> - ntfs_error(vol->sb, "Failed to restore attribute "
> + NVolSetErrors(vol);
> + }
> + if (ntfs_attr_record_resize(ctx->mrec, a, old_alen)) {
> + ntfs_error(vol->sb, "Failed to restore attribute "
> "record.%s", es);
> - NVolSetErrors(vol);
> + NVolSetErrors(vol);
> + }
> + flush_dcache_mft_record_page(ctx->ntfs_ino);
> + mark_mft_record_dirty(ctx->ntfs_ino);
> }
> - flush_dcache_mft_record_page(ctx->ntfs_ino);
> - mark_mft_record_dirty(ctx->ntfs_ino);
> - } else if (IS_ERR(ctx->mrec)) {
> - ntfs_error(vol->sb, "Failed to restore attribute search "
> + else if (IS_ERR(ctx->mrec)) {
> + ntfs_error(vol->sb, "Failed to restore attribute search "
> "context.%s", es);
> - NVolSetErrors(vol);
> + NVolSetErrors(vol);
> + }
> + if (ctx)
> + ntfs_attr_put_search_ctx(ctx);
> }
> - if (ctx)
> - ntfs_attr_put_search_ctx(ctx);
> if (!IS_ERR(mrec))
> unmap_mft_record(mft_ni);
> up_write(&mft_ni->runlist.lock);
> --
> 2.25.1
>
>
2023-04-08 4:44 GMT+09:00, Danila Chernetsov <listdansp@mail.ru>:
> In ntfs_mft_data_extend_allocation_nolock(), if an error condition occurs
> prior to 'ctx' being set to a non-NULL value, avoid dereferencing the NULL
> 'ctx' pointer in error handling.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Danila Chernetsov <listdansp@mail.ru>
> ---
> fs/ntfs/mft.c | 38 +++++++++++++++++++++-----------------
> 1 file changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c
> index 48030899dc6e..e1126ce6f8ec 100644
> --- a/fs/ntfs/mft.c
> +++ b/fs/ntfs/mft.c
> @@ -1955,36 +1955,40 @@ static int
> ntfs_mft_data_extend_allocation_nolock(ntfs_volume *vol)
> "attribute.%s", es);
> NVolSetErrors(vol);
> }
> - a = ctx->attr;
> +
> if (ntfs_rl_truncate_nolock(vol, &mft_ni->runlist, old_last_vcn)) {
> ntfs_error(vol->sb, "Failed to truncate mft data attribute "
> "runlist.%s", es);
> NVolSetErrors(vol);
> }
> - if (mp_rebuilt && !IS_ERR(ctx->mrec)) {
> - if (ntfs_mapping_pairs_build(vol, (u8*)a + le16_to_cpu(
> + if (ctx) {
> + a = ctx->attr;
> + if (mp_rebuilt && !IS_ERR(ctx->mrec)) {
> + if (ntfs_mapping_pairs_build(vol, (u8*)a + le16_to_cpu(
> a->data.non_resident.mapping_pairs_offset),
> old_alen - le16_to_cpu(
> - a->data.non_resident.mapping_pairs_offset),
> + a->data.non_resident.mapping_pairs_offset),
> rl2, ll, -1, NULL)) {
> - ntfs_error(vol->sb, "Failed to restore mapping pairs "
> + ntfs_error(vol->sb, "Failed to restore mapping pairs "
> "array.%s", es);
> - NVolSetErrors(vol);
> - }
> - if (ntfs_attr_record_resize(ctx->mrec, a, old_alen)) {
> - ntfs_error(vol->sb, "Failed to restore attribute "
> + NVolSetErrors(vol);
> + }
> + if (ntfs_attr_record_resize(ctx->mrec, a, old_alen)) {
> + ntfs_error(vol->sb, "Failed to restore attribute "
> "record.%s", es);
> - NVolSetErrors(vol);
> + NVolSetErrors(vol);
> + }
> + flush_dcache_mft_record_page(ctx->ntfs_ino);
> + mark_mft_record_dirty(ctx->ntfs_ino);
> }
> - flush_dcache_mft_record_page(ctx->ntfs_ino);
> - mark_mft_record_dirty(ctx->ntfs_ino);
> - } else if (IS_ERR(ctx->mrec)) {
> - ntfs_error(vol->sb, "Failed to restore attribute search "
> + else if (IS_ERR(ctx->mrec)) {
> + ntfs_error(vol->sb, "Failed to restore attribute search "
> "context.%s", es);
> - NVolSetErrors(vol);
> + NVolSetErrors(vol);
> + }
> + if (ctx)
I think that this check is not needed.
> + ntfs_attr_put_search_ctx(ctx);
> }
> - if (ctx)
> - ntfs_attr_put_search_ctx(ctx);
> if (!IS_ERR(mrec))
> unmap_mft_record(mft_ni);
> up_write(&mft_ni->runlist.lock);
> --
> 2.25.1
>
>
On Fri, Apr 07, 2023 at 07:44:33PM +0000, Danila Chernetsov wrote: > In ntfs_mft_data_extend_allocation_nolock(), if an error condition occurs > prior to 'ctx' being set to a non-NULL value, avoid dereferencing the NULL > 'ctx' pointer in error handling. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. Duplicate of 10-years-old outstanding patch at [1]. I'm not speaking of the patch itself but rather on unfortunate state of ntfs subsystem. It seems like the maintainer is MIA (has not responding to patch submissions for a long time). Some trivial patches, however, are merged through mm tree. Konstantin (from newer ntfs3 subsystem), Andrew, would you like to take a look on this orphaned subsystem (and help reviewing)? I'd like to send MAINTAINERS update if it turns out to be the case. Thanks. [1]: https://lore.kernel.org/all/1358389709-20561-1-git-send-email-nickolai@csail.mit.edu/ -- An old man doll... just what I always wanted! - Clara
On Wed, 3 May 2023 11:16:02 +0700 Bagas Sanjaya <bagasdotme@gmail.com> wrote: > On Fri, Apr 07, 2023 at 07:44:33PM +0000, Danila Chernetsov wrote: > > In ntfs_mft_data_extend_allocation_nolock(), if an error condition occurs > > prior to 'ctx' being set to a non-NULL value, avoid dereferencing the NULL > > 'ctx' pointer in error handling. > > > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Duplicate of 10-years-old outstanding patch at [1]. Well, the patches are actually quite different. Is Danila's longer one better? > I'm not speaking of the patch itself but rather on unfortunate state > of ntfs subsystem. It seems like the maintainer is MIA (has not > responding to patch submissions for a long time). Some trivial > patches, however, are merged through mm tree. > > Konstantin (from newer ntfs3 subsystem), Andrew, would you like to take a > look on this orphaned subsystem (and help reviewing)? I'd like to send > MAINTAINERS update if it turns out to be the case. > Sure, I can join linux-ntfs-dev@lists.sourceforge.net and hendle things which come along. Or Christian may want to do that? > > [1]: https://lore.kernel.org/all/1358389709-20561-1-git-send-email-nickolai@csail.mit.edu/
2023-05-04 6:06 GMT+09:00, Andrew Morton <akpm@linux-foundation.org>: > On Wed, 3 May 2023 11:16:02 +0700 Bagas Sanjaya <bagasdotme@gmail.com> > wrote: > >> On Fri, Apr 07, 2023 at 07:44:33PM +0000, Danila Chernetsov wrote: >> > In ntfs_mft_data_extend_allocation_nolock(), if an error condition >> > occurs >> > prior to 'ctx' being set to a non-NULL value, avoid dereferencing the >> > NULL >> > 'ctx' pointer in error handling. >> > >> > Found by Linux Verification Center (linuxtesting.org) with SVACE. >> >> Duplicate of 10-years-old outstanding patch at [1]. > > Well, the patches are actually quite different. Is Danila's longer one > better? Danila's patch is better than old one. old patch doesn't free allocated cluster in error handling. And Christian will apply this to his queue if you drop this patch. > >> I'm not speaking of the patch itself but rather on unfortunate state >> of ntfs subsystem. It seems like the maintainer is MIA (has not >> responding to patch submissions for a long time). Some trivial >> patches, however, are merged through mm tree. >> >> Konstantin (from newer ntfs3 subsystem), Andrew, would you like to take a >> look on this orphaned subsystem (and help reviewing)? I'd like to send >> MAINTAINERS update if it turns out to be the case. >> > > Sure, I can join linux-ntfs-dev@lists.sourceforge.net and hendle things > which come along. > > Or Christian may want to do that? > >> >> [1]: >> https://lore.kernel.org/all/1358389709-20561-1-git-send-email-nickolai@csail.mit.edu/ > > >
On Wed, May 03, 2023 at 02:06:09PM -0700, Andrew Morton wrote: > On Wed, 3 May 2023 11:16:02 +0700 Bagas Sanjaya <bagasdotme@gmail.com> wrote: > > > On Fri, Apr 07, 2023 at 07:44:33PM +0000, Danila Chernetsov wrote: > > > In ntfs_mft_data_extend_allocation_nolock(), if an error condition occurs > > > prior to 'ctx' being set to a non-NULL value, avoid dereferencing the NULL > > > 'ctx' pointer in error handling. > > > > > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > > > Duplicate of 10-years-old outstanding patch at [1]. > > Well, the patches are actually quite different. Is Danila's longer one > better? > > > I'm not speaking of the patch itself but rather on unfortunate state > > of ntfs subsystem. It seems like the maintainer is MIA (has not > > responding to patch submissions for a long time). Some trivial > > patches, however, are merged through mm tree. > > > > Konstantin (from newer ntfs3 subsystem), Andrew, would you like to take a > > look on this orphaned subsystem (and help reviewing)? I'd like to send > > MAINTAINERS update if it turns out to be the case. > > > > Sure, I can join linux-ntfs-dev@lists.sourceforge.net and hendle things > which come along. > > Or Christian may want to do that? Thanks for asking. I would be able to pick up those patches into fs.misc. I got an off-list ping about someone also wanting to help with review apparently.
© 2016 - 2026 Red Hat, Inc.