* [U-Boot] [PATCH] fs/fat: fix fatbuf leak
@ 2017-09-12 20:40 Rob Clark
2017-09-13 4:27 ` Simon Glass
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Rob Clark @ 2017-09-12 20:40 UTC (permalink / raw)
To: u-boot
A new fatbuf was allocated by get_fs_info() (called by fat_itr_root()),
but not freed, resulting in eventually running out of memory. Spotted
by running 'ls -r' in a large FAT filesystem from Shell.efi.
fatbuf is mainly used to cache FAT entry lookups (get_fatent())..
possibly once fat_write.c it can move into the iterator to simplify
this.
Signed-off-by: Rob Clark <robdclark@gmail.com>
---
I can squash this back in to the earlier readdir patches and resend
them if that is preferred.
fs/fat/fat.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index f5f74c12ff..f0284398b4 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -1042,6 +1042,7 @@ int fat_exists(const char *filename)
return 0;
ret = fat_itr_resolve(itr, filename, TYPE_ANY);
+ free(fsdata.fatbuf);
return ret == 0;
}
@@ -1061,17 +1062,19 @@ int fat_size(const char *filename, loff_t *size)
* Directories don't have size, but fs_size() is not
* expected to fail if passed a directory path:
*/
+ free(fsdata.fatbuf);
fat_itr_root(itr, &fsdata);
if (!fat_itr_resolve(itr, filename, TYPE_DIR)) {
*size = 0;
- return 0;
+ ret = 0;
}
- return ret;
+ goto out;
}
*size = FAT2CPU32(itr->dent->size);
-
- return 0;
+out:
+ free(fsdata.fatbuf);
+ return ret;
}
int file_fat_read_at(const char *filename, loff_t pos, void *buffer,
@@ -1087,10 +1090,14 @@ int file_fat_read_at(const char *filename, loff_t pos, void *buffer,
ret = fat_itr_resolve(itr, filename, TYPE_FILE);
if (ret)
- return ret;
+ goto out;
printf("reading %s\n", filename);
- return get_contents(&fsdata, itr->dent, pos, buffer, maxsize, actread);
+ ret = get_contents(&fsdata, itr->dent, pos, buffer, maxsize, actread);
+
+out:
+ free(fsdata.fatbuf);
+ return ret;
}
int file_fat_read(const char *filename, void *buffer, int maxsize)
@@ -1126,7 +1133,7 @@ typedef struct {
int fat_opendir(const char *filename, struct fs_dir_stream **dirsp)
{
- fat_dir *dir = malloc(sizeof(*dir));
+ fat_dir *dir = calloc(1, sizeof(*dir));
int ret;
if (!dir)
@@ -1144,6 +1151,7 @@ int fat_opendir(const char *filename, struct fs_dir_stream **dirsp)
return 0;
fail:
+ free(dir->fsdata.fatbuf);
free(dir);
return ret;
}
@@ -1174,6 +1182,7 @@ int fat_readdir(struct fs_dir_stream *dirs, struct fs_dirent **dentp)
void fat_closedir(struct fs_dir_stream *dirs)
{
fat_dir *dir = (fat_dir *)dirs;
+ free(dir->fsdata.fatbuf);
free(dir);
}
--
2.13.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] fs/fat: fix fatbuf leak
2017-09-12 20:40 [U-Boot] [PATCH] fs/fat: fix fatbuf leak Rob Clark
@ 2017-09-13 4:27 ` Simon Glass
2017-09-13 11:10 ` Łukasz Majewski
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Simon Glass @ 2017-09-13 4:27 UTC (permalink / raw)
To: u-boot
On 12 September 2017 at 14:40, Rob Clark <robdclark@gmail.com> wrote:
> A new fatbuf was allocated by get_fs_info() (called by fat_itr_root()),
> but not freed, resulting in eventually running out of memory. Spotted
> by running 'ls -r' in a large FAT filesystem from Shell.efi.
>
> fatbuf is mainly used to cache FAT entry lookups (get_fatent())..
> possibly once fat_write.c it can move into the iterator to simplify
> this.
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
> I can squash this back in to the earlier readdir patches and resend
> them if that is preferred.
>
> fs/fat/fat.c | 23 ++++++++++++++++-------
> 1 file changed, 16 insertions(+), 7 deletions(-)
Reviewed-by: Simon Glass <sjg@chromium.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] fs/fat: fix fatbuf leak
2017-09-12 20:40 [U-Boot] [PATCH] fs/fat: fix fatbuf leak Rob Clark
2017-09-13 4:27 ` Simon Glass
@ 2017-09-13 11:10 ` Łukasz Majewski
2017-09-13 16:01 ` Tom Rini
2017-09-16 2:32 ` [U-Boot] " Tom Rini
3 siblings, 0 replies; 5+ messages in thread
From: Łukasz Majewski @ 2017-09-13 11:10 UTC (permalink / raw)
To: u-boot
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 3095 bytes --]
On 09/12/2017 10:40 PM, Rob Clark wrote:
> A new fatbuf was allocated by get_fs_info() (called by fat_itr_root()),
> but not freed, resulting in eventually running out of memory. Spotted
> by running 'ls -r' in a large FAT filesystem from Shell.efi.
>
> fatbuf is mainly used to cache FAT entry lookups (get_fatent())..
> possibly once fat_write.c it can move into the iterator to simplify
> this.
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
> I can squash this back in to the earlier readdir patches and resend
> them if that is preferred.
>
> fs/fat/fat.c | 23 ++++++++++++++++-------
> 1 file changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index f5f74c12ff..f0284398b4 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -1042,6 +1042,7 @@ int fat_exists(const char *filename)
> return 0;
>
> ret = fat_itr_resolve(itr, filename, TYPE_ANY);
> + free(fsdata.fatbuf);
> return ret == 0;
> }
>
> @@ -1061,17 +1062,19 @@ int fat_size(const char *filename, loff_t *size)
> * Directories don't have size, but fs_size() is not
> * expected to fail if passed a directory path:
> */
> + free(fsdata.fatbuf);
> fat_itr_root(itr, &fsdata);
> if (!fat_itr_resolve(itr, filename, TYPE_DIR)) {
> *size = 0;
> - return 0;
> + ret = 0;
> }
> - return ret;
> + goto out;
> }
>
> *size = FAT2CPU32(itr->dent->size);
> -
> - return 0;
> +out:
> + free(fsdata.fatbuf);
> + return ret;
> }
>
> int file_fat_read_at(const char *filename, loff_t pos, void *buffer,
> @@ -1087,10 +1090,14 @@ int file_fat_read_at(const char *filename, loff_t pos, void *buffer,
>
> ret = fat_itr_resolve(itr, filename, TYPE_FILE);
> if (ret)
> - return ret;
> + goto out;
>
> printf("reading %s\n", filename);
> - return get_contents(&fsdata, itr->dent, pos, buffer, maxsize, actread);
> + ret = get_contents(&fsdata, itr->dent, pos, buffer, maxsize, actread);
> +
> +out:
> + free(fsdata.fatbuf);
> + return ret;
> }
>
> int file_fat_read(const char *filename, void *buffer, int maxsize)
> @@ -1126,7 +1133,7 @@ typedef struct {
>
> int fat_opendir(const char *filename, struct fs_dir_stream **dirsp)
> {
> - fat_dir *dir = malloc(sizeof(*dir));
> + fat_dir *dir = calloc(1, sizeof(*dir));
> int ret;
>
> if (!dir)
> @@ -1144,6 +1151,7 @@ int fat_opendir(const char *filename, struct fs_dir_stream **dirsp)
> return 0;
>
> fail:
> + free(dir->fsdata.fatbuf);
> free(dir);
> return ret;
> }
> @@ -1174,6 +1182,7 @@ int fat_readdir(struct fs_dir_stream *dirs, struct fs_dirent **dentp)
> void fat_closedir(struct fs_dir_stream *dirs)
> {
> fat_dir *dir = (fat_dir *)dirs;
> + free(dir->fsdata.fatbuf);
> free(dir);
> }
>
>
Reviewed-by: Łukasz Majewski <lukma@denx.de>
--
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] fs/fat: fix fatbuf leak
2017-09-12 20:40 [U-Boot] [PATCH] fs/fat: fix fatbuf leak Rob Clark
2017-09-13 4:27 ` Simon Glass
2017-09-13 11:10 ` Łukasz Majewski
@ 2017-09-13 16:01 ` Tom Rini
2017-09-16 2:32 ` [U-Boot] " Tom Rini
3 siblings, 0 replies; 5+ messages in thread
From: Tom Rini @ 2017-09-13 16:01 UTC (permalink / raw)
To: u-boot
On Tue, Sep 12, 2017 at 04:40:01PM -0400, Rob Clark wrote:
> A new fatbuf was allocated by get_fs_info() (called by fat_itr_root()),
> but not freed, resulting in eventually running out of memory. Spotted
> by running 'ls -r' in a large FAT filesystem from Shell.efi.
>
> fatbuf is mainly used to cache FAT entry lookups (get_fatent())..
> possibly once fat_write.c it can move into the iterator to simplify
> this.
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
> I can squash this back in to the earlier readdir patches and resend
> them if that is preferred.
Separate is fine, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170913/4c7383c7/attachment.sig>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] fs/fat: fix fatbuf leak
2017-09-12 20:40 [U-Boot] [PATCH] fs/fat: fix fatbuf leak Rob Clark
` (2 preceding siblings ...)
2017-09-13 16:01 ` Tom Rini
@ 2017-09-16 2:32 ` Tom Rini
3 siblings, 0 replies; 5+ messages in thread
From: Tom Rini @ 2017-09-16 2:32 UTC (permalink / raw)
To: u-boot
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 869 bytes --]
On Tue, Sep 12, 2017 at 04:40:01PM -0400, Rob Clark wrote:
> A new fatbuf was allocated by get_fs_info() (called by fat_itr_root()),
> but not freed, resulting in eventually running out of memory. Spotted
> by running 'ls -r' in a large FAT filesystem from Shell.efi.
>
> fatbuf is mainly used to cache FAT entry lookups (get_fatent())..
> possibly once fat_write.c it can move into the iterator to simplify
> this.
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Łukasz Majewski <lukma@denx.de>
Applied to u-boot/master, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170915/6e5fa42e/attachment.sig>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-09-16 2:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-12 20:40 [U-Boot] [PATCH] fs/fat: fix fatbuf leak Rob Clark
2017-09-13 4:27 ` Simon Glass
2017-09-13 11:10 ` Łukasz Majewski
2017-09-13 16:01 ` Tom Rini
2017-09-16 2:32 ` [U-Boot] " Tom Rini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.