[Jfs-discussion] [PATCH] jfs: fix slab-out-of-bounds write in jfs_readdir() with multibyte names

[Deepanshu K. <kar...@gm...>]

syzbot reported a KASAN slab-out-of-bounds write in utf32_to_utf8(),
reached via jfs_strfromUCS_le() from jfs_readdir() while handling
getdents64() on a crafted JFS image.

jfs_readdir() converts on-disk UTF-16 directory entry names into
dirent_buf, a PAGE_SIZE buffer. Before converting an entry, it checks
that the name fits by assuming one output byte per UTF-16 unit:

    jfs_dirent + d->namlen + 1 > dirent_buf + PAGE_SIZE

With iocharset=utf8 a single UTF-16 unit can expand to up to three
bytes, so a name can be approved for space it does not have.

jfs_strfromUCS_le() makes this worse: it has no destination size and
always passes NLS_MAX_CHARSET_SIZE as the output bound to uni2char(),
so the converter believes it has room regardless of how much of the
buffer is actually left. The conversion then writes past the end of
dirent_buf, and the trailing NUL can land one byte out of bounds too.

Fix this in two places:

 - In jfs_readdir(), reserve the worst case of NLS_MAX_CHARSET_SIZE
   bytes per UTF-16 unit when checking whether an entry fits.

 - Give jfs_strfromUCS_le() a destination size (tolen) and pass the
   real remaining space to uni2char() instead of the constant, keeping
   one byte for the terminating NUL. Clamp the length in the
   non-codepage path in the same way. Callers pass the distance to the
   end of dirent_buf.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syz...@sy...
Closes: https://syzkaller.appspot.com/bug?extid=a2748ba908c108e7e525
Tested-by: syz...@sy...
Assisted-by: LLM
Signed-off-by: Deepanshu Kartikey <kar...@gm...>
---
 fs/jfs/jfs_dtree.c   |  8 +++++---
 fs/jfs/jfs_unicode.c | 15 ++++++++++++---
 fs/jfs/jfs_unicode.h |  2 +-
 3 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/fs/jfs/jfs_dtree.c b/fs/jfs/jfs_dtree.c
index 8ce6e4458cc2..5ccc90e3c068 100644
--- a/fs/jfs/jfs_dtree.c
+++ b/fs/jfs/jfs_dtree.c
@@ -2738,6 +2738,7 @@ int jfs_readdir(struct file *file, struct dir_context *ctx)
 	int jfs_dirents;
 	int overflow, fix_page, page_fixed = 0;
 	static int unique_pos = 2;	/* If we can't fix broken index */
+	char *buf_end;
 
 	if (ctx->pos == DIREND)
 		return 0;
@@ -2892,6 +2893,7 @@ int jfs_readdir(struct file *file, struct dir_context *ctx)
 		return -ENOMEM;
 	}
 
+	buf_end = (char *)dirent_buf + PAGE_SIZE;
 	while (1) {
 		jfs_dirent = dirent_buf;
 		jfs_dirents = 0;
@@ -2910,7 +2912,7 @@ int jfs_readdir(struct file *file, struct dir_context *ctx)
 
 			d = (struct ldtentry *) & p->slot[stbl[i]];
 
-			if (((long) jfs_dirent + d->namlen + 1) >
+			if (((long) jfs_dirent + (long)d->namlen * NLS_MAX_CHARSET_SIZE + 1) >
 			    ((long)dirent_buf + PAGE_SIZE)) {
 				/* DBCS codepages could overrun dirent_buf */
 				index = i;
@@ -2961,7 +2963,7 @@ int jfs_readdir(struct file *file, struct dir_context *ctx)
 			}
 
 			/* copy the name of head/only segment */
-			outlen = jfs_strfromUCS_le(name_ptr, d->name, len,
+			outlen = jfs_strfromUCS_le(name_ptr, buf_end - name_ptr, d->name, len,
 						   codepage);
 			jfs_dirent->name_len = outlen;
 
@@ -2981,7 +2983,7 @@ int jfs_readdir(struct file *file, struct dir_context *ctx)
 					goto skip_one;
 				}
 				len = min(d_namleft, DTSLOTDATALEN);
-				outlen = jfs_strfromUCS_le(name_ptr, t->name,
+				outlen = jfs_strfromUCS_le(name_ptr, buf_end - name_ptr, t->name,
 							   len, codepage);
 				jfs_dirent->name_len += outlen;
 
diff --git a/fs/jfs/jfs_unicode.c b/fs/jfs/jfs_unicode.c
index 0c1e9027245a..bcff7e0031b2 100644
--- a/fs/jfs/jfs_unicode.c
+++ b/fs/jfs/jfs_unicode.c
@@ -16,27 +16,36 @@
  * FUNCTION:	Convert little-endian unicode string to character string
  *
  */
-int jfs_strfromUCS_le(char *to, const __le16 * from,
+int jfs_strfromUCS_le(char *to, int tolen, const __le16 *from,
 		      int len, struct nls_table *codepage)
 {
-	int i;
+	int i, room;
 	int outlen = 0;
 	static int warn_again = 5;	/* Only warn up to 5 times total */
 	int warn = !!warn_again;	/* once per string */
 
+	if (tolen <= 0)
+		return 0;
+
 	if (codepage) {
 		for (i = 0; (i < len) && from[i]; i++) {
 			int charlen;
+			room = tolen - outlen - 1;
+			if (room <= 0)
+				break;
 			charlen =
 			    codepage->uni2char(le16_to_cpu(from[i]),
 					       &to[outlen],
-					       NLS_MAX_CHARSET_SIZE);
+					       room);
 			if (charlen > 0)
 				outlen += charlen;
 			else
 				to[outlen++] = '?';
 		}
 	} else {
+		if (len > tolen - 1)
+			len = tolen - 1;
+
 		for (i = 0; (i < len) && from[i]; i++) {
 			if (unlikely(le16_to_cpu(from[i]) & 0xff00)) {
 				to[i] = '?';
diff --git a/fs/jfs/jfs_unicode.h b/fs/jfs/jfs_unicode.h
index b6a78d4aef1b..ea03dd5a0e0c 100644
--- a/fs/jfs/jfs_unicode.h
+++ b/fs/jfs/jfs_unicode.h
@@ -12,7 +12,7 @@
 #include "jfs_types.h"
 
 extern int get_UCSname(struct component_name *, struct dentry *);
-extern int jfs_strfromUCS_le(char *, const __le16 *, int, struct nls_table *);
+extern int jfs_strfromUCS_le(char *, int, const __le16 *, int, struct nls_table *);
 
 #define free_UCSname(COMP) kfree((COMP)->name)
 
-- 
2.43.0