* [PATCH 0/3] fs/ntfs3: Refactoring and bugfix
@ 2022-05-27 14:15 Almaz Alexandrovich
2022-05-27 14:21 ` [PATCH 1/3] fs/ntfs3: Refactoring of indx_find function Almaz Alexandrovich
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Almaz Alexandrovich @ 2022-05-27 14:15 UTC (permalink / raw)
To: ntfs3; +Cc: linux-kernel, linux-fsdevel
There are 3 commits.
First - to improve code readability.
Second - bugfix for xfstest (fixing wrong logic).
Third - restructuring function logic so
we can restore inode in some error cases.
Konstantin Komarov (3):
fs/ntfs3: Refactoring of indx_find function
fs/ntfs3: Fix double free on remount
fs/ntfs3: Refactor ni_try_remove_attr_list function
fs/ntfs3/frecord.c | 49 ++++++++++++++++++++++++++++++++++------------
fs/ntfs3/index.c | 20 ++++++-------------
fs/ntfs3/record.c | 5 ++---
fs/ntfs3/super.c | 8 ++++----
4 files changed, 49 insertions(+), 33 deletions(-)
--
2.36.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] fs/ntfs3: Refactoring of indx_find function
2022-05-27 14:15 [PATCH 0/3] fs/ntfs3: Refactoring and bugfix Almaz Alexandrovich
@ 2022-05-27 14:21 ` Almaz Alexandrovich
2022-05-27 16:07 ` Joe Perches
2022-05-28 0:06 ` Dave Chinner
2022-05-27 14:22 ` [PATCH 2/3] fs/ntfs3: Fix double free on remount Almaz Alexandrovich
2022-05-27 14:22 ` [PATCH 3/3] fs/ntfs3: Refactor ni_try_remove_attr_list function Almaz Alexandrovich
2 siblings, 2 replies; 8+ messages in thread
From: Almaz Alexandrovich @ 2022-05-27 14:21 UTC (permalink / raw)
To: ntfs3; +Cc: linux-kernel, linux-fsdevel
This commit makes function a bit more readable
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
fs/ntfs3/index.c | 20 ++++++--------------
1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c
index 6f81e3a49abf..511f872b6650 100644
--- a/fs/ntfs3/index.c
+++ b/fs/ntfs3/index.c
@@ -1042,19 +1042,16 @@ int indx_find(struct ntfs_index *indx, struct ntfs_inode *ni,
{
int err;
struct NTFS_DE *e;
- const struct INDEX_HDR *hdr;
struct indx_node *node;
if (!root)
root = indx_get_root(&ni->dir, ni, NULL, NULL);
if (!root) {
- err = -EINVAL;
- goto out;
+ /* Should not happed. */
+ return -EINVAL;
}
- hdr = &root->ihdr;
-
/* Check cache. */
e = fnd->level ? fnd->de[fnd->level - 1] : fnd->root_de;
if (e && !de_is_last(e) &&
@@ -1068,39 +1065,34 @@ int indx_find(struct ntfs_index *indx, struct ntfs_inode *ni,
fnd_clear(fnd);
/* Lookup entry that is <= to the search value. */
- e = hdr_find_e(indx, hdr, key, key_len, ctx, diff);
+ e = hdr_find_e(indx, &root->ihdr, key, key_len, ctx, diff);
if (!e)
return -EINVAL;
fnd->root_de = e;
- err = 0;
for (;;) {
node = NULL;
if (*diff >= 0 || !de_has_vcn_ex(e)) {
*entry = e;
- goto out;
+ return 0;
}
/* Read next level. */
err = indx_read(indx, ni, de_get_vbn(e), &node);
if (err)
- goto out;
+ return err;
/* Lookup entry that is <= to the search value. */
e = hdr_find_e(indx, &node->index->ihdr, key, key_len, ctx,
diff);
if (!e) {
- err = -EINVAL;
put_indx_node(node);
- goto out;
+ return -EINVAL;
}
fnd_push(fnd, node, e);
}
-
-out:
- return err;
}
int indx_find_sort(struct ntfs_index *indx, struct ntfs_inode *ni,
--
2.36.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] fs/ntfs3: Fix double free on remount
2022-05-27 14:15 [PATCH 0/3] fs/ntfs3: Refactoring and bugfix Almaz Alexandrovich
2022-05-27 14:21 ` [PATCH 1/3] fs/ntfs3: Refactoring of indx_find function Almaz Alexandrovich
@ 2022-05-27 14:22 ` Almaz Alexandrovich
2022-05-27 14:22 ` [PATCH 3/3] fs/ntfs3: Refactor ni_try_remove_attr_list function Almaz Alexandrovich
2 siblings, 0 replies; 8+ messages in thread
From: Almaz Alexandrovich @ 2022-05-27 14:22 UTC (permalink / raw)
To: ntfs3; +Cc: linux-kernel, linux-fsdevel
Pointer to options was freed twice on remount
Fixes xfstest generic/361
Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block")
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
fs/ntfs3/super.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index d41d76979e12..697a84ed395e 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -30,6 +30,7 @@
#include <linux/fs_context.h>
#include <linux/fs_parser.h>
#include <linux/log2.h>
+#include <linux/minmax.h>
#include <linux/module.h>
#include <linux/nls.h>
#include <linux/seq_file.h>
@@ -390,7 +391,7 @@ static int ntfs_fs_reconfigure(struct fs_context *fc)
return -EINVAL;
}
- memcpy(sbi->options, new_opts, sizeof(*new_opts));
+ swap(sbi->options, fc->fs_private);
return 0;
}
@@ -897,6 +898,8 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
ref.high = 0;
sbi->sb = sb;
+ sbi->options = fc->fs_private;
+ fc->fs_private = NULL;
sb->s_flags |= SB_NODIRATIME;
sb->s_magic = 0x7366746e; // "ntfs"
sb->s_op = &ntfs_sops;
@@ -1260,8 +1263,6 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
goto put_inode_out;
}
- fc->fs_private = NULL;
-
return 0;
put_inode_out:
@@ -1414,7 +1415,6 @@ static int ntfs_init_fs_context(struct fs_context *fc)
mutex_init(&sbi->compress.mtx_lzx);
#endif
- sbi->options = opts;
fc->s_fs_info = sbi;
ok:
fc->fs_private = opts;
--
2.36.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] fs/ntfs3: Refactor ni_try_remove_attr_list function
2022-05-27 14:15 [PATCH 0/3] fs/ntfs3: Refactoring and bugfix Almaz Alexandrovich
2022-05-27 14:21 ` [PATCH 1/3] fs/ntfs3: Refactoring of indx_find function Almaz Alexandrovich
2022-05-27 14:22 ` [PATCH 2/3] fs/ntfs3: Fix double free on remount Almaz Alexandrovich
@ 2022-05-27 14:22 ` Almaz Alexandrovich
2 siblings, 0 replies; 8+ messages in thread
From: Almaz Alexandrovich @ 2022-05-27 14:22 UTC (permalink / raw)
To: ntfs3; +Cc: linux-kernel, linux-fsdevel
Now we save a copy of primary record for restoration.
Also now we remove all attributes from subrecords.
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
fs/ntfs3/frecord.c | 49 ++++++++++++++++++++++++++++++++++------------
fs/ntfs3/record.c | 5 ++---
2 files changed, 39 insertions(+), 15 deletions(-)
diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c
index 18842998c8fa..3576268ee0a1 100644
--- a/fs/ntfs3/frecord.c
+++ b/fs/ntfs3/frecord.c
@@ -7,6 +7,7 @@
#include <linux/fiemap.h>
#include <linux/fs.h>
+#include <linux/minmax.h>
#include <linux/vmalloc.h>
#include "debug.h"
@@ -649,6 +650,7 @@ static int ni_try_remove_attr_list(struct ntfs_inode *ni)
struct mft_inode *mi;
u32 asize, free;
struct MFT_REF ref;
+ struct MFT_REC *mrec;
__le16 id;
if (!ni->attr_list.dirty)
@@ -692,11 +694,17 @@ static int ni_try_remove_attr_list(struct ntfs_inode *ni)
free -= asize;
}
+ /* Make a copy of primary record to restore if error. */
+ mrec = kmemdup(ni->mi.mrec, sbi->record_size, GFP_NOFS);
+ if (!mrec)
+ return 0; /* Not critical. */
+
/* It seems that attribute list can be removed from primary record. */
mi_remove_attr(NULL, &ni->mi, attr_list);
/*
- * Repeat the cycle above and move all attributes to primary record.
+ * Repeat the cycle above and copy all attributes to primary record.
+ * Do not remove original attributes from subrecords!
* It should be success!
*/
le = NULL;
@@ -707,14 +715,14 @@ static int ni_try_remove_attr_list(struct ntfs_inode *ni)
mi = ni_find_mi(ni, ino_get(&le->ref));
if (!mi) {
/* Should never happened, 'cause already checked. */
- goto bad;
+ goto out;
}
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;
+ goto out;
}
asize = le32_to_cpu(attr->size);
@@ -724,18 +732,33 @@ static int ni_try_remove_attr_list(struct ntfs_inode *ni)
le16_to_cpu(attr->name_off));
if (!attr_ins) {
/*
- * Internal error.
- * Either no space in primary record (already checked).
- * Either tried to insert another
- * non indexed attribute (logic error).
+ * No space in primary record (already checked).
*/
- goto bad;
+ goto out;
}
/* Copy all except id. */
id = attr_ins->id;
memcpy(attr_ins, attr, asize);
attr_ins->id = id;
+ }
+
+ /*
+ * Repeat the cycle above and remove all attributes from subrecords.
+ */
+ le = NULL;
+ while ((le = al_enumerate(ni, le))) {
+ if (!memcmp(&le->ref, &ref, sizeof(ref)))
+ continue;
+
+ mi = ni_find_mi(ni, ino_get(&le->ref));
+ if (!mi)
+ continue;
+
+ attr = mi_find_attr(mi, NULL, le->type, le_name(le),
+ le->name_len, &le->id);
+ if (!attr)
+ continue;
/* Remove from original record. */
mi_remove_attr(NULL, mi, attr);
@@ -748,11 +771,13 @@ static int ni_try_remove_attr_list(struct ntfs_inode *ni)
ni->attr_list.le = NULL;
ni->attr_list.dirty = false;
+ kfree(mrec);
+ return 0;
+out:
+ /* Restore primary record. */
+ swap(mrec, ni->mi.mrec);
+ kfree(mrec);
return 0;
-bad:
- ntfs_inode_err(&ni->vfs_inode, "Internal error");
- make_bad_inode(&ni->vfs_inode);
- return -EINVAL;
}
/*
diff --git a/fs/ntfs3/record.c b/fs/ntfs3/record.c
index 861e35791506..8fe0a876400a 100644
--- a/fs/ntfs3/record.c
+++ b/fs/ntfs3/record.c
@@ -445,12 +445,11 @@ struct ATTRIB *mi_insert_attr(struct mft_inode *mi, enum ATTR_TYPE type,
attr = NULL;
while ((attr = mi_enum_attr(mi, attr))) {
diff = compare_attr(attr, type, name, name_len, upcase);
- if (diff > 0)
- break;
+
if (diff < 0)
continue;
- if (!is_attr_indexed(attr))
+ if (!diff && !is_attr_indexed(attr))
return NULL;
break;
}
--
2.36.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] fs/ntfs3: Refactoring of indx_find function
2022-05-27 14:21 ` [PATCH 1/3] fs/ntfs3: Refactoring of indx_find function Almaz Alexandrovich
@ 2022-05-27 16:07 ` Joe Perches
2022-05-30 17:11 ` Konstantin Komarov
2022-05-28 0:06 ` Dave Chinner
1 sibling, 1 reply; 8+ messages in thread
From: Joe Perches @ 2022-05-27 16:07 UTC (permalink / raw)
To: Almaz Alexandrovich, ntfs3; +Cc: linux-kernel, linux-fsdevel
On Fri, 2022-05-27 at 17:21 +0300, Almaz Alexandrovich wrote:
> This commit makes function a bit more readable
trivia:
> diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c
[]
> @@ -1042,19 +1042,16 @@ int indx_find(struct ntfs_index *indx, struct ntfs_inode *ni,
> {
> int err;
> struct NTFS_DE *e;
> - const struct INDEX_HDR *hdr;
> struct indx_node *node;
>
> if (!root)
> root = indx_get_root(&ni->dir, ni, NULL, NULL);
>
> if (!root) {
> - err = -EINVAL;
> - goto out;
> + /* Should not happed. */
> + return -EINVAL;
s/happed/happen/
> for (;;) {
> node = NULL;
> if (*diff >= 0 || !de_has_vcn_ex(e)) {
> *entry = e;
> - goto out;
> + return 0;
> }
might be nicer with a break; or a while like
while (*diff < 0 && de_has_vcn_ex(e)) {
node = NULL;
> /* Read next level. */
> err = indx_read(indx, ni, de_get_vbn(e), &node);
> if (err)
> - goto out;
> + return err;
>
> /* Lookup entry that is <= to the search value. */
> e = hdr_find_e(indx, &node->index->ihdr, key, key_len, ctx,
> diff);
> if (!e) {
> - err = -EINVAL;
> put_indx_node(node);
> - goto out;
> + return -EINVAL;
> }
>
> fnd_push(fnd, node, e);
> }
> -
> -out:
> - return err;
and a return 0;
or
*entry = e;
return 0;
so it appears that the function has a typical return value.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] fs/ntfs3: Refactoring of indx_find function
2022-05-27 14:21 ` [PATCH 1/3] fs/ntfs3: Refactoring of indx_find function Almaz Alexandrovich
2022-05-27 16:07 ` Joe Perches
@ 2022-05-28 0:06 ` Dave Chinner
2022-05-30 16:15 ` Konstantin Komarov
1 sibling, 1 reply; 8+ messages in thread
From: Dave Chinner @ 2022-05-28 0:06 UTC (permalink / raw)
To: Almaz Alexandrovich; +Cc: ntfs3, linux-kernel, linux-fsdevel
On Fri, May 27, 2022 at 05:21:03PM +0300, Almaz Alexandrovich wrote:
> This commit makes function a bit more readable
>
> Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
This looks wrong. The email is from
"From: Almaz Alexandrovich <almaz.alexandrovich@paragon-software.com>"
So it looks like the S-o-B has the wrong email address in it. All
the patches have this same problem.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] fs/ntfs3: Refactoring of indx_find function
2022-05-28 0:06 ` Dave Chinner
@ 2022-05-30 16:15 ` Konstantin Komarov
0 siblings, 0 replies; 8+ messages in thread
From: Konstantin Komarov @ 2022-05-30 16:15 UTC (permalink / raw)
To: Dave Chinner; +Cc: ntfs3, linux-kernel, linux-fsdevel
This was caused by an incorrect account setting in Thunderbird.
Thanks for catching this.
On 5/28/22 03:06, Dave Chinner wrote:
> On Fri, May 27, 2022 at 05:21:03PM +0300, Almaz Alexandrovich wrote:
>> This commit makes function a bit more readable
>>
>> Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
>
> This looks wrong. The email is from
>
> "From: Almaz Alexandrovich <almaz.alexandrovich@paragon-software.com>"
>
> So it looks like the S-o-B has the wrong email address in it. All
> the patches have this same problem.
>
> Cheers,
>
> Dave.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] fs/ntfs3: Refactoring of indx_find function
2022-05-27 16:07 ` Joe Perches
@ 2022-05-30 17:11 ` Konstantin Komarov
0 siblings, 0 replies; 8+ messages in thread
From: Konstantin Komarov @ 2022-05-30 17:11 UTC (permalink / raw)
To: Joe Perches, ntfs3; +Cc: linux-kernel, linux-fsdevel
Hello.
Thanks for your input.
It's nice to have a clear typical return value, so I've updated patch.
On 5/27/22 19:07, Joe Perches wrote:
> On Fri, 2022-05-27 at 17:21 +0300, Almaz Alexandrovich wrote:
>> This commit makes function a bit more readable
>
> trivia:
>
>> diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c
> []
>> @@ -1042,19 +1042,16 @@ int indx_find(struct ntfs_index *indx, struct ntfs_inode *ni,
>> {
>> int err;
>> struct NTFS_DE *e;
>> - const struct INDEX_HDR *hdr;
>> struct indx_node *node;
>>
>> if (!root)
>> root = indx_get_root(&ni->dir, ni, NULL, NULL);
>>
>> if (!root) {
>> - err = -EINVAL;
>> - goto out;
>> + /* Should not happed. */
>> + return -EINVAL;
>
> s/happed/happen/
>
>> for (;;) {
>> node = NULL;
>> if (*diff >= 0 || !de_has_vcn_ex(e)) {
>> *entry = e;
>> - goto out;
>> + return 0;
>> }
>
> might be nicer with a break; or a while like
>
> while (*diff < 0 && de_has_vcn_ex(e)) {
> node = NULL;
>
>
>> /* Read next level. */
>> err = indx_read(indx, ni, de_get_vbn(e), &node);
>> if (err)
>> - goto out;
>> + return err;
>>
>> /* Lookup entry that is <= to the search value. */
>> e = hdr_find_e(indx, &node->index->ihdr, key, key_len, ctx,
>> diff);
>> if (!e) {
>> - err = -EINVAL;
>> put_indx_node(node);
>> - goto out;
>> + return -EINVAL;
>> }
>>
>> fnd_push(fnd, node, e);
>> }
>> -
>> -out:
>> - return err;
>
> and a return 0;
>
> or
> *entry = e;
> return 0;
>
> so it appears that the function has a typical return value.
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-05-30 17:11 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-27 14:15 [PATCH 0/3] fs/ntfs3: Refactoring and bugfix Almaz Alexandrovich
2022-05-27 14:21 ` [PATCH 1/3] fs/ntfs3: Refactoring of indx_find function Almaz Alexandrovich
2022-05-27 16:07 ` Joe Perches
2022-05-30 17:11 ` Konstantin Komarov
2022-05-28 0:06 ` Dave Chinner
2022-05-30 16:15 ` Konstantin Komarov
2022-05-27 14:22 ` [PATCH 2/3] fs/ntfs3: Fix double free on remount Almaz Alexandrovich
2022-05-27 14:22 ` [PATCH 3/3] fs/ntfs3: Refactor ni_try_remove_attr_list function Almaz Alexandrovich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).