Support and General Use > Audio Playback, Database and Playlists

Database list size (e.g. artists/albums) > global_settings.max_files_in_dir

(1/3) > >>

chris_s:
So I noticed on the latest dev version, since this commit a week ago which explicitly prevents a buffer overflow, that the database will now panic when I select an item from the album or artist list with an index (e.g. the 1100th item)  apparently larger than the "max files in directory" setting.


(From apps/tree.c – this will return NULL: )

--- Code: ---struct entry* tree_get_entry_at(struct tree_context *t, int index)
{
    if(index < 0 || index >= t->cache.max_entries)
        return NULL; /* no entry */
--- End code ---
https://github.com/Rockbox/rockbox/blob/3f110daf3032187c052a6e3c1b05d01d1a4582d0/apps/tree.c#L113
 
and then panic:

--- Code: ---struct entry *entry = tree_get_entry_at(local_tc, selected_item);
    if (!entry)
        panicf("Invalid tree entry");
--- End code ---


I guess that makes sense because of the following line (1080) in tree_mem_init(void):


--- Code: ---cache->max_entries = global_settings.max_files_in_dir;
--- End code ---

My question – Is the database supposed to be constrained in this way? If so, I haven't seen this documented anywhere. The problem disappeared by increasing the "max files in dir" setting.

I never noticed this issue before, because selecting any item in the database used to (seemingly) work fine before the aforementioned commit, although I suppose the buffer still got silently corrupted?!

Bilgus:
Thanks for the report, before it just overflowed & corrupted the bufferlib but i'll look into it 

chris_s:
Awesome! The added bounds checking certainly made sense to me, by itself and actually brought to light the fact that I needed to increase the value for "max files in dir".

Bilgus:
it looks like it isn't detrimental in the sense of buffer overflow.

However, dirbrowse() is receiving bad data and it'll take me some time
to look through the code and see what can/should be done about it.

Bilgus:
Ok, got it figured out should be fixed in dev builds after 12-20-2018

The database browser uses the function dirbrowse()
dirbrowse was grabbing file attributes it didn't need in the
particular circumstance with an invalid index and thus triggered the panic

http://gerrit.rockbox.org/r/#/c/2015/

Thanks again for the report

Bilgus

Navigation

[0] Message Index

[#] Next page