1. Patchew
  2. linux
  3. View series

[PATCHv2 5.15] udf: Allocate name buffer in directory iterator on heap

Sergey Senozhatsky posted 1 patch 1 week ago 
fs/udf/directory.c | 24 ++++++++++++++++--------
fs/udf/udfdecl.h   |  2 +-
2 files changed, 17 insertions(+), 9 deletions(-)
[PATCHv2 5.15] udf: Allocate name buffer in directory iterator on heap
Posted by Sergey Senozhatsky 1 week ago 
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>
[ senozhatsky: explicitly include slab.h to address build
  failure reported by sashal@ ]
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 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.338.g60cca15819-goog
Re: [PATCHv2 5.15] udf: Allocate name buffer in directory iterator on heap
Posted by Jan Kara 1 week ago 
On Fri 15-11-24 15:08:48, 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>
> [ senozhatsky: explicitly include slab.h to address build
>   failure reported by sashal@ ]
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>

Mauke beat you to this, Greg already queued up his patches :)

[1] https://lore.kernel.org/all/20241114212657.306989-1-hauke@hauke-m.de

								Honza
> ---
>  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.338.g60cca15819-goog
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
Re: [PATCHv2 5.15] udf: Allocate name buffer in directory iterator on heap
Posted by Sergey Senozhatsky 6 days, 13 hours ago 
On (24/11/15 13:07), Jan Kara wrote:
> On Fri 15-11-24 15:08:48, 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>
> > [ senozhatsky: explicitly include slab.h to address build
> >   failure reported by sashal@ ]
> > Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> 
> Mauke beat you to this, Greg already queued up his patches :)

Story of my life :)

> [1] https://lore.kernel.org/all/20241114212657.306989-1-hauke@hauke-m.de

Awesome, thanks everyone!

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.