linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] fs/ntfs3: Reworking symlink functions
@ 2021-10-05 16:43 Konstantin Komarov
  2021-10-05 16:47 ` [PATCH 1/5] fs/ntfs3: Rework ntfs_utf16_to_nls Konstantin Komarov
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Konstantin Komarov @ 2021-10-05 16:43 UTC (permalink / raw)
  To: ntfs3; +Cc: linux-kernel, linux-fsdevel

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/5] fs/ntfs3: Rework ntfs_utf16_to_nls
  2021-10-05 16:43 [PATCH 0/5] fs/ntfs3: Reworking symlink functions Konstantin Komarov
@ 2021-10-05 16:47 ` Konstantin Komarov
  2021-10-17  8:10   ` Kari Argillander
  2021-10-05 16:47 ` [PATCH 2/5] fs/ntfs3: Refactor ntfs_readlink_hlp Konstantin Komarov
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 7+ messages in thread
From: Konstantin Komarov @ 2021-10-05 16:47 UTC (permalink / raw)
  To: ntfs3; +Cc: linux-kernel, linux-fsdevel

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 <almaz.alexandrovich@paragon-software.com>
---
 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



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/5] fs/ntfs3: Refactor ntfs_readlink_hlp
  2021-10-05 16:43 [PATCH 0/5] fs/ntfs3: Reworking symlink functions Konstantin Komarov
  2021-10-05 16:47 ` [PATCH 1/5] fs/ntfs3: Rework ntfs_utf16_to_nls Konstantin Komarov
@ 2021-10-05 16:47 ` Konstantin Komarov
  2021-10-05 16:47 ` [PATCH 3/5] fs/ntfs3: Refactor ntfs_create_inode Konstantin Komarov
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Konstantin Komarov @ 2021-10-05 16:47 UTC (permalink / raw)
  To: ntfs3; +Cc: linux-kernel, linux-fsdevel

Rename some variables.
Returned err by default is EINVAL.

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
 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



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/5] fs/ntfs3: Refactor ntfs_create_inode
  2021-10-05 16:43 [PATCH 0/5] fs/ntfs3: Reworking symlink functions Konstantin Komarov
  2021-10-05 16:47 ` [PATCH 1/5] fs/ntfs3: Rework ntfs_utf16_to_nls Konstantin Komarov
  2021-10-05 16:47 ` [PATCH 2/5] fs/ntfs3: Refactor ntfs_readlink_hlp Konstantin Komarov
@ 2021-10-05 16:47 ` Konstantin Komarov
  2021-10-05 16:48 ` [PATCH 4/5] fs/ntfs3: Refactor ni_parse_reparse Konstantin Komarov
  2021-10-05 16:48 ` [PATCH 5/5] fs/ntfs3: Refactor ntfs_read_mft Konstantin Komarov
  4 siblings, 0 replies; 7+ messages in thread
From: Konstantin Komarov @ 2021-10-05 16:47 UTC (permalink / raw)
  To: ntfs3; +Cc: linux-kernel, linux-fsdevel

Set size for symlink, so we don't need to calculate it on the fly.

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
---
 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



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/5] fs/ntfs3: Refactor ni_parse_reparse
  2021-10-05 16:43 [PATCH 0/5] fs/ntfs3: Reworking symlink functions Konstantin Komarov
                   ` (2 preceding siblings ...)
  2021-10-05 16:47 ` [PATCH 3/5] fs/ntfs3: Refactor ntfs_create_inode Konstantin Komarov
@ 2021-10-05 16:48 ` Konstantin Komarov
  2021-10-05 16:48 ` [PATCH 5/5] fs/ntfs3: Refactor ntfs_read_mft Konstantin Komarov
  4 siblings, 0 replies; 7+ messages in thread
From: Konstantin Komarov @ 2021-10-05 16:48 UTC (permalink / raw)
  To: ntfs3; +Cc: linux-kernel, linux-fsdevel

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 <almaz.alexandrovich@paragon-software.com>
---
 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



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 5/5] fs/ntfs3: Refactor ntfs_read_mft
  2021-10-05 16:43 [PATCH 0/5] fs/ntfs3: Reworking symlink functions Konstantin Komarov
                   ` (3 preceding siblings ...)
  2021-10-05 16:48 ` [PATCH 4/5] fs/ntfs3: Refactor ni_parse_reparse Konstantin Komarov
@ 2021-10-05 16:48 ` Konstantin Komarov
  4 siblings, 0 replies; 7+ messages in thread
From: Konstantin Komarov @ 2021-10-05 16:48 UTC (permalink / raw)
  To: ntfs3; +Cc: linux-kernel, linux-fsdevel

[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 <almaz.alexandrovich@paragon-software.com>
---
 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



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/5] fs/ntfs3: Rework ntfs_utf16_to_nls
  2021-10-05 16:47 ` [PATCH 1/5] fs/ntfs3: Rework ntfs_utf16_to_nls Konstantin Komarov
@ 2021-10-17  8:10   ` Kari Argillander
  0 siblings, 0 replies; 7+ messages in thread
From: Kari Argillander @ 2021-10-17  8:10 UTC (permalink / raw)
  To: Konstantin Komarov; +Cc: ntfs3, linux-kernel, linux-fsdevel

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 <almaz.alexandrovich@paragon-software.com>
> ---
>  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
> 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-10-17  8:10 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-05 16:43 [PATCH 0/5] fs/ntfs3: Reworking symlink functions Konstantin Komarov
2021-10-05 16:47 ` [PATCH 1/5] fs/ntfs3: Rework ntfs_utf16_to_nls Konstantin Komarov
2021-10-17  8:10   ` Kari Argillander
2021-10-05 16:47 ` [PATCH 2/5] fs/ntfs3: Refactor ntfs_readlink_hlp Konstantin Komarov
2021-10-05 16:47 ` [PATCH 3/5] fs/ntfs3: Refactor ntfs_create_inode Konstantin Komarov
2021-10-05 16:48 ` [PATCH 4/5] fs/ntfs3: Refactor ni_parse_reparse Konstantin Komarov
2021-10-05 16:48 ` [PATCH 5/5] fs/ntfs3: Refactor ntfs_read_mft Konstantin Komarov

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).