[PATCH stable 5.15 0/2] backport: udf: Allocate name buffer in directory iterator on heap

[PATCH stable 5.15 0/2] backport: udf: Allocate name buffer in directory iterator on heap

[Hauke Mehrtens]

I am running into this compile error with Linux kernel 5.15.171 in OpenWrt on 32 bit systems.
```
fs/udf/namei.c: In function 'udf_rename':
fs/udf/namei.c:878:1: error: the frame size of 1144 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
  878 | }
      | ^
cc1: all warnings being treated as errors
make[2]: *** [scripts/Makefile.build:289: fs/udf/namei.o] Error 1
make[1]: *** [scripts/Makefile.build:552: fs/udf] Error 2
``` 

This problem was introduced with kernel 5.15.169.
The first patch needs an extra linux/slab.h include on x86, which is the only modification I did to it compared to the upstream version.

These patches should go into 5.15. They were already backported to kernel 6.1.

Jan Kara (2):
  udf: Allocate name buffer in directory iterator on heap
  udf: Avoid directory type conversion failure due to ENOMEM

 fs/udf/directory.c | 27 +++++++++++++++++++--------
 fs/udf/udfdecl.h   |  2 +-
 2 files changed, 20 insertions(+), 9 deletions(-)

-- 
2.47.0

[PATCH stable 5.15 1/2] udf: Allocate name buffer in directory iterator on heap

[Hauke Mehrtens]

From: Jan Kara <jack@suse.cz>

commit 0aba4860b0d0216a1a300484ff536171894d49d8 upstream.

Currently we allocate name buffer in directory iterators (struct
udf_fileident_iter) on stack. These structures are relatively large
(some 360 bytes on 64-bit architectures). For udf_rename() which needs
to keep three of these structures in parallel the stack usage becomes
rather heavy - 1536 bytes in total. Allocate the name buffer in the
iterator from heap to avoid excessive stack usage.

Link: https://lore.kernel.org/all/202212200558.lK9x1KW0-lkp@intel.com
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Jan Kara <jack@suse.cz>
[Add extra include linux/slab.h]
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 fs/udf/directory.c | 24 ++++++++++++++++--------
 fs/udf/udfdecl.h   |  2 +-
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/fs/udf/directory.c b/fs/udf/directory.c
index e97ffae07833..a30898debdd1 100644
--- a/fs/udf/directory.c
+++ b/fs/udf/directory.c
@@ -19,6 +19,7 @@
 #include <linux/bio.h>
 #include <linux/crc-itu-t.h>
 #include <linux/iversion.h>
+#include <linux/slab.h>
 
 static int udf_verify_fi(struct udf_fileident_iter *iter)
 {
@@ -248,9 +249,14 @@ int udf_fiiter_init(struct udf_fileident_iter *iter, struct inode *dir,
 	iter->elen = 0;
 	iter->epos.bh = NULL;
 	iter->name = NULL;
+	iter->namebuf = kmalloc(UDF_NAME_LEN_CS0, GFP_KERNEL);
+	if (!iter->namebuf)
+		return -ENOMEM;
 
-	if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB)
-		return udf_copy_fi(iter);
+	if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB) {
+		err = udf_copy_fi(iter);
+		goto out;
+	}
 
 	if (inode_bmap(dir, iter->pos >> dir->i_blkbits, &iter->epos,
 		       &iter->eloc, &iter->elen, &iter->loffset) !=
@@ -260,17 +266,17 @@ int udf_fiiter_init(struct udf_fileident_iter *iter, struct inode *dir,
 		udf_err(dir->i_sb,
 			"position %llu not allocated in directory (ino %lu)\n",
 			(unsigned long long)pos, dir->i_ino);
-		return -EFSCORRUPTED;
+		err = -EFSCORRUPTED;
+		goto out;
 	}
 	err = udf_fiiter_load_bhs(iter);
 	if (err < 0)
-		return err;
+		goto out;
 	err = udf_copy_fi(iter);
-	if (err < 0) {
+out:
+	if (err < 0)
 		udf_fiiter_release(iter);
-		return err;
-	}
-	return 0;
+	return err;
 }
 
 int udf_fiiter_advance(struct udf_fileident_iter *iter)
@@ -307,6 +313,8 @@ void udf_fiiter_release(struct udf_fileident_iter *iter)
 	brelse(iter->bh[0]);
 	brelse(iter->bh[1]);
 	iter->bh[0] = iter->bh[1] = NULL;
+	kfree(iter->namebuf);
+	iter->namebuf = NULL;
 }
 
 static void udf_copy_to_bufs(void *buf1, int len1, void *buf2, int len2,
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index f764b4d15094..d35aa42bb577 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -99,7 +99,7 @@ struct udf_fileident_iter {
 	struct extent_position epos;	/* Position after the above extent */
 	struct fileIdentDesc fi;	/* Copied directory entry */
 	uint8_t *name;			/* Pointer to entry name */
-	uint8_t namebuf[UDF_NAME_LEN_CS0]; /* Storage for entry name in case
+	uint8_t *namebuf;		/* Storage for entry name in case
 					 * the name is split between two blocks
 					 */
 };
-- 
2.47.0

[PATCH stable 5.15 2/2] udf: Avoid directory type conversion failure due to ENOMEM

[Hauke Mehrtens]

From: Jan Kara <jack@suse.cz>

commit df97f64dfa317a5485daf247b6c043a584ef95f9 upstream.

When converting directory from in-ICB to normal format, the last
iteration through the directory fixing up directory enteries can fail
due to ENOMEM. We do not expect this iteration to fail since the
directory is already verified to be correct and it is difficult to undo
the conversion at this point. So just use GFP_NOFAIL to make sure the
small allocation cannot fail.

Reported-by: syzbot+111eaa994ff74f8d440f@syzkaller.appspotmail.com
Fixes: 0aba4860b0d0 ("udf: Allocate name buffer in directory iterator on heap")
Signed-off-by: Jan Kara <jack@suse.cz>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 fs/udf/directory.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/udf/directory.c b/fs/udf/directory.c
index a30898debdd1..4f6c7b546bea 100644
--- a/fs/udf/directory.c
+++ b/fs/udf/directory.c
@@ -249,9 +249,12 @@ int udf_fiiter_init(struct udf_fileident_iter *iter, struct inode *dir,
 	iter->elen = 0;
 	iter->epos.bh = NULL;
 	iter->name = NULL;
-	iter->namebuf = kmalloc(UDF_NAME_LEN_CS0, GFP_KERNEL);
-	if (!iter->namebuf)
-		return -ENOMEM;
+	/*
+	 * When directory is verified, we don't expect directory iteration to
+	 * fail and it can be difficult to undo without corrupting filesystem.
+	 * So just do not allow memory allocation failures here.
+	 */
+	iter->namebuf = kmalloc(UDF_NAME_LEN_CS0, GFP_KERNEL | __GFP_NOFAIL);
 
 	if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB) {
 		err = udf_copy_fi(iter);
-- 
2.47.0

Re: [PATCH stable 5.15 0/2] backport: udf: Allocate name buffer in directory iterator on heap

[Greg KH]

On Thu, Nov 14, 2024 at 10:26:55PM +0100, Hauke Mehrtens wrote:
> I am running into this compile error with Linux kernel 5.15.171 in OpenWrt on 32 bit systems.
> ```
> fs/udf/namei.c: In function 'udf_rename':
> fs/udf/namei.c:878:1: error: the frame size of 1144 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
>   878 | }
>       | ^
> cc1: all warnings being treated as errors
> make[2]: *** [scripts/Makefile.build:289: fs/udf/namei.o] Error 1
> make[1]: *** [scripts/Makefile.build:552: fs/udf] Error 2
> ``` 
> 
> This problem was introduced with kernel 5.15.169.
> The first patch needs an extra linux/slab.h include on x86, which is the only modification I did to it compared to the upstream version.
> 
> These patches should go into 5.15. They were already backported to kernel 6.1.
> 
> Jan Kara (2):
>   udf: Allocate name buffer in directory iterator on heap
>   udf: Avoid directory type conversion failure due to ENOMEM
> 
>  fs/udf/directory.c | 27 +++++++++++++++++++--------
>  fs/udf/udfdecl.h   |  2 +-
>  2 files changed, 20 insertions(+), 9 deletions(-)
> 
> -- 
> 2.47.0
> 
> 

Now queued up, thanks.
greg k-h