Skip to content

Commit

Permalink
libflash/libffs: Refcount ffs entries
Browse files Browse the repository at this point in the history
Currently consumers can add an new ffs entry to multiple headers, this
is fine but freeing any of the headers will cause the entry to be freed,
this causes double free problems.

Even if only one header is uses, the consumer of the library still has a
reference to the entry, which they may well reuse at some other point.

libffs will now refcount entries and only free when there are no more
references.

This patch also removes the pointless return value of ffs_hdr_free()

Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
  • Loading branch information
cyrilbur-ibm authored and stewartsmith committed Apr 9, 2018
1 parent 3d47dbb commit 79316cb
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 9 deletions.
2 changes: 1 addition & 1 deletion external/ffspart/ffspart.c
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ int main(int argc, char *argv[])

continue;
out_while:
free(new_entry);
ffs_entry_put(new_entry);
goto out_close_bl;
}

Expand Down
2 changes: 2 additions & 0 deletions external/pflash/pflash.c
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ static uint32_t print_ffs_info(struct ffs_handle *ffsh, uint32_t toc)
}

user = ffs_entry_user_get(ent);
ffs_entry_put(ent);
flags = ffs_entry_user_to_string(&user);
if (!flags)
goto out;
Expand Down Expand Up @@ -589,6 +590,7 @@ static void print_partition_detail(struct ffs_handle *ffsh, uint32_t part_id)
"REPROVISION [F]\n" : "",
has_flag(ent, FFS_MISCFLAGS_VOLATILE) ? "VOLATILE [V]\n" : "",
has_flag(ent, FFS_MISCFLAGS_CLEARECC) ? "CLEARECC [C]\n" : "");
ffs_entry_put(ent);
if (l < 0) {
fprintf(stderr, "Memory allocation failure printing flags!\n");
goto out;
Expand Down
2 changes: 2 additions & 0 deletions libflash/ffs.h
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ struct __ffs_entry {
* @type: Describe type of partition
* @flags: Partition attributes (optional)
* @user: User data (optional)
* @ref: Refcount
*/
struct ffs_entry {
char name[FFS_PART_NAME_MAX + 1];
Expand All @@ -161,6 +162,7 @@ struct ffs_entry {
enum ffs_type type;
uint32_t flags;
struct ffs_entry_user user;
unsigned int ref;
};


Expand Down
38 changes: 31 additions & 7 deletions libflash/libffs.c
Original file line number Diff line number Diff line change
Expand Up @@ -280,13 +280,35 @@ bool has_flag(struct ffs_entry *ent, uint16_t flag)
return ((ent->user.miscflags & flag) != 0);
}

struct ffs_entry *ffs_entry_get(struct ffs_handle *ffs, uint32_t index)
static struct ffs_entry *__ffs_entry_get(struct ffs_handle *ffs, uint32_t index)
{
if (!ffs || index >= ffs->hdr.count)
if (index >= ffs->hdr.count)
return NULL;
return ffs->hdr.entries[index];
}

struct ffs_entry *ffs_entry_get(struct ffs_handle *ffs, uint32_t index)
{
struct ffs_entry *ret = __ffs_entry_get(ffs, index);
if (ret)
ret->ref++;
return ret;
}

struct ffs_entry *ffs_entry_put(struct ffs_entry *ent)
{
if (!ent)
return NULL;

ent->ref--;
if (ent->ref == 0) {
free(ent);
ent = NULL;
}

return ent;
}

bool has_ecc(struct ffs_entry *ent)
{
return ((ent->user.datainteg & FFS_ENRY_INTEG_ECC) != 0);
Expand Down Expand Up @@ -402,6 +424,7 @@ int ffs_init(uint32_t offset, uint32_t max_size, struct blocklevel_device *bl,
}

f->hdr.entries[f->hdr.count++] = ent;
ent->ref = 1;
rc = ffs_entry_to_cpu(&f->hdr, ent, &f->cache->entries[i]);
if (rc) {
FL_DBG("FFS: Failed checksum for partition %s\n",
Expand Down Expand Up @@ -436,15 +459,14 @@ static void __hdr_free(struct ffs_hdr *hdr)
return;

for (i = 0; i < hdr->count; i++)
free(hdr->entries[i]);
ffs_entry_put(hdr->entries[i]);
free(hdr->entries);
}

int ffs_hdr_free(struct ffs_hdr *hdr)
void ffs_hdr_free(struct ffs_hdr *hdr)
{
__hdr_free(hdr);
free(hdr);
return 0;
}

void ffs_close(struct ffs_handle *ffs)
Expand Down Expand Up @@ -483,7 +505,7 @@ int ffs_part_info(struct ffs_handle *ffs, uint32_t part_idx,
struct ffs_entry *ent;
char *n;

ent = ffs_entry_get(ffs, part_idx);
ent = __ffs_entry_get(ffs, part_idx);
if (!ent)
return FFS_ERR_PART_NOT_FOUND;

Expand Down Expand Up @@ -609,6 +631,7 @@ int ffs_entry_add(struct ffs_hdr *hdr, struct ffs_entry *entry)
}
hdr->entries_size += HDR_ENTRIES_NUM;
}
entry->ref++;
hdr->entries[hdr->count++] = entry;

return 0;
Expand Down Expand Up @@ -728,6 +751,7 @@ int ffs_entry_new(const char *name, uint32_t base, uint32_t size, struct ffs_ent
ret->actual = size;
ret->pid = FFS_PID_TOPLEVEL;
ret->type = FFS_TYPE_DATA;
ret->ref = 1;

*r = ret;
return 0;
Expand Down Expand Up @@ -790,7 +814,7 @@ int ffs_update_act_size(struct ffs_handle *ffs, uint32_t part_idx,
uint32_t offset;
int rc;

ent = ffs_entry_get(ffs, part_idx);
ent = __ffs_entry_get(ffs, part_idx);
if (!ent) {
FL_DBG("FFS: Entry not found\n");
return FFS_ERR_PART_NOT_FOUND;
Expand Down
4 changes: 3 additions & 1 deletion libflash/libffs.h
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,8 @@ int ffs_hdr_add_side(struct ffs_hdr *hdr);

int ffs_entry_new(const char *name, uint32_t base, uint32_t size, struct ffs_entry **r);

struct ffs_entry *ffs_entry_put(struct ffs_entry *ent);

int ffs_entry_user_set(struct ffs_entry *ent, struct ffs_entry_user *user);

int ffs_entry_set_act_size(struct ffs_entry *ent, uint32_t actual_size);
Expand All @@ -158,5 +160,5 @@ int ffs_entry_add(struct ffs_hdr *hdr, struct ffs_entry *entry);

int ffs_hdr_finalise(struct blocklevel_device *bl, struct ffs_hdr *hdr);

int ffs_hdr_free(struct ffs_hdr *hdr);
void ffs_hdr_free(struct ffs_hdr *hdr);
#endif /* __LIBFFS_H */

0 comments on commit 79316cb

Please sign in to comment.