coreboot-mirror
/
ffs
mirror of https://review.coreboot.org/ffs.git
1
0
Fork 0
Code Issues Releases Wiki Activity

fix high memory usage during any operation with '--buffer' 

The --buffer command line option was passed to setvbuf(3).
Computers are fast enough and libc is good enough that
we realistically do not need this.

Hostboot's buildpnor.pl would pass --buffer 0x40000000
which resulted in a malloc(2GB), which would fail on systems
with less than 2GB of memory because sometimes
libc and the kernel are sane.

So, we keep the command line option for backwards compatibility
but completely ignore it.

In the few places where it was used to allocate a buffer to do
work in (and this was via a rather round-about way), we instead
just allocate something the size of the PNOR image.

Fixes: https://github.com/open-power/ffs/issues/7
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
This commit is contained in:
Stewart Smith 2017-11-30 18:43:56 +11:00
parent 1ccd69266b
commit 6bd1a1fe3e
16 changed files with 17 additions and 151 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
fcp/src/cmd_copy.c
View File

@ -425,16 +425,6 @@ static int __copy_compare(args_t * args, off_t offset, entry_list_t * done_list)
if (__ffs_info(src_ffs, FFS_INFO_BLOCK_SIZE, &block_size) < 0)
return -1;
if (args->buffer != NULL) {
uint32_t buffer;
if (parse_size(args->buffer, &buffer) < 0)
return -1;
if (__ffs_buffer(src_ffs, buffer) < 0)
return -1;
if (__ffs_buffer(dst_ffs, buffer) < 0)
return -1;
}
ffs_entry_t src_parent;
ffs_entry_t dst_parent;

8
fcp/src/cmd_erase.c
View File

@ -91,14 +91,6 @@ static int __erase(args_t * args, off_t offset, entry_list_t * done_list)
if (__ffs_info(ffs, FFS_INFO_BLOCK_SIZE, &block_size) < 0)
return -1;
if (args->buffer != NULL) {
uint32_t buffer;
if (parse_size(args->buffer, &buffer) < 0)
return -1;
if (__ffs_buffer(ffs, buffer) < 0)
return -1;
}
ffs_entry_t parent;
if (__ffs_entry_find(ffs, name, &parent) == false) {
UNEXPECTED("partition entry '%s' not found\n", name);

8
fcp/src/cmd_read.c
View File

@ -83,14 +83,6 @@ static int __read(args_t * args, off_t offset, entry_list_t * done_list)
if (__ffs_info(ffs, FFS_INFO_BLOCK_SIZE, &block_size) < 0)
return -1;
if (args->buffer != NULL) {
uint32_t buffer;
if (parse_size(args->buffer, &buffer) < 0)
return -1;
if (__ffs_buffer(ffs, buffer) < 0)
return -1;
}
ffs_entry_t entry;
if (__ffs_entry_find(ffs, name, &entry) == false) {
UNEXPECTED("partition entry '%s' not found\n", name);

8
fcp/src/cmd_trunc.c
View File

@ -80,14 +80,6 @@ static int __trunc(args_t * args, off_t offset)
if (__ffs_info(ffs, FFS_INFO_BLOCK_SIZE, &block_size) < 0)
return -1;
if (args->buffer != NULL) {
uint32_t buffer;
if (parse_size(args->buffer, &buffer) < 0)
return -1;
if (__ffs_buffer(ffs, buffer) < 0)
return -1;
}
ffs_entry_t entry;
if (__ffs_entry_find(ffs, name, &entry) == false) {
UNEXPECTED("partition entry '%s' not found\n", name);

8
fcp/src/cmd_user.c
View File

@ -82,14 +82,6 @@ static int __user(args_t * args, off_t offset)
if (__ffs_info(ffs, FFS_INFO_BLOCK_SIZE, &block_size) < 0)
return -1;
if (args->buffer != NULL) {
uint32_t buffer;
if (parse_size(args->buffer, &buffer) < 0)
return -1;
if (__ffs_buffer(ffs, buffer) < 0)
return -1;
}
ffs_entry_t entry;
if (__ffs_entry_find(ffs, name, &entry) == false) {
UNEXPECTED("partition entry '%s' not found\n", name);

8
fcp/src/cmd_write.c
View File

@ -85,14 +85,6 @@ static int __write(args_t * args, off_t offset, entry_list_t * done_list)
if (__ffs_info(ffs, FFS_INFO_BLOCK_SIZE, &block_size) < 0)
return -1;
if (args->buffer != NULL) {
uint32_t buffer;
if (parse_size(args->buffer, &buffer) < 0)
return -1;
if (__ffs_buffer(ffs, buffer) < 0)
return -1;
}
ffs_entry_t entry;
if (__ffs_entry_find(ffs, name, &entry) == false) {
UNEXPECTED("partition entry '%s' not found\n", name);

12
fcp/src/main.c
View File

@ -205,9 +205,7 @@ static void usage(bool verbose)
fprintf(e, " -b, --buffer <value>\n");
if (verbose)
fprintf(e,
"\n Specifies the buffer size used by --read, --write,"
" --copy and --compare\n commands <value> is a "
"decimal (or hex) number, default is buffer size.\n\n");
"\n Ignored.\n\n");
fprintf(e, "\n");
/* =============================== */
@ -266,7 +264,7 @@ static int process_argument(args_t * args, int opt, const char *optarg)
args->offset = strdup(optarg);
break;
case o_BUFFER: /* buffer */
args->buffer = strdup(optarg);
/* We ignore it, it's useless but kept for backwards compat */
break;
case f_FORCE: /* force */
args->force = (flag_t) opt;
@ -449,7 +447,6 @@ static int validate_args(args_t * args)
return -1;
}
UNSUP_OPT(buffer, probe);
} else if (args->cmd == c_LIST) {
void syntax(void) {
fprintf(stderr, "Syntax: %s [<dst_type>:]<dst_target>"
@ -462,7 +459,6 @@ static int validate_args(args_t * args)
return -1;
}
UNSUP_OPT(buffer, list);
} else if (args->cmd == c_READ) {
void syntax(void) {
fprintf(stderr, "Syntax: %s [<src_type>:]<src_source>"
@ -520,7 +516,6 @@ static int validate_args(args_t * args)
REQ_FIELD(dst_name, trunc);
UNSUP_OPT(buffer, trunc);
} else if (args->cmd == c_USER) {
void syntax(void) {
fprintf(stderr, "Syntax: %s [<dst_type>:]<dst_target>"
@ -531,7 +526,6 @@ static int validate_args(args_t * args)
REQ_FIELD(dst_name, user);
UNSUP_OPT(buffer, user);
} else if (args->cmd == c_COPY) {
void syntax(void) {
fprintf(stderr, "Syntax: %s [<src_type>:]<src_target>"
@ -614,8 +608,6 @@ static void args_dump(args_t * args)
printf("cmd[%c]\n", args->cmd);
if (args->offset != NULL)
printf("offset[%s]\n", args->offset);
if (args->buffer != NULL)
printf("buffer[%s]\n", args->buffer);
if (args->force != 0)
printf("force[%c]\n", args->force);
if (args->protected != 0)

1
fcp/src/main.h
View File

@ -90,7 +90,6 @@ typedef struct {
/* options */
const char *offset;
const char *buffer;
/* flags */
flag_t force;

30
fcp/src/misc.c
View File

@ -551,11 +551,11 @@ int fcp_read_entry(ffs_t * src, const char * name, FILE * out)
if (__ffs_info(src, FFS_INFO_BLOCK_SIZE, &block_size) < 0)
return -1;
uint32_t buffer_count;
if (__ffs_info(src, FFS_INFO_BUFFER_COUNT, &buffer_count) < 0)
uint32_t block_count;
if (__ffs_info(src, FFS_INFO_BLOCK_COUNT, &block_count) < 0)
return -1;
size_t buffer_size = block_size * buffer_count;
size_t buffer_size = block_size * block_count;
RAII(void*, buffer, malloc(buffer_size), free);
if (buffer == NULL) {
ERRNO(errno);
@ -621,11 +621,11 @@ int fcp_write_entry(ffs_t * dst, const char * name, FILE * in)
if (__ffs_info(dst, FFS_INFO_BLOCK_SIZE, &block_size) < 0)
return -1;
uint32_t buffer_count;
if (__ffs_info(dst, FFS_INFO_BUFFER_COUNT, &buffer_count) < 0)
uint32_t block_count;
if (__ffs_info(dst, FFS_INFO_BLOCK_COUNT, &block_count) < 0)
return -1;
size_t buffer_size = block_size * buffer_count;
size_t buffer_size = block_size * block_count;
RAII(void*, buffer, malloc(buffer_size), free);
if (buffer == NULL) {
ERRNO(errno);
@ -697,11 +697,11 @@ int fcp_erase_entry(ffs_t * dst, const char * name, char fill)
if (__ffs_info(dst, FFS_INFO_BLOCK_SIZE, &block_size) < 0)
return -1;
uint32_t buffer_count;
if (__ffs_info(dst, FFS_INFO_BUFFER_COUNT, &buffer_count) < 0)
uint32_t block_count;
if (__ffs_info(dst, FFS_INFO_BLOCK_COUNT, &block_count) < 0)
return -1;
size_t buffer_size = block_size * buffer_count;
size_t buffer_size = block_size * block_count;
RAII(void*, buffer, malloc(buffer_size), free);
if (buffer == NULL) {
ERRNO(errno);
@ -774,11 +774,11 @@ int fcp_copy_entry(ffs_t * src, const char * src_name,
if (__ffs_info(src, FFS_INFO_BLOCK_SIZE, &block_size) < 0)
return -1;
uint32_t buffer_count;
if (__ffs_info(dst, FFS_INFO_BUFFER_COUNT, &buffer_count) < 0)
uint32_t block_count;
if (__ffs_info(dst, FFS_INFO_BLOCK_COUNT, &block_count) < 0)
return -1;
size_t buffer_size = block_size * buffer_count;
size_t buffer_size = block_size * block_count;
RAII(void*, buffer, malloc(buffer_size), free);
if (buffer == NULL) {
ERRNO(errno);
@ -851,11 +851,11 @@ int fcp_compare_entry(ffs_t * src, const char * src_name,
if (__ffs_info(src, FFS_INFO_BLOCK_SIZE, &block_size) < 0)
return -1;
uint32_t buffer_count;
if (__ffs_info(dst, FFS_INFO_BUFFER_COUNT, &buffer_count) < 0)
uint32_t block_count;
if (__ffs_info(dst, FFS_INFO_BLOCK_COUNT, &block_count) < 0)
return -1;
size_t buffer_size = block_size * buffer_count;
size_t buffer_size = block_size * block_count;
RAII(void*, src_buffer, malloc(buffer_size), free);
if (src_buffer == NULL) {

7
ffs/libffs.h
View File

@ -52,9 +52,6 @@ struct ffs {
FILE * file;
uint32_t count;
void * buf; // FILE* buffer
uint32_t buf_count; // multiple of hdr->block_size
bool dirty;
};
@ -76,7 +73,6 @@ typedef struct ffs_exception ffs_exception_t;
#define FFS_INFO_ENTRY_COUNT 4
#define FFS_INFO_BLOCK_SIZE 5
#define FFS_INFO_BLOCK_COUNT 6
#define FFS_INFO_BUFFER_COUNT 7
#define FFS_INFO_OFFSET 8
#define FFS_CHECK_PATH -3
@ -109,9 +105,6 @@ extern ffs_t * __ffs_open(const char *, off_t)
extern int __ffs_info(ffs_t *, int, uint32_t *)
/*! @cond */ __nonnull ((1)) /*! @endcond */ ;
extern int __ffs_buffer(ffs_t *, size_t)
/*! @cond */ __nonnull ((1)) /*! @endcond */ ;
extern int __ffs_close(ffs_t *)
/*! @cond */ __nonnull ((1)) /*! @endcond */ ;

49
ffs/src/libffs.c
View File

@ -583,25 +583,9 @@ ffs_t *__ffs_fopen(FILE * file, off_t offset)
goto error;
}
self->buf_count = 1; // default to 1
self->buf = malloc(self->buf_count * self->hdr->block_size);
if (self->hdr == NULL) {
ERRNO(errno);
goto error;
}
if (setvbuf(self->file, self->buf, _IOFBF,
self->buf_count * self->hdr->block_size) != 0) {
ERRNO(errno);
goto error;
}
if (false) {
error:
if (self != NULL) {
if (self->buf != NULL)
free(self->buf), self->buf = NULL;
if (self->hdr != NULL)
free(self->hdr), self->hdr = NULL;
@ -670,9 +654,6 @@ int __ffs_info(ffs_t * self, int name, uint32_t *value)
case FFS_INFO_BLOCK_COUNT:
*value = self->hdr->block_count;
break;
case FFS_INFO_BUFFER_COUNT:
*value = self->buf_count;
break;
case FFS_INFO_OFFSET:
*value = self->offset;
break;
@ -684,34 +665,6 @@ int __ffs_info(ffs_t * self, int name, uint32_t *value)
return 0;
}
int __ffs_buffer(ffs_t * self, size_t size)
{
assert(self != NULL);
if (size == 0)
size = self->hdr->block_size;
if (self->buf != NULL) {
free(self->buf);
self->buf_count = 0;
}
self->buf_count = size / self->hdr->block_size;
size = self->buf_count * self->hdr->block_size;
self->buf = malloc(size);
if (self->buf == NULL) {
ERRNO(errno);
return -1;
}
if (setvbuf(self->file, self->buf, _IOFBF, size) < 0) {
ERRNO(errno);
return -1;
}
return 0;
}
int __ffs_fclose(ffs_t * self)
{
@ -722,8 +675,6 @@ int __ffs_fclose(ffs_t * self)
if (ffs_flush(self) < 0)
return -1;
if (self->buf != NULL)
free(self->buf), self->buf = NULL;
if (self->hdr != NULL)
free(self->hdr), self->hdr = NULL;

5
fpart/src/cmd_compare.c
View File

@ -131,11 +131,6 @@ int command_compare(args_t * args)
}
}
if (__ffs_buffer(__in, block_size) < 0)
return -1;
if (__ffs_buffer(__out, block_size) < 0)
return -1;
RAII(void*, src_block, malloc(block_size), free);
if (src_block == NULL) {
ERRNO(errno);

5
fpart/src/cmd_copy.c
View File

@ -155,11 +155,6 @@ int command_copy(args_t * args)
}
}
if (__ffs_buffer(__in, block_size) < 0)
return -1;
if (__ffs_buffer(__out, block_size) < 0)
return -1;
RAII(void*, block, malloc(block_size), free);
if (block == NULL) {
ERRNO(errno);

3
fpart/src/cmd_erase.c
View File

@ -144,9 +144,6 @@ int command_erase(args_t * args)
}
}
if (__ffs_buffer(__ffs, block_size) < 0)
return -1;
RAII(void*, block, malloc(block_size), free);
if (block == NULL) {
ERRNO(errno);

3
fpart/src/cmd_read.c
View File

@ -99,9 +99,6 @@ int command_read(args_t * args)
}
}
if (__ffs_buffer(ffs, block_size) < 0)
return -1;
RAII(void*, block, malloc(block_size), free);
if (block == NULL) {
ERRNO(errno);

3
fpart/src/cmd_write.c
View File

@ -149,9 +149,6 @@ int command_write(args_t * args)
}
}
if (__ffs_buffer(__ffs, block_size) < 0)
return -1;
RAII(void*, block, malloc(block_size), free);
if (block == NULL) {
ERRNO(errno);