If length of symlink > 255, then we tried to convert
length of symlink +- some random number.
So main theme is removing 255 symbols limit in ntfs_utf16_to_nls.
Other bug - we haven't always returned correct size of symlink,
so we save it now in ntfs_create_inode.
Many commits affected, so no fixes tag.
This series fixes xfstest generic/423.
Konstantin Komarov (5):
fs/ntfs3: Rework ntfs_utf16_to_nls
fs/ntfs3: Refactor ntfs_readlink_hlp
fs/ntfs3: Refactor ntfs_create_inode
fs/ntfs3: Refactor ni_parse_reparse
fs/ntfs3: Refactor ntfs_read_mft
fs/ntfs3/dir.c | 19 +++----
fs/ntfs3/frecord.c | 9 ++--
fs/ntfs3/inode.c | 124 +++++++++++++++++++++------------------------
fs/ntfs3/ntfs_fs.h | 4 +-
4 files changed, 74 insertions(+), 82 deletions(-)
--
2.33.0
Now ntfs_utf16_to_nls takes length as one of arguments.
If length of symlink > 255, then we tried to convert
length of symlink +- some random number.
Now 255 symbols limit was removed.
Signed-off-by: Konstantin Komarov <[email protected]>
---
fs/ntfs3/dir.c | 19 ++++++++-----------
fs/ntfs3/ntfs_fs.h | 2 +-
2 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/fs/ntfs3/dir.c b/fs/ntfs3/dir.c
index 785e72d4392e..fb438d604040 100644
--- a/fs/ntfs3/dir.c
+++ b/fs/ntfs3/dir.c
@@ -15,11 +15,10 @@
#include "ntfs_fs.h"
/* Convert little endian UTF-16 to NLS string. */
-int ntfs_utf16_to_nls(struct ntfs_sb_info *sbi, const struct le_str *uni,
+int ntfs_utf16_to_nls(struct ntfs_sb_info *sbi, const __le16 *name, u32 len,
u8 *buf, int buf_len)
{
- int ret, uni_len, warn;
- const __le16 *ip;
+ int ret, warn;
u8 *op;
struct nls_table *nls = sbi->options->nls;
@@ -27,18 +26,16 @@ int ntfs_utf16_to_nls(struct ntfs_sb_info *sbi, const struct le_str *uni,
if (!nls) {
/* UTF-16 -> UTF-8 */
- ret = utf16s_to_utf8s((wchar_t *)uni->name, uni->len,
- UTF16_LITTLE_ENDIAN, buf, buf_len);
+ ret = utf16s_to_utf8s(name, len, UTF16_LITTLE_ENDIAN, buf,
+ buf_len);
buf[ret] = '\0';
return ret;
}
- ip = uni->name;
op = buf;
- uni_len = uni->len;
warn = 0;
- while (uni_len--) {
+ while (len--) {
u16 ec;
int charlen;
char dump[5];
@@ -49,7 +46,7 @@ int ntfs_utf16_to_nls(struct ntfs_sb_info *sbi, const struct le_str *uni,
break;
}
- ec = le16_to_cpu(*ip++);
+ ec = le16_to_cpu(*name++);
charlen = nls->uni2char(ec, op, buf_len);
if (charlen > 0) {
@@ -304,8 +301,8 @@ static inline int ntfs_filldir(struct ntfs_sb_info *sbi, struct ntfs_inode *ni,
if (sbi->options->nohidden && (fname->dup.fa & FILE_ATTRIBUTE_HIDDEN))
return 0;
- name_len = ntfs_utf16_to_nls(sbi, (struct le_str *)&fname->name_len,
- name, PATH_MAX);
+ name_len = ntfs_utf16_to_nls(sbi, fname->name, fname->name_len, name,
+ PATH_MAX);
if (name_len <= 0) {
ntfs_warn(sbi->sb, "failed to convert name for inode %lx.",
ino);
diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index 38b7c1a9dc52..9277b552f257 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -475,7 +475,7 @@ bool are_bits_set(const ulong *map, size_t bit, size_t nbits);
size_t get_set_bits_ex(const ulong *map, size_t bit, size_t nbits);
/* Globals from dir.c */
-int ntfs_utf16_to_nls(struct ntfs_sb_info *sbi, const struct le_str *uni,
+int ntfs_utf16_to_nls(struct ntfs_sb_info *sbi, const __le16 *name, u32 len,
u8 *buf, int buf_len);
int ntfs_nls_to_utf16(struct ntfs_sb_info *sbi, const u8 *name, u32 name_len,
struct cpu_str *uni, u32 max_ulen,
--
2.33.0
Rename some variables.
Returned err by default is EINVAL.
Signed-off-by: Konstantin Komarov <[email protected]>
---
fs/ntfs3/inode.c | 91 +++++++++++++++++++++++-------------------------
1 file changed, 43 insertions(+), 48 deletions(-)
diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index 7dd162f6a7e2..d618b0573533 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -1763,15 +1763,15 @@ void ntfs_evict_inode(struct inode *inode)
static noinline int ntfs_readlink_hlp(struct inode *inode, char *buffer,
int buflen)
{
- int i, err = 0;
+ int i, err = -EINVAL;
struct ntfs_inode *ni = ntfs_i(inode);
struct super_block *sb = inode->i_sb;
struct ntfs_sb_info *sbi = sb->s_fs_info;
- u64 i_size = inode->i_size;
- u16 nlen = 0;
+ u64 size;
+ u16 ulen = 0;
void *to_free = NULL;
struct REPARSE_DATA_BUFFER *rp;
- struct le_str *uni;
+ const __le16 *uname;
struct ATTRIB *attr;
/* Reparse data present. Try to parse it. */
@@ -1780,68 +1780,64 @@ static noinline int ntfs_readlink_hlp(struct inode *inode, char *buffer,
*buffer = 0;
- /* Read into temporal buffer. */
- if (i_size > sbi->reparse.max_size || i_size <= sizeof(u32)) {
- err = -EINVAL;
- goto out;
- }
-
attr = ni_find_attr(ni, NULL, NULL, ATTR_REPARSE, NULL, 0, NULL, NULL);
- if (!attr) {
- err = -EINVAL;
+ if (!attr)
goto out;
- }
if (!attr->non_res) {
- rp = resident_data_ex(attr, i_size);
- if (!rp) {
- err = -EINVAL;
+ rp = resident_data_ex(attr, sizeof(struct REPARSE_DATA_BUFFER));
+ if (!rp)
goto out;
- }
+ size = le32_to_cpu(attr->res.data_size);
} else {
- rp = kmalloc(i_size, GFP_NOFS);
+ size = le64_to_cpu(attr->nres.data_size);
+ rp = NULL;
+ }
+
+ if (size > sbi->reparse.max_size || size <= sizeof(u32))
+ goto out;
+
+ if (!rp) {
+ rp = kmalloc(size, GFP_NOFS);
if (!rp) {
err = -ENOMEM;
goto out;
}
to_free = rp;
- err = ntfs_read_run_nb(sbi, &ni->file.run, 0, rp, i_size, NULL);
+ /* Read into temporal buffer. */
+ err = ntfs_read_run_nb(sbi, &ni->file.run, 0, rp, size, NULL);
if (err)
goto out;
}
- err = -EINVAL;
-
/* Microsoft Tag. */
switch (rp->ReparseTag) {
case IO_REPARSE_TAG_MOUNT_POINT:
/* Mount points and junctions. */
/* Can we use 'Rp->MountPointReparseBuffer.PrintNameLength'? */
- if (i_size <= offsetof(struct REPARSE_DATA_BUFFER,
- MountPointReparseBuffer.PathBuffer))
+ if (size <= offsetof(struct REPARSE_DATA_BUFFER,
+ MountPointReparseBuffer.PathBuffer))
goto out;
- uni = Add2Ptr(rp,
- offsetof(struct REPARSE_DATA_BUFFER,
- MountPointReparseBuffer.PathBuffer) +
- le16_to_cpu(rp->MountPointReparseBuffer
- .PrintNameOffset) -
- 2);
- nlen = le16_to_cpu(rp->MountPointReparseBuffer.PrintNameLength);
+ uname = Add2Ptr(rp,
+ offsetof(struct REPARSE_DATA_BUFFER,
+ MountPointReparseBuffer.PathBuffer) +
+ le16_to_cpu(rp->MountPointReparseBuffer
+ .PrintNameOffset));
+ ulen = le16_to_cpu(rp->MountPointReparseBuffer.PrintNameLength);
break;
case IO_REPARSE_TAG_SYMLINK:
/* FolderSymbolicLink */
/* Can we use 'Rp->SymbolicLinkReparseBuffer.PrintNameLength'? */
- if (i_size <= offsetof(struct REPARSE_DATA_BUFFER,
- SymbolicLinkReparseBuffer.PathBuffer))
+ if (size <= offsetof(struct REPARSE_DATA_BUFFER,
+ SymbolicLinkReparseBuffer.PathBuffer))
goto out;
- uni = Add2Ptr(rp,
- offsetof(struct REPARSE_DATA_BUFFER,
- SymbolicLinkReparseBuffer.PathBuffer) +
- le16_to_cpu(rp->SymbolicLinkReparseBuffer
- .PrintNameOffset) -
- 2);
- nlen = le16_to_cpu(
+ uname = Add2Ptr(
+ rp, offsetof(struct REPARSE_DATA_BUFFER,
+ SymbolicLinkReparseBuffer.PathBuffer) +
+ le16_to_cpu(rp->SymbolicLinkReparseBuffer
+ .PrintNameOffset));
+ ulen = le16_to_cpu(
rp->SymbolicLinkReparseBuffer.PrintNameLength);
break;
@@ -1873,29 +1869,28 @@ static noinline int ntfs_readlink_hlp(struct inode *inode, char *buffer,
goto out;
}
if (!IsReparseTagNameSurrogate(rp->ReparseTag) ||
- i_size <= sizeof(struct REPARSE_POINT)) {
+ size <= sizeof(struct REPARSE_POINT)) {
goto out;
}
/* Users tag. */
- uni = Add2Ptr(rp, sizeof(struct REPARSE_POINT) - 2);
- nlen = le16_to_cpu(rp->ReparseDataLength) -
+ uname = Add2Ptr(rp, sizeof(struct REPARSE_POINT));
+ ulen = le16_to_cpu(rp->ReparseDataLength) -
sizeof(struct REPARSE_POINT);
}
/* Convert nlen from bytes to UNICODE chars. */
- nlen >>= 1;
+ ulen >>= 1;
/* Check that name is available. */
- if (!nlen || &uni->name[nlen] > (__le16 *)Add2Ptr(rp, i_size))
+ if (!ulen || uname + ulen > (__le16 *)Add2Ptr(rp, size))
goto out;
/* If name is already zero terminated then truncate it now. */
- if (!uni->name[nlen - 1])
- nlen -= 1;
- uni->len = nlen;
+ if (!uname[ulen - 1])
+ ulen -= 1;
- err = ntfs_utf16_to_nls(sbi, uni, buffer, buflen);
+ err = ntfs_utf16_to_nls(sbi, uname, ulen, buffer, buflen);
if (err < 0)
goto out;
--
2.33.0
Set size for symlink, so we don't need to calculate it on the fly.
Signed-off-by: Konstantin Komarov <[email protected]>
---
fs/ntfs3/inode.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index d618b0573533..bdebbbd53e76 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -1488,7 +1488,10 @@ struct inode *ntfs_create_inode(struct user_namespace *mnt_userns,
asize = ALIGN(SIZEOF_RESIDENT + nsize, 8);
t16 = PtrOffset(rec, attr);
- /* 0x78 - the size of EA + EAINFO to store WSL */
+ /*
+ * Below function 'ntfs_save_wsl_perm' requires 0x78 bytes.
+ * It is good idea to keep extened attributes resident.
+ */
if (asize + t16 + 0x78 + 8 > sbi->record_size) {
CLST alen;
CLST clst = bytes_to_cluster(sbi, nsize);
@@ -1523,14 +1526,14 @@ struct inode *ntfs_create_inode(struct user_namespace *mnt_userns,
}
asize = SIZEOF_NONRESIDENT + ALIGN(err, 8);
- inode->i_size = nsize;
} else {
attr->res.data_off = SIZEOF_RESIDENT_LE;
attr->res.data_size = cpu_to_le32(nsize);
memcpy(Add2Ptr(attr, SIZEOF_RESIDENT), rp, nsize);
- inode->i_size = nsize;
nsize = 0;
}
+ /* Size of symlink equals the length of input string. */
+ inode->i_size = size;
attr->size = cpu_to_le32(asize);
@@ -1567,6 +1570,8 @@ struct inode *ntfs_create_inode(struct user_namespace *mnt_userns,
inode->i_op = &ntfs_link_inode_operations;
inode->i_fop = NULL;
inode->i_mapping->a_ops = &ntfs_aops;
+ inode->i_size = size;
+ inode_nohighmem(inode);
} else if (S_ISREG(mode)) {
inode->i_op = &ntfs_file_inode_operations;
inode->i_fop = &ntfs_file_operations;
--
2.33.0
[PATCH 5/5] fs/ntfs3: Refactor ntfs_read_mft
Don't save size of attribute reparse point as size of symlink.
Signed-off-by: Konstantin Komarov <[email protected]>
---
fs/ntfs3/inode.c | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index bdebbbd53e76..859951d785cb 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -222,9 +222,6 @@ static struct inode *ntfs_read_mft(struct inode *inode,
if (!attr->non_res) {
ni->i_valid = inode->i_size = rsize;
inode_set_bytes(inode, rsize);
- t32 = asize;
- } else {
- t32 = le16_to_cpu(attr->nres.run_off);
}
mode = S_IFREG | (0777 & sbi->options->fs_fmask_inv);
@@ -313,17 +310,14 @@ static struct inode *ntfs_read_mft(struct inode *inode,
rp_fa = ni_parse_reparse(ni, attr, &rp);
switch (rp_fa) {
case REPARSE_LINK:
- if (!attr->non_res) {
- inode->i_size = rsize;
- inode_set_bytes(inode, rsize);
- t32 = asize;
- } else {
- inode->i_size =
- le64_to_cpu(attr->nres.data_size);
- t32 = le16_to_cpu(attr->nres.run_off);
- }
+ /*
+ * Normal symlink.
+ * Assume one unicode symbol == one utf8.
+ */
+ inode->i_size = le16_to_cpu(rp.SymbolicLinkReparseBuffer
+ .PrintNameLength) /
+ sizeof(u16);
- /* Looks like normal symlink. */
ni->i_valid = inode->i_size;
/* Clear directory bit. */
@@ -420,7 +414,7 @@ static struct inode *ntfs_read_mft(struct inode *inode,
ni->std_fa &= ~FILE_ATTRIBUTE_DIRECTORY;
inode->i_op = &ntfs_link_inode_operations;
inode->i_fop = NULL;
- inode_nohighmem(inode); // ??
+ inode_nohighmem(inode);
} else if (S_ISREG(mode)) {
ni->std_fa &= ~FILE_ATTRIBUTE_DIRECTORY;
inode->i_op = &ntfs_file_inode_operations;
--
2.33.0
Change argument from void* to struct REPARSE_DATA_BUFFER*
We copy data to buffer, so we can read it later in ntfs_read_mft.
Signed-off-by: Konstantin Komarov <[email protected]>
---
fs/ntfs3/frecord.c | 9 +++++----
fs/ntfs3/ntfs_fs.h | 2 +-
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c
index 007602badd90..ecb965e4afd0 100644
--- a/fs/ntfs3/frecord.c
+++ b/fs/ntfs3/frecord.c
@@ -1710,18 +1710,16 @@ int ni_new_attr_flags(struct ntfs_inode *ni, enum FILE_ATTRIBUTE new_fa)
/*
* ni_parse_reparse
*
- * Buffer is at least 24 bytes.
+ * buffer - memory for reparse buffer header
*/
enum REPARSE_SIGN ni_parse_reparse(struct ntfs_inode *ni, struct ATTRIB *attr,
- void *buffer)
+ struct REPARSE_DATA_BUFFER *buffer)
{
const struct REPARSE_DATA_BUFFER *rp = NULL;
u8 bits;
u16 len;
typeof(rp->CompressReparseBuffer) *cmpr;
- static_assert(sizeof(struct REPARSE_DATA_BUFFER) <= 24);
-
/* Try to estimate reparse point. */
if (!attr->non_res) {
rp = resident_data_ex(attr, sizeof(struct REPARSE_DATA_BUFFER));
@@ -1807,6 +1805,9 @@ enum REPARSE_SIGN ni_parse_reparse(struct ntfs_inode *ni, struct ATTRIB *attr,
return REPARSE_NONE;
}
+ if (buffer != rp)
+ memcpy(buffer, rp, sizeof(struct REPARSE_DATA_BUFFER));
+
/* Looks like normal symlink. */
return REPARSE_LINK;
}
diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index 9277b552f257..e95d93c683ed 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -547,7 +547,7 @@ struct ATTR_FILE_NAME *ni_fname_type(struct ntfs_inode *ni, u8 name_type,
struct ATTR_LIST_ENTRY **entry);
int ni_new_attr_flags(struct ntfs_inode *ni, enum FILE_ATTRIBUTE new_fa);
enum REPARSE_SIGN ni_parse_reparse(struct ntfs_inode *ni, struct ATTRIB *attr,
- void *buffer);
+ struct REPARSE_DATA_BUFFER *buffer);
int ni_write_inode(struct inode *inode, int sync, const char *hint);
#define _ni_write_inode(i, w) ni_write_inode(i, w, __func__)
int ni_fiemap(struct ntfs_inode *ni, struct fiemap_extent_info *fieinfo,
--
2.33.0
On Tue, Oct 05, 2021 at 07:47:03PM +0300, Konstantin Komarov wrote:
> Now ntfs_utf16_to_nls takes length as one of arguments.
> If length of symlink > 255, then we tried to convert
> length of symlink +- some random number.
> Now 255 symbols limit was removed.
>
> Signed-off-by: Konstantin Komarov <[email protected]>
> ---
> fs/ntfs3/dir.c | 19 ++++++++-----------
> fs/ntfs3/ntfs_fs.h | 2 +-
> 2 files changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/fs/ntfs3/dir.c b/fs/ntfs3/dir.c
> index 785e72d4392e..fb438d604040 100644
> --- a/fs/ntfs3/dir.c
> +++ b/fs/ntfs3/dir.c
> @@ -15,11 +15,10 @@
> #include "ntfs_fs.h"
>
> /* Convert little endian UTF-16 to NLS string. */
> -int ntfs_utf16_to_nls(struct ntfs_sb_info *sbi, const struct le_str *uni,
> +int ntfs_utf16_to_nls(struct ntfs_sb_info *sbi, const __le16 *name, u32 len,
> u8 *buf, int buf_len)
I just like to point out that this patch break the build. We cannot
break build in any commit. If you make change you need to make sure
everything still works. This is of course now late, but I just bring it
up if you did not know it already.
Problem here is that you change callers to match this in patch 2/5. You
need to make it in same patch. Basically every patch should build it is
own and even all tests should work.
Reason here is git bisecting. We really want to easy way to bisect bugs
and if some commit break something then it make bisecting more
difficult.
I also wonder why 0day bot did not report this. Maybe because there was
no base or something, but I have to look into that as for now that is
only CI system we have. When we get ntfs3 to kernel.org then we also can
use patchwork and then my first thing to do is to start CI system that
reports results to patchwork.
I will use Snowpatch to fetch patches from patchwork and Jenkins to CI.
> {
> - int ret, uni_len, warn;
> - const __le16 *ip;
> + int ret, warn;
> u8 *op;
> struct nls_table *nls = sbi->options->nls;
>
> @@ -27,18 +26,16 @@ int ntfs_utf16_to_nls(struct ntfs_sb_info *sbi, const struct le_str *uni,
>
> if (!nls) {
> /* UTF-16 -> UTF-8 */
> - ret = utf16s_to_utf8s((wchar_t *)uni->name, uni->len,
> - UTF16_LITTLE_ENDIAN, buf, buf_len);
> + ret = utf16s_to_utf8s(name, len, UTF16_LITTLE_ENDIAN, buf,
> + buf_len);
> buf[ret] = '\0';
> return ret;
> }
>
> - ip = uni->name;
> op = buf;
> - uni_len = uni->len;
> warn = 0;
>
> - while (uni_len--) {
> + while (len--) {
> u16 ec;
> int charlen;
> char dump[5];
> @@ -49,7 +46,7 @@ int ntfs_utf16_to_nls(struct ntfs_sb_info *sbi, const struct le_str *uni,
> break;
> }
>
> - ec = le16_to_cpu(*ip++);
> + ec = le16_to_cpu(*name++);
> charlen = nls->uni2char(ec, op, buf_len);
>
> if (charlen > 0) {
> @@ -304,8 +301,8 @@ static inline int ntfs_filldir(struct ntfs_sb_info *sbi, struct ntfs_inode *ni,
> if (sbi->options->nohidden && (fname->dup.fa & FILE_ATTRIBUTE_HIDDEN))
> return 0;
>
> - name_len = ntfs_utf16_to_nls(sbi, (struct le_str *)&fname->name_len,
> - name, PATH_MAX);
> + name_len = ntfs_utf16_to_nls(sbi, fname->name, fname->name_len, name,
> + PATH_MAX);
> if (name_len <= 0) {
> ntfs_warn(sbi->sb, "failed to convert name for inode %lx.",
> ino);
> diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
> index 38b7c1a9dc52..9277b552f257 100644
> --- a/fs/ntfs3/ntfs_fs.h
> +++ b/fs/ntfs3/ntfs_fs.h
> @@ -475,7 +475,7 @@ bool are_bits_set(const ulong *map, size_t bit, size_t nbits);
> size_t get_set_bits_ex(const ulong *map, size_t bit, size_t nbits);
>
> /* Globals from dir.c */
> -int ntfs_utf16_to_nls(struct ntfs_sb_info *sbi, const struct le_str *uni,
> +int ntfs_utf16_to_nls(struct ntfs_sb_info *sbi, const __le16 *name, u32 len,
> u8 *buf, int buf_len);
> int ntfs_nls_to_utf16(struct ntfs_sb_info *sbi, const u8 *name, u32 name_len,
> struct cpu_str *uni, u32 max_ulen,
> --
> 2.33.0
>
>