All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mohammad Rasim <mohammad.rasim96@gmail.com>
To: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
Cc: ntfs3@lists.linux.dev, linux-kernel@vger.kernel.org,
	linux-fsdevel@vger.kernel.org,
	Kari Argillander <kari.argillander@gmail.com>
Subject: Re: [PATCH] fs/ntfs3: Check for NULL if ATTR_EA_INFO is incorrect
Date: Mon, 11 Oct 2021 23:56:22 +0300	[thread overview]
Message-ID: <0021ec0c-737a-398f-53ca-8daa284744b6@gmail.com> (raw)
In-Reply-To: <7e5b8dc9-9989-0e8a-9e8d-ae26b6e74df4@paragon-software.com>


On 10/11/21 19:55, Konstantin Komarov wrote:
> Hello.
>
> Presumably we found the code, that panics.
> But it panics in place, where pointer must be always not NULL.
> Please try patch provided below.
> If it helps (there is no panic), then check dmesg for
> message "Looks like internal error".
> And please compare copied folders.
> This way it will be clear what file / folder cause this logic error.
>
> Thanks for all your help so far.

Ok,

This helped, unfortunately the error is sporadic and i can't easily 
track down which file caused the crash .

In one test it seemd it was caused by files in three directories 
"package", "system" , "support" (all these directories are from the 
"buildroot" tree, most of the files that failed to copy were symlinks, 
don't know if that makes a difference)  but after rebooting and loading 
the unpatched ntfs3.ko i was able to copy these files without a crash!

It seems that the crash happens when copying large number of files so 
even a failed file can be copied if it was copied alone (I might be very 
wrong in my conclusion here)

anyways, i did multiple tests. in the first a few it copied without a 
crash and skipped a few files( the dmesg didn't contain the "Looks like 
internal error" message).

on subsequent tests i did get that message like so:

[  186.295722] ntfs3: sdb1: ino=1a, Looks like internal error
[  186.296219] ntfs3: sdb1: ntfs3_write_inode r=1a failed, -22

That "ino=1a" looks wrong to me !

  I will try to do more tests if i can but it's a bit annoying because 
each crash causes the file system to be corrupted and "ntfsfix" can't 
fix these errors so i have to reboot to windows os to be able to use 
"chkdsk" to fix the filesystem before doing the next test.

It would be nice if Paragon  releases "fsck.ntfs" that works well in 
these situations so we don't need to boot to windows to fix them


Regards


>
> [PATCH] fs/ntfs3: Check for NULL pointers in ni_try_remove_attr_list
>
> All these checks must be redundant.
> If this commit helps, then there is bug in code.
>
> Signed-off-by: Konstantin 
> Komarov<almaz.alexandrovich@paragon-software.com>
> ---
> fs/ntfs3/frecord.c | 23 ++++++++++++++++++++++-
> 1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c
> index ecb965e4afd0..37e19fe7d496 100644
> --- a/fs/ntfs3/frecord.c
> +++ b/fs/ntfs3/frecord.c
> @@ -705,18 +705,35 @@ static int ni_try_remove_attr_list(struct 
> ntfs_inode *ni)
> continue;
> mi = ni_find_mi(ni, ino_get(&le->ref));
> + if (!mi) {
> + /* Should never happened, 'cause already checked. */
> + goto bad;
> + }
> attr = mi_find_attr(mi, NULL, le->type, le_name(le),
> le->name_len, &le->id);
> + if (!attr) {
> + /* Should never happened, 'cause already checked. */
> + goto bad;
> + }
> asize = le32_to_cpu(attr->size);
> /* Insert into primary record. */
> attr_ins = mi_insert_attr(&ni->mi, le->type, le_name(le),
> le->name_len, asize,
> le16_to_cpu(attr->name_off));
> - id = attr_ins->id;
> + if (!attr_ins) {
> + /*
> + * Internal error.
> + * Either no space in primary record (already checked).
> + * Either tried to insert another
> + * non indexed attribute (logic error).
> + */
> + goto bad;
> + }
> /* Copy all except id. */
> + id = attr_ins->id;
> memcpy(attr_ins, attr, asize);
> attr_ins->id = id;
> @@ -732,6 +749,10 @@ static int ni_try_remove_attr_list(struct 
> ntfs_inode *ni)
> ni->attr_list.dirty = false;
> return 0;
> +bad:
> + ntfs_inode_err(&ni->vfs_inode, "Looks like internal error");
> + make_bad_inode(&ni->vfs_inode);
> + return -EINVAL;
> }
> /*

  reply	other threads:[~2021-10-11 20:56 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-29 16:35 Konstantin Komarov
2021-09-29 17:44 ` Kari Argillander
2021-10-03 17:50 ` Kari Argillander
2021-10-04 20:39   ` Mohammad Rasim
2021-10-06 14:47     ` Konstantin Komarov
2021-10-06 17:42       ` Mohammad Rasim
2021-10-11 16:55         ` Konstantin Komarov
2021-10-11 20:56           ` Mohammad Rasim [this message]
2021-10-06 14:51   ` Konstantin Komarov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0021ec0c-737a-398f-53ca-8daa284744b6@gmail.com \
    --to=mohammad.rasim96@gmail.com \
    --cc=almaz.alexandrovich@paragon-software.com \
    --cc=kari.argillander@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ntfs3@lists.linux.dev \
    --subject='Re: [PATCH] fs/ntfs3: Check for NULL if ATTR_EA_INFO is incorrect' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.