fs/udf/directory.c | 23 +++++++++++++++-------- fs/udf/udfdecl.h | 2 +- 2 files changed, 16 insertions(+), 9 deletions(-)
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
*/
};
--
2.47.0.277.g8800431eea-googOn 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
--
Thanks,
Sasha
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?
© 2016 - 2024 Red Hat, Inc.