2021-10-25 16:59:54

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 0/4] fs/ntfs3: Various fixes for xfstests problems

This series fixes generic/444 generic/092 generic/228 generic/240

Konstantin Komarov (4):
fs/ntfs3: In function ntfs_set_acl_ex do not change inode->i_mode if
called from function ntfs_init_acl
fs/ntfs3: Fix fiemap + fix shrink file size (to remove preallocated
space)
fs/ntfs3: Check new size for limits
fs/ntfs3: Update valid size if -EIOCBQUEUED

fs/ntfs3/file.c | 10 ++++++++--
fs/ntfs3/frecord.c | 10 +++++++---
fs/ntfs3/inode.c | 9 +++++++--
fs/ntfs3/xattr.c | 13 +++++++------
4 files changed, 29 insertions(+), 13 deletions(-)

--
2.33.0


2021-10-25 17:01:56

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 1/4] fs/ntfs3: In function ntfs_set_acl_ex do not change inode->i_mode if called from function ntfs_init_acl

ntfs_init_acl sets mode. ntfs_init_acl calls ntfs_set_acl_ex.
ntfs_set_acl_ex must not change this mode.
Fixes xfstest generic/444
Fixes: 83e8f5032e2d ("fs/ntfs3: Add attrib operations")

Signed-off-by: Konstantin Komarov <[email protected]>
---
fs/ntfs3/xattr.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
index 2143099cffdf..97b5f8417d85 100644
--- a/fs/ntfs3/xattr.c
+++ b/fs/ntfs3/xattr.c
@@ -538,7 +538,7 @@ struct posix_acl *ntfs_get_acl(struct inode *inode, int type)

static noinline int ntfs_set_acl_ex(struct user_namespace *mnt_userns,
struct inode *inode, struct posix_acl *acl,
- int type)
+ int type, int init_acl)
{
const char *name;
size_t size, name_len;
@@ -551,8 +551,9 @@ static noinline int ntfs_set_acl_ex(struct user_namespace *mnt_userns,

switch (type) {
case ACL_TYPE_ACCESS:
- if (acl) {
- umode_t mode = inode->i_mode;
+ /* Do not change i_mode if we are in init_acl */
+ if (acl && !init_acl) {
+ umode_t mode;

err = posix_acl_update_mode(mnt_userns, inode, &mode,
&acl);
@@ -613,7 +614,7 @@ static noinline int ntfs_set_acl_ex(struct user_namespace *mnt_userns,
int ntfs_set_acl(struct user_namespace *mnt_userns, struct inode *inode,
struct posix_acl *acl, int type)
{
- return ntfs_set_acl_ex(mnt_userns, inode, acl, type);
+ return ntfs_set_acl_ex(mnt_userns, inode, acl, type, 0);
}

/*
@@ -633,7 +634,7 @@ int ntfs_init_acl(struct user_namespace *mnt_userns, struct inode *inode,

if (default_acl) {
err = ntfs_set_acl_ex(mnt_userns, inode, default_acl,
- ACL_TYPE_DEFAULT);
+ ACL_TYPE_DEFAULT, 1);
posix_acl_release(default_acl);
} else {
inode->i_default_acl = NULL;
@@ -644,7 +645,7 @@ int ntfs_init_acl(struct user_namespace *mnt_userns, struct inode *inode,
else {
if (!err)
err = ntfs_set_acl_ex(mnt_userns, inode, acl,
- ACL_TYPE_ACCESS);
+ ACL_TYPE_ACCESS, 1);
posix_acl_release(acl);
}

--
2.33.0

2021-10-25 17:02:01

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 2/4] fs/ntfs3: Fix fiemap + fix shrink file size (to remove preallocated space)

Two problems:
1. ntfs3_setattr can't truncate preallocated space;
2. if allocated fragment "cross" valid size, then fragment splits into two parts:
- normal part;
- unwritten part (here we must return FIEMAP_EXTENT_LAST).
Before this commit we returned FIEMAP_EXTENT_LAST for whole fragment.
Fixes xfstest generic/092
Fixes: 4342306f0f0d ("fs/ntfs3: Add file operations and implementation")

Signed-off-by: Konstantin Komarov <[email protected]>
---
fs/ntfs3/file.c | 2 +-
fs/ntfs3/frecord.c | 10 +++++++---
2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c
index 43b1451bff53..5418e5ba64b3 100644
--- a/fs/ntfs3/file.c
+++ b/fs/ntfs3/file.c
@@ -761,7 +761,7 @@ int ntfs3_setattr(struct user_namespace *mnt_userns, struct dentry *dentry,
}
inode_dio_wait(inode);

- if (attr->ia_size < oldsize)
+ if (attr->ia_size <= oldsize)
err = ntfs_truncate(inode, attr->ia_size);
else if (attr->ia_size > oldsize)
err = ntfs_extend(inode, attr->ia_size, 0, NULL);
diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c
index 6f47a9c17f89..18842998c8fa 100644
--- a/fs/ntfs3/frecord.c
+++ b/fs/ntfs3/frecord.c
@@ -1964,10 +1964,8 @@ int ni_fiemap(struct ntfs_inode *ni, struct fiemap_extent_info *fieinfo,

vcn += clen;

- if (vbo + bytes >= end) {
+ if (vbo + bytes >= end)
bytes = end - vbo;
- flags |= FIEMAP_EXTENT_LAST;
- }

if (vbo + bytes <= valid) {
;
@@ -1977,6 +1975,9 @@ int ni_fiemap(struct ntfs_inode *ni, struct fiemap_extent_info *fieinfo,
/* vbo < valid && valid < vbo + bytes */
u64 dlen = valid - vbo;

+ if (vbo + dlen >= end)
+ flags |= FIEMAP_EXTENT_LAST;
+
err = fiemap_fill_next_extent(fieinfo, vbo, lbo, dlen,
flags);
if (err < 0)
@@ -1995,6 +1996,9 @@ int ni_fiemap(struct ntfs_inode *ni, struct fiemap_extent_info *fieinfo,
flags |= FIEMAP_EXTENT_UNWRITTEN;
}

+ if (vbo + bytes >= end)
+ flags |= FIEMAP_EXTENT_LAST;
+
err = fiemap_fill_next_extent(fieinfo, vbo, lbo, bytes, flags);
if (err < 0)
break;
--
2.33.0


2021-10-25 17:02:05

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 3/4] fs/ntfs3: Check new size for limits

We must check size before trying to allocate.
Size can be set for example by "ulimit -f".
Fixes xfstest generic/228
Fixes: 4342306f0f0d ("fs/ntfs3: Add file operations and implementation")

Signed-off-by: Konstantin Komarov <[email protected]>
---
fs/ntfs3/file.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c
index 5418e5ba64b3..efb3110e1790 100644
--- a/fs/ntfs3/file.c
+++ b/fs/ntfs3/file.c
@@ -661,7 +661,13 @@ static long ntfs_fallocate(struct file *file, int mode, loff_t vbo, loff_t len)
/*
* Normal file: Allocate clusters, do not change 'valid' size.
*/
- err = ntfs_set_size(inode, max(end, i_size));
+ loff_t new_size = max(end, i_size);
+
+ err = inode_newsize_ok(inode, new_size);
+ if (err)
+ goto out;
+
+ err = ntfs_set_size(inode, new_size);
if (err)
goto out;

--
2.33.0

2021-10-25 17:03:10

by Konstantin Komarov

[permalink] [raw]
Subject: [PATCH 4/4] fs/ntfs3: Update valid size if -EIOCBQUEUED

Update valid size if write is still in I/O queue.
Fixes xfstest generic/240
Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block")

Signed-off-by: Konstantin Komarov <[email protected]>
---
fs/ntfs3/inode.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index 859951d785cb..c211c64e6b17 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -757,6 +757,7 @@ static ssize_t ntfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
loff_t vbo = iocb->ki_pos;
loff_t end;
int wr = iov_iter_rw(iter) & WRITE;
+ size_t iter_count = iov_iter_count(iter);
loff_t valid;
ssize_t ret;

@@ -770,10 +771,14 @@ static ssize_t ntfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
wr ? ntfs_get_block_direct_IO_W
: ntfs_get_block_direct_IO_R);

- if (ret <= 0)
+ if (ret > 0)
+ end = vbo + ret;
+ else if (wr && -EIOCBQUEUED == ret)
+ end = vbo + iter_count;
+ else {
goto out;
+ }

- end = vbo + ret;
valid = ni->i_valid;
if (wr) {
if (end > valid && !S_ISBLK(inode->i_mode)) {
--
2.33.0


2021-10-25 17:16:42

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 1/4] fs/ntfs3: In function ntfs_set_acl_ex do not change inode->i_mode if called from function ntfs_init_acl

On Mon, 2021-10-25 at 19:58 +0300, Konstantin Komarov wrote:
> ntfs_init_acl sets mode. ntfs_init_acl calls ntfs_set_acl_ex.
> ntfs_set_acl_ex must not change this mode.
> Fixes xfstest generic/444
> Fixes: 83e8f5032e2d ("fs/ntfs3: Add attrib operations")

trivia:

> diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
[]
> @@ -538,7 +538,7 @@ struct posix_acl *ntfs_get_acl(struct inode *inode, int type)
>
> static noinline int ntfs_set_acl_ex(struct user_namespace *mnt_userns,
> struct inode *inode, struct posix_acl *acl,
> - int type)
> + int type, int init_acl)

bool init_acl?

> @@ -613,7 +614,7 @@ static noinline int ntfs_set_acl_ex(struct user_namespace *mnt_userns,
> int ntfs_set_acl(struct user_namespace *mnt_userns, struct inode *inode,
> struct posix_acl *acl, int type)
> {
> - return ntfs_set_acl_ex(mnt_userns, inode, acl, type);
> + return ntfs_set_acl_ex(mnt_userns, inode, acl, type, 0);

false

> }
>
> /*
> @@ -633,7 +634,7 @@ int ntfs_init_acl(struct user_namespace *mnt_userns, struct inode *inode,
>
> if (default_acl) {
> err = ntfs_set_acl_ex(mnt_userns, inode, default_acl,
> - ACL_TYPE_DEFAULT);
> + ACL_TYPE_DEFAULT, 1);

true, etc...


2021-10-27 07:08:23

by Kari Argillander

[permalink] [raw]
Subject: Re: [PATCH 1/4] fs/ntfs3: In function ntfs_set_acl_ex do not change inode->i_mode if called from function ntfs_init_acl

Maybe little too long subject line.

On Mon, Oct 25, 2021 at 07:58:26PM +0300, Konstantin Komarov wrote:
> ntfs_init_acl sets mode. ntfs_init_acl calls ntfs_set_acl_ex.
> ntfs_set_acl_ex must not change this mode.
> Fixes xfstest generic/444
> Fixes: 83e8f5032e2d ("fs/ntfs3: Add attrib operations")

Where does this commit id come from? Seems wrong to me.

>
> Signed-off-by: Konstantin Komarov <[email protected]>
> ---
> fs/ntfs3/xattr.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
> index 2143099cffdf..97b5f8417d85 100644
> --- a/fs/ntfs3/xattr.c
> +++ b/fs/ntfs3/xattr.c
> @@ -538,7 +538,7 @@ struct posix_acl *ntfs_get_acl(struct inode *inode, int type)
>
> static noinline int ntfs_set_acl_ex(struct user_namespace *mnt_userns,
> struct inode *inode, struct posix_acl *acl,
> - int type)
> + int type, int init_acl)

Like Joe say. Bool here and use true/false

> {
> const char *name;
> size_t size, name_len;
> @@ -551,8 +551,9 @@ static noinline int ntfs_set_acl_ex(struct user_namespace *mnt_userns,
>
> switch (type) {
> case ACL_TYPE_ACCESS:
> - if (acl) {
> - umode_t mode = inode->i_mode;
> + /* Do not change i_mode if we are in init_acl */
> + if (acl && !init_acl) {
> + umode_t mode;
>
> err = posix_acl_update_mode(mnt_userns, inode, &mode,
> &acl);
> @@ -613,7 +614,7 @@ static noinline int ntfs_set_acl_ex(struct user_namespace *mnt_userns,
> int ntfs_set_acl(struct user_namespace *mnt_userns, struct inode *inode,
> struct posix_acl *acl, int type)
> {
> - return ntfs_set_acl_ex(mnt_userns, inode, acl, type);
> + return ntfs_set_acl_ex(mnt_userns, inode, acl, type, 0);
> }
>
> /*
> @@ -633,7 +634,7 @@ int ntfs_init_acl(struct user_namespace *mnt_userns, struct inode *inode,
>
> if (default_acl) {
> err = ntfs_set_acl_ex(mnt_userns, inode, default_acl,
> - ACL_TYPE_DEFAULT);
> + ACL_TYPE_DEFAULT, 1);
> posix_acl_release(default_acl);
> } else {
> inode->i_default_acl = NULL;
> @@ -644,7 +645,7 @@ int ntfs_init_acl(struct user_namespace *mnt_userns, struct inode *inode,
> else {
> if (!err)
> err = ntfs_set_acl_ex(mnt_userns, inode, acl,
> - ACL_TYPE_ACCESS);
> + ACL_TYPE_ACCESS, 1);
> posix_acl_release(acl);
> }
>
> --
> 2.33.0
>

2021-10-27 11:04:18

by Kari Argillander

[permalink] [raw]
Subject: Re: [PATCH 3/4] fs/ntfs3: Check new size for limits

On Mon, Oct 25, 2021 at 07:59:26PM +0300, Konstantin Komarov wrote:
> We must check size before trying to allocate.
> Size can be set for example by "ulimit -f".
> Fixes xfstest generic/228

generic/228 [21:20:39][ 18.058334] run fstests generic/228 at 2021-10-26 21:20:39
[21:20:41] 2s
Ran: generic/228
Passed all 1 tests

> Fixes: 4342306f0f0d ("fs/ntfs3: Add file operations and implementation")
>
> Signed-off-by: Konstantin Komarov <[email protected]>

Reviewed-by: Kari Argillander <[email protected]>

> ---
> fs/ntfs3/file.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c
> index 5418e5ba64b3..efb3110e1790 100644
> --- a/fs/ntfs3/file.c
> +++ b/fs/ntfs3/file.c
> @@ -661,7 +661,13 @@ static long ntfs_fallocate(struct file *file, int mode, loff_t vbo, loff_t len)
> /*
> * Normal file: Allocate clusters, do not change 'valid' size.
> */
> - err = ntfs_set_size(inode, max(end, i_size));
> + loff_t new_size = max(end, i_size);
> +
> + err = inode_newsize_ok(inode, new_size);
> + if (err)
> + goto out;
> +
> + err = ntfs_set_size(inode, new_size);
> if (err)
> goto out;
>
> --
> 2.33.0
>
>

2021-10-27 11:05:04

by Kari Argillander

[permalink] [raw]
Subject: Re: [PATCH 2/4] fs/ntfs3: Fix fiemap + fix shrink file size (to remove preallocated space)

On Mon, Oct 25, 2021 at 07:58:57PM +0300, Konstantin Komarov wrote:
> Two problems:
> 1. ntfs3_setattr can't truncate preallocated space;
> 2. if allocated fragment "cross" valid size, then fragment splits into two parts:
> - normal part;
> - unwritten part (here we must return FIEMAP_EXTENT_LAST).
> Before this commit we returned FIEMAP_EXTENT_LAST for whole fragment.
> Fixes xfstest generic/092

I do not have time for now to got through more. Maybe you have some
ideas here.

generic/092 [21:16:19][ 18.906951] run fstests generic/092 at 2021-10-26 21:16:19
[21:16:22]- output mismatch (see /results/ntfs3/results-default/generic/092.out.bad)
--- tests/generic/092.out 2021-08-03 00:08:10.000000000 +0000
+++ /results/ntfs3/results-default/generic/092.out.bad 2021-10-26 21:16:22.127859289 +0000
@@ -2,5 +2,6 @@
wrote 5242880/5242880 bytes at offset 0
XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
0: [0..10239]: data
+1: [10240..20479]: unwritten
0: [0..10239]: data
1: [10240..20479]: unwritten

Argillander

> Fixes: 4342306f0f0d ("fs/ntfs3: Add file operations and implementation")
>
> Signed-off-by: Konstantin Komarov <[email protected]>
> ---
> fs/ntfs3/file.c | 2 +-
> fs/ntfs3/frecord.c | 10 +++++++---
> 2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c
> index 43b1451bff53..5418e5ba64b3 100644
> --- a/fs/ntfs3/file.c
> +++ b/fs/ntfs3/file.c
> @@ -761,7 +761,7 @@ int ntfs3_setattr(struct user_namespace *mnt_userns, struct dentry *dentry,
> }
> inode_dio_wait(inode);
>
> - if (attr->ia_size < oldsize)
> + if (attr->ia_size <= oldsize)
> err = ntfs_truncate(inode, attr->ia_size);
> else if (attr->ia_size > oldsize)
> err = ntfs_extend(inode, attr->ia_size, 0, NULL);
> diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c
> index 6f47a9c17f89..18842998c8fa 100644
> --- a/fs/ntfs3/frecord.c
> +++ b/fs/ntfs3/frecord.c
> @@ -1964,10 +1964,8 @@ int ni_fiemap(struct ntfs_inode *ni, struct fiemap_extent_info *fieinfo,
>
> vcn += clen;
>
> - if (vbo + bytes >= end) {
> + if (vbo + bytes >= end)
> bytes = end - vbo;
> - flags |= FIEMAP_EXTENT_LAST;
> - }
>
> if (vbo + bytes <= valid) {
> ;
> @@ -1977,6 +1975,9 @@ int ni_fiemap(struct ntfs_inode *ni, struct fiemap_extent_info *fieinfo,
> /* vbo < valid && valid < vbo + bytes */
> u64 dlen = valid - vbo;
>
> + if (vbo + dlen >= end)
> + flags |= FIEMAP_EXTENT_LAST;
> +
> err = fiemap_fill_next_extent(fieinfo, vbo, lbo, dlen,
> flags);
> if (err < 0)
> @@ -1995,6 +1996,9 @@ int ni_fiemap(struct ntfs_inode *ni, struct fiemap_extent_info *fieinfo,
> flags |= FIEMAP_EXTENT_UNWRITTEN;
> }
>
> + if (vbo + bytes >= end)
> + flags |= FIEMAP_EXTENT_LAST;
> +
> err = fiemap_fill_next_extent(fieinfo, vbo, lbo, bytes, flags);
> if (err < 0)
> break;
> --
> 2.33.0
>
>
>

2021-10-27 11:21:13

by Kari Argillander

[permalink] [raw]
Subject: Re: [PATCH 4/4] fs/ntfs3: Update valid size if -EIOCBQUEUED

On Mon, Oct 25, 2021 at 07:59:56PM +0300, Konstantin Komarov wrote:
> Update valid size if write is still in I/O queue.
> Fixes xfstest generic/240

generic/240 [21:23:16][ 17.933690] run fstests generic/240 at 2021-10-26 21:23:16
[21:23:18] 2s
Ran: generic/240
Passed all 1 tests

> Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block")
>
> Signed-off-by: Konstantin Komarov <[email protected]>
> ---
> fs/ntfs3/inode.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
> index 859951d785cb..c211c64e6b17 100644
> --- a/fs/ntfs3/inode.c
> +++ b/fs/ntfs3/inode.c
> @@ -757,6 +757,7 @@ static ssize_t ntfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
> loff_t vbo = iocb->ki_pos;
> loff_t end;
> int wr = iov_iter_rw(iter) & WRITE;
> + size_t iter_count = iov_iter_count(iter);
> loff_t valid;
> ssize_t ret;
>
> @@ -770,10 +771,14 @@ static ssize_t ntfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
> wr ? ntfs_get_block_direct_IO_W
> : ntfs_get_block_direct_IO_R);
>
> - if (ret <= 0)
> + if (ret > 0)
> + end = vbo + ret;
> + else if (wr && -EIOCBQUEUED == ret)

ret == -EIOCBQUEUED

> + end = vbo + iter_count;

Use iov_iter_count() instead of tmp var?

> + else {

Take brackets off.

> goto out;
> + }
>
> - end = vbo + ret;
> valid = ni->i_valid;
> if (wr) {
> if (end > valid && !S_ISBLK(inode->i_mode)) {
> --
> 2.33.0
>
>
>

2021-10-27 11:22:00

by Kari Argillander

[permalink] [raw]
Subject: Re: [PATCH 0/4] fs/ntfs3: Various fixes for xfstests problems

On Mon, Oct 25, 2021 at 07:57:30PM +0300, Konstantin Komarov wrote:
> This series fixes generic/444 generic/092 generic/228 generic/240

Now that 5.15 is just behind corner we need to start think about stable
also. Please read [1]. So basically this kind of fixes needs also

Cc: [email protected]

to sign-off-area. I know that stable team will try to pick also fixes
automatically, but this is "right" way. You can add this Cc lable also
when you make git am, but it is good the be there right away people can
discuss if it should be stable or not.

Please also add cc tag to other patch series 1-3 patches.

[1]: https://www.kernel.org/doc/Documentation/process/stable-kernel-rules.rst

> Konstantin Komarov (4):
> fs/ntfs3: In function ntfs_set_acl_ex do not change inode->i_mode if
> called from function ntfs_init_acl
> fs/ntfs3: Fix fiemap + fix shrink file size (to remove preallocated
> space)
> fs/ntfs3: Check new size for limits
> fs/ntfs3: Update valid size if -EIOCBQUEUED
>
> fs/ntfs3/file.c | 10 ++++++++--
> fs/ntfs3/frecord.c | 10 +++++++---
> fs/ntfs3/inode.c | 9 +++++++--
> fs/ntfs3/xattr.c | 13 +++++++------
> 4 files changed, 29 insertions(+), 13 deletions(-)
>
> --
> 2.33.0
>