len0rd/rockbox
1
0
Fork 1
mirror of https://github.com/Rockbox/rockbox.git synced 2025-10-14 02:27:39 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

Dircache: Refine name allocation and error handling.

Browse source 
* 8 bits is enough to allow 260 character base names when five
bytes is the minimum indirect storage size (0..255->5..260).

* Don't truncate anything that's too long as that can lead to
bad behavior, simply don't include the offending entry in the
parent.

* Set the .tinyname flag to 1 by default to indicate that
the entry's name doesn't need freeing. Clear it only when
allocating indirect storage.

* Rename some things to help catch all instances

Change-Id: Iff747b624acbb8e03ed26c24afdf0fc715fd9d99
This commit is contained in:
Michael Sevakis 2017-03-12 20:59:44 -04:00
parent e3081b35cd
commit 70c929179b
3 changed files with 49 additions and 35 deletions
Download patch file Download diff file Expand all files Collapse all files

72
firmware/common/dircache.c
View file

@ -150,14 +150,17 @@ enum
}; };
#define MAX_TINYNAME sizeof (uint32_t) #define MAX_TINYNAME sizeof (uint32_t)
#define DC_MAX_NAME MIN(MAX_NAME, UINT8_MAX) #define NAMELEN_ADJ (MAX_TINYNAME+1)
#define DC_MAX_NAME (UINT8_MAX+NAMELEN_ADJ)
#define CE_NAMESIZE(len) ((len)+NAMELEN_ADJ)
#define NAMESIZE_CE(size) ((size)-NAMELEN_ADJ)
/* Throw some warnings if about the limits if things may not work */ /* Throw some warnings if about the limits if things may not work */
#if MAX_NAME > UINT8_MAX #if MAX_COMPNAME > UINT8_MAX+5
#warning Need more than 8 bits in name length bitfield #warning Need more than 8 bits in name length bitfield
#endif #endif
#if DIRCACHE_LIMIT > ((1 << 24)-255) #if DIRCACHE_LIMIT > ((1 << 24)-1+5)
#warning Names may not be addressable with 24 bits #warning Names may not be addressable with 24 bits
#endif #endif
@ -173,7 +176,7 @@ struct dircache_entry
union { union {
struct { struct {
uint32_t name : 24; /* indirect storage (.tinyname == 0) */ uint32_t name : 24; /* indirect storage (.tinyname == 0) */
uint32_t length : 8; /* length of name indexed by 'name' */ uint32_t namelen : 8; /* length of name - (MAX_TINYNAME+1) */
}; };
unsigned char namebuf[MAX_TINYNAME]; /* direct storage (.tinyname == 1) */ unsigned char namebuf[MAX_TINYNAME]; /* direct storage (.tinyname == 1) */
}; };
@ -743,7 +746,7 @@ static void entry_name_copy(char *dst, const struct dircache_entry *ce)
{ {
if (LIKELY(!ce->tinyname)) if (LIKELY(!ce->tinyname))
{ {
strmemcpy(dst, get_name(ce->name), ce->length); strmemcpy(dst, get_name(ce->name), CE_NAMESIZE(ce->namelen));
return; return;
} }
@ -876,7 +879,7 @@ static void free_name(int nameidx, size_t size)
/** /**
* allocate and assign a name to the entry * allocate and assign a name to the entry
*/ */
static bool entry_assign_name(struct dircache_entry *ce, static int entry_assign_name(struct dircache_entry *ce,
const unsigned char *name, size_t size) const unsigned char *name, size_t size)
{ {
unsigned char *copyto; unsigned char *copyto;
@ -893,21 +896,21 @@ static bool entry_assign_name(struct dircache_entry *ce,
else else
{ {
if (size > DC_MAX_NAME) if (size > DC_MAX_NAME)
size = DC_MAX_NAME; return -ENAMETOOLONG;
int nameidx = alloc_name(size); int nameidx = alloc_name(size);
if (!nameidx) if (!nameidx)
return false; return -ENOSPC;
copyto = get_name(nameidx); copyto = get_name(nameidx);
ce->tinyname = 0; ce->tinyname = 0;
ce->name = nameidx; ce->name = nameidx;
ce->length = size; ce->namelen = NAMESIZE_CE(size);
} }
memcpy(copyto, name, size); memcpy(copyto, name, size);
return true; return 0;
} }
/** /**
@ -915,17 +918,20 @@ static bool entry_assign_name(struct dircache_entry *ce,
*/ */
static void entry_unassign_name(struct dircache_entry *ce) static void entry_unassign_name(struct dircache_entry *ce)
{ {
if (!ce->tinyname) if (ce->tinyname)
free_name(ce->name, ce->length); return;
free_name(ce->name, CE_NAMESIZE(ce->namelen));
ce->tinyname = 1;
} }
/** /**
* assign a new name to the entry * assign a new name to the entry
*/ */
static bool entry_reassign_name(struct dircache_entry *ce, static int entry_reassign_name(struct dircache_entry *ce,
const unsigned char *newname) const unsigned char *newname)
{ {
size_t oldlen = ce->tinyname ? 0 : ce->length; size_t oldlen = ce->tinyname ? 0 : CE_NAMESIZE(ce->namelen);
size_t newlen = strlen(newname); size_t newlen = strlen(newname);
if (oldlen == newlen || (oldlen == 0 && newlen <= MAX_TINYNAME)) if (oldlen == newlen || (oldlen == 0 && newlen <= MAX_TINYNAME))
@ -934,7 +940,7 @@ static bool entry_reassign_name(struct dircache_entry *ce,
newname, newlen); newname, newlen);
if (newlen < MAX_TINYNAME) if (newlen < MAX_TINYNAME)
*p = '\0'; *p = '\0';
return true; return 0;
} }
/* needs a new name allocation; if the new name fits in the freed block, /* needs a new name allocation; if the new name fits in the freed block,
@ -968,7 +974,7 @@ static int alloc_entry(struct dircache_entry **res)
{ {
dircache.last_size = 0; dircache.last_size = 0;
*res = NULL; *res = NULL;
return 0; return -ENOSPC;
} }
dircache.sizeused += ENTRYSIZE; dircache.sizeused += ENTRYSIZE;
@ -976,7 +982,7 @@ static int alloc_entry(struct dircache_entry **res)
ce->next = 0; ce->next = 0;
ce->up = 0; ce->up = 0;
ce->down = 0; ce->down = 0;
ce->length = 0; ce->tinyname = 1;
ce->frontier = FRONTIER_SETTLED; ce->frontier = FRONTIER_SETTLED;
ce->serialnum = next_serialnum(); ce->serialnum = next_serialnum();
@ -1030,18 +1036,19 @@ static int create_entry(const char *basename,
struct dircache_entry **res) struct dircache_entry **res)
{ {
int idx = alloc_entry(res); int idx = alloc_entry(res);
if (!idx)
{
logf("size limit reached (entry)");
return 0; /* full */
}
if (!entry_assign_name(*res, basename, strlen(basename))) if (idx > 0)
{
int rc = entry_assign_name(*res, basename, strlen(basename));
if (rc < 0)
{ {
free_orphan_entry(NULL, *res, idx); free_orphan_entry(NULL, *res, idx);
logf("size limit reached (name)"); idx = rc;
return 0; /* really full! */
} }
}
if (idx == -ENOSPC)
logf("size limit reached");
return idx; return idx;
} }
@ -1304,8 +1311,15 @@ static void sab_process_sub(struct sab *sabp)
} }
int idx = create_entry(fatentp->name, &ce); int idx = create_entry(fatentp->name, &ce);
if (!idx) if (idx <= 0)
{ {
if (idx == -ENAMETOOLONG)
{
/* not fatal; just don't include it */
establish_frontier(compp->idx, FRONTIER_ZONED);
continue;
}
sabp->quit = true; sabp->quit = true;
break; break;
} }
@ -2328,7 +2342,7 @@ void dircache_fileop_create(struct file_base_info *dirinfop,
struct dircache_entry *ce; struct dircache_entry *ce;
int idx = create_entry(basename, &ce); int idx = create_entry(basename, &ce);
if (idx == 0) if (idx <= 0)
{ {
/* failed allocation; parent cache contents are not complete */ /* failed allocation; parent cache contents are not complete */
establish_frontier(dirinfop->dcfile.idx, FRONTIER_ZONED); establish_frontier(dirinfop->dcfile.idx, FRONTIER_ZONED);
@ -2427,7 +2441,7 @@ void dircache_fileop_rename(struct file_base_info *dirinfop,
insert_file_entry(dirinfop, ce); insert_file_entry(dirinfop, ce);
/* lastly, update the entry name itself */ /* lastly, update the entry name itself */
if (entry_reassign_name(ce, basename)) if (entry_reassign_name(ce, basename) == 0)
{ {
/* it's not really the same one now so re-stamp it */ /* it's not really the same one now so re-stamp it */
dc_serial_t serialnum = next_serialnum(); dc_serial_t serialnum = next_serialnum();
@ -2506,7 +2520,7 @@ static ssize_t get_path_sub(int idx, struct get_path_sub_data *data)
if (len < 0) if (len < 0)
return -2; return -2;
cename = alloca(MAX_NAME + 1); cename = alloca(DC_MAX_NAME + 1);
entry_name_copy(cename, ce); entry_name_copy(cename, ce);
} }
else /* idx < 0 */ else /* idx < 0 */

2
firmware/common/file_internal.c
View file

@ -505,7 +505,7 @@ walk_path(struct pathwalk *walkp, struct pathwalk_component *compp,
if (!(compp->attr & ATTR_DIRECTORY)) if (!(compp->attr & ATTR_DIRECTORY))
return -ENOTDIR; return -ENOTDIR;
if (len >= MAX_NAME) if (len > MAX_COMPNAME)
return -ENAMETOOLONG; return -ENAMETOOLONG;
/* check for "." and ".." */ /* check for "." and ".." */

2
firmware/include/file_internal.h
View file

@ -56,7 +56,7 @@
root + 'Music' + 'Artist' + 'Album' + 'Disc N' + filename */ root + 'Music' + 'Artist' + 'Album' + 'Disc N' + filename */
#define STATIC_PATHCOMP_NUM 6 #define STATIC_PATHCOMP_NUM 6
#define MAX_NAME 255 #define MAX_COMPNAME 260
/* unsigned value that will also hold the off_t range we need without /* unsigned value that will also hold the off_t range we need without
overflow */ overflow */