[PATCH 1/2] isofs: size the name conversion buffer from a named constant

[Matthias Goergens]

isofs_readdir() and isofs_lookup() each allocate a 1024-byte scratch
buffer that four converters write into: get_rock_ridge_filename(),
get_joliet_filename(), get_acorn_filename() and isofs_name_translate().
None of them is told how big it is.

get_joliet_filename() gets this wrong today.  It passes PAGE_SIZE as the
output limit to utf16s_to_utf8s(), four times the buffer it is actually
given.  That was correct until commit b2eb2e288604 ("isofs: Drop support
of directory entries straddling blocks"), which shrank the allocation
from a page to 1024 bytes in both callers without updating the bound.

No overflow is possible today: de->name_len is a single byte, so the
largest name any converter can produce is 763 bytes, on the Joliet
iocharset path ((255 >> 1) units, each expanding to at most
NLS_MAX_CHARSET_SIZE bytes, plus a terminator).  But nothing says so,
and joliet.c says the opposite.

Give the buffer a name, use it at both allocation sites and as the
Joliet bound, and have each converter assert that its own worst case
fits.  Shrinking the buffer, or NLS_MAX_CHARSET_SIZE growing, now fails
the build instead of silently overflowing:

  fs/isofs/joliet.c:46:1: error: static assertion failed:
    "(255 >> 1) * NLS_MAX_CHARSET_SIZE + 1 <= ISOFS_NAME_BUF_SIZE"

This follows fs/ntfs3, which couples its name buffer to the on-disk
length field the same way (fs/ntfs3/dir.c: static_assert(NTFS_NAME_LEN *
4 < PATH_MAX)).

No functional change.

Signed-off-by: Matthias Goergens <matthias.goergens@xxxxxxxxx>
---
 fs/isofs/dir.c    | 9 ++++++++-
 fs/isofs/isofs.h  | 9 +++++++++
 fs/isofs/joliet.c | 9 ++++++++-
 fs/isofs/namei.c  | 2 +-
 fs/isofs/rock.c   | 3 +++
 5 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/fs/isofs/dir.c b/fs/isofs/dir.c
index c7ca7603e97a..87c64612ef7f 100644
--- a/fs/isofs/dir.c
+++ b/fs/isofs/dir.c
@@ -50,6 +50,13 @@ int isofs_name_translate(struct iso_directory_record *de, char *new, struct inod
 }
 
 /* Acorn extensions written by Matthew Wilcox <willy@xxxxxxxxxxxxx> 1998 */
+/*
+ * isofs_name_translate() copies at most de->name_len[0] bytes one for one,
+ * and get_acorn_filename() may append "," plus three hex digits and a NUL.
+ * de->name_len is a single byte.
+ */
+static_assert(255 + 5 <= ISOFS_NAME_BUF_SIZE);
+
 int get_acorn_filename(struct iso_directory_record *de,
 			    char *retname, struct inode *inode)
 {
@@ -237,7 +244,7 @@ static int isofs_readdir(struct file *file, struct dir_context *ctx)
 	char *tmpname;
 	struct inode *inode = file_inode(file);
 
-	tmpname = kmalloc(1024, GFP_KERNEL);
+	tmpname = kmalloc(ISOFS_NAME_BUF_SIZE, GFP_KERNEL);
 	if (tmpname == NULL)
 		return -ENOMEM;
 
diff --git a/fs/isofs/isofs.h b/fs/isofs/isofs.h
index dacb9cdae4fd..2af41cc23f38 100644
--- a/fs/isofs/isofs.h
+++ b/fs/isofs/isofs.h
@@ -5,6 +5,15 @@
 #include <linux/iso_fs.h>
 #include <linux/unaligned.h>
 
+/*
+ * Scratch buffer for converting an on-disk name to its in-kernel form.
+ * Allocated by isofs_readdir() and isofs_lookup(), written by
+ * get_rock_ridge_filename(), get_joliet_filename(), get_acorn_filename()
+ * and isofs_name_translate().  Each of those asserts that its own worst
+ * case fits, next to the code that does the writing.
+ */
+#define ISOFS_NAME_BUF_SIZE 1024
+
 enum isofs_file_format {
 	isofs_file_normal = 0,
 	isofs_file_sparse = 1,
diff --git a/fs/isofs/joliet.c b/fs/isofs/joliet.c
index c0f04a1e7f69..b1f4a105ee87 100644
--- a/fs/isofs/joliet.c
+++ b/fs/isofs/joliet.c
@@ -38,6 +38,13 @@ uni16_to_x8(unsigned char *ascii, __be16 *uni, int len, struct nls_table *nls)
 	return (op - ascii);
 }
 
+/*
+ * The worst case is the iocharset path: de->name_len is a single byte, so
+ * at most 255 >> 1 UTF-16 units, each of which uni2char() may expand to
+ * NLS_MAX_CHARSET_SIZE bytes, plus the terminator.
+ */
+static_assert((255 >> 1) * NLS_MAX_CHARSET_SIZE + 1 <= ISOFS_NAME_BUF_SIZE);
+
 int
 get_joliet_filename(struct iso_directory_record * de, unsigned char *outname, struct inode * inode)
 {
@@ -49,7 +56,7 @@ get_joliet_filename(struct iso_directory_record * de, unsigned char *outname, st
 	if (!nls) {
 		len = utf16s_to_utf8s((const wchar_t *) de->name,
 				de->name_len[0] >> 1, UTF16_BIG_ENDIAN,
-				outname, PAGE_SIZE);
+				outname, ISOFS_NAME_BUF_SIZE);
 	} else {
 		len = uni16_to_x8(outname, (__be16 *) de->name,
 				de->name_len[0] >> 1, nls);
diff --git a/fs/isofs/namei.c b/fs/isofs/namei.c
index 010682f5901a..0146aef58b10 100644
--- a/fs/isofs/namei.c
+++ b/fs/isofs/namei.c
@@ -153,7 +153,7 @@ struct dentry *isofs_lookup(struct inode *dir, struct dentry *dentry, unsigned i
 	struct inode *inode;
 	char *tmpname;
 
-	tmpname = kmalloc(1024, GFP_USER);
+	tmpname = kmalloc(ISOFS_NAME_BUF_SIZE, GFP_USER);
 	if (!tmpname)
 		return ERR_PTR(-ENOMEM);
 
diff --git a/fs/isofs/rock.c b/fs/isofs/rock.c
index 2628f31bd3a5..7d95443917ea 100644
--- a/fs/isofs/rock.c
+++ b/fs/isofs/rock.c
@@ -204,6 +204,9 @@ static int rock_check_overflow(struct rock_state *rs, int sig)
 /*
  * return length of name field; 0: not found, -1: to be ignored
  */
+/* get_rock_ridge_filename() bounds the name it builds by NAME_MAX, plus a NUL. */
+static_assert(NAME_MAX + 1 <= ISOFS_NAME_BUF_SIZE);
+
 int get_rock_ridge_filename(struct iso_directory_record *de,
 			    char *retname, struct inode *inode)
 {
-- 
2.55.0