From: Jan Kara jack@suse.cz
[ Upstream commit 0aba4860b0d0216a1a300484ff536171894d49d8 ]
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 --- fs/udf/directory.c | 23 +++++++++++++++-------- fs/udf/udfdecl.h | 2 +- 2 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/fs/udf/directory.c b/fs/udf/directory.c index e7e8b30876d9..a4c91905b033 100644 --- a/fs/udf/directory.c +++ b/fs/udf/directory.c @@ -248,9 +248,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 +265,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 +312,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 */ };
On Wed, Nov 13, 2024 at 01:30:35PM +0900, Sergey Senozhatsky wrote:
From: Jan Kara jack@suse.cz
[ Upstream commit 0aba4860b0d0216a1a300484ff536171894d49d8 ]
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
Your S-O-B is missing, but also it doesn't build:
fs/udf/directory.c: In function 'udf_fiiter_init': fs/udf/directory.c:251:25: error: implicit declaration of function 'kmalloc'; did you mean 'kvmalloc'? [-Werror=implicit-function-declaration] 251 | iter->namebuf = kmalloc(UDF_NAME_LEN_CS0, GFP_KERNEL); | ^~~~~~~ | kvmalloc fs/udf/directory.c:251:23: warning: assignment to 'uint8_t *' {aka 'unsigned char *'} from 'int' makes pointer from integer without a cast [-Wint-conversion] 251 | iter->namebuf = kmalloc(UDF_NAME_LEN_CS0, GFP_KERNEL); | ^ fs/udf/directory.c: In function 'udf_fiiter_release': fs/udf/directory.c:315:9: error: implicit declaration of function 'kfree'; did you mean 'kvfree'? [-Werror=implicit-function-declaration] 315 | kfree(iter->namebuf); | ^~~~~ | kvfree
On (24/11/14 13:31), Sasha Levin wrote:
On Wed, Nov 13, 2024 at 01:30:35PM +0900, Sergey Senozhatsky wrote:
From: Jan Kara jack@suse.cz
[ Upstream commit 0aba4860b0d0216a1a300484ff536171894d49d8 ]
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
Your S-O-B is missing, but also it doesn't build:
OK, didn't know that I need to add my SoB.
fs/udf/directory.c: In function 'udf_fiiter_init': fs/udf/directory.c:251:25: error: implicit declaration of function 'kmalloc'; did you mean 'kvmalloc'? [-Werror=implicit-function-declaration] 251 | iter->namebuf = kmalloc(UDF_NAME_LEN_CS0, GFP_KERNEL); | ^~~~~~~ | kvmalloc fs/udf/directory.c:251:23: warning: assignment to 'uint8_t *' {aka 'unsigned char *'} from 'int' makes pointer from integer without a cast [-Wint-conversion] 251 | iter->namebuf = kmalloc(UDF_NAME_LEN_CS0, GFP_KERNEL); | ^ fs/udf/directory.c: In function 'udf_fiiter_release': fs/udf/directory.c:315:9: error: implicit declaration of function 'kfree'; did you mean 'kvfree'? [-Werror=implicit-function-declaration] 315 | kfree(iter->namebuf); | ^~~~~ | kvfree
Hmm. Upstream fs/udf/directory.c doesn't include slab.h and 5.15 with this patch applied "builds on my computer". So I can amend Jan's patch to include slab.h, I guess? Is that okay?
linux-stable-mirror@lists.linaro.org