blender
/
blender
136
406
Fork 589
Code Issues 6.2k Pull Requests 753 Projects 19 Wiki Activity

Fix use-after-free of asset catalog data in node add menu 

(Probably requires ASan for a reliable crash.)

Steps to reproduce were:
* Enter Geometry Nodes Workspace
* Press "New" button in the geometry nodes editor header
* Right-click the data-block selector -> "Mark as Asset"
* Change 3D View to Asset Browser
* Create a catalog
* Drag new Geometry Nodes asset into the catalog
* Save the file
* Press Shift+A in the geometry nodes editor

There was a general issue here with keeping catalog pointers around
during the add menu building. The way it does things, catalogs may be
reloaded in between.
Since the Current File asset library isn't loaded in a separate thread,
the use-after-free would always happen in between. For other libraries
it could still happen, but apparently didn't by chance.
This commit is contained in:
Julian Eisel 2022-11-18 17:20:07 +01:00
parent b211266226
commit c6e4953719
4 changed files with 22 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
source/blender/asset_system/AS_asset_catalog.hh
View File

@ -341,8 +341,14 @@ class AssetCatalogDefinitionFile {
bool ensure_directory_exists(const CatalogFilePath directory_path) const;
};
/** Asset Catalog definition, containing a symbolic ID and a path that points to a node in the
* catalog hierarchy. */
/**
* Asset Catalog definition, containing a symbolic ID and a path that points to a node in the
* catalog hierarchy.
*
* \warning The asset system may reload catalogs, invalidating pointers. Thus it's not recommended
* to store pointers to asset catalogs. Store the #CatalogID instead and do a lookup when
* needed.
*/
class AssetCatalog {
public:
CatalogID catalog_id;

4
source/blender/asset_system/AS_asset_library.hh
View File

@ -113,6 +113,10 @@ Vector<AssetLibraryReference> all_valid_asset_library_refs();
} // namespace blender::asset_system
/**
* \warning Catalogs are reloaded, invalidating catalog pointers. Do not store catalog pointers,
* store CatalogIDs instead and lookup the catalog where needed.
*/
blender::asset_system::AssetLibrary *AS_asset_library_load(
const Main *bmain, const AssetLibraryReference &library_reference);

3
source/blender/editors/asset/ED_asset_list.h
View File

@ -20,6 +20,9 @@ struct wmNotifier;
/**
* Invoke asset list reading, potentially in a parallel job. Won't wait until the job is done,
* and may return earlier.
*
* \warning: Asset list reading involves an #AS_asset_library_load() call which may reload asset
* library data like catalogs (invalidating pointers). Refer to its warning for details.
*/
void ED_assetlist_storage_fetch(const struct AssetLibraryReference *library_reference,
const struct bContext *C);

10
source/blender/editors/space_node/add_menu_assets.cc
View File

@ -51,7 +51,9 @@ struct LibraryAsset {
struct LibraryCatalog {
asset_system::AssetLibrary *library;
const asset_system::AssetCatalog *catalog;
/* Catalog pointers are not save to store. Use the catalog ID instead and lookup the catalog when
* needed. */
const asset_system::CatalogID catalog_id;
};
struct AssetItemTree {
@ -91,7 +93,7 @@ static AssetItemTree build_catalog_tree(const bContext &C, const bNodeTree *node
const asset_system::CatalogID &id = item.get_catalog_id();
asset_system::AssetCatalog *catalog = library->catalog_service->find_catalog(id);
catalogs_from_all_libraries.insert_item(*catalog);
id_to_catalog_map.add(item.get_catalog_id(), LibraryCatalog{library, catalog});
id_to_catalog_map.add(item.get_catalog_id(), LibraryCatalog{library, id});
});
}
}
@ -121,7 +123,9 @@ static AssetItemTree build_catalog_tree(const bContext &C, const bNodeTree *node
if (library_catalog == nullptr) {
return true;
}
assets_per_path.add(library_catalog->catalog->path, LibraryAsset{library_ref, asset});
const asset_system::AssetCatalog *catalog =
library_catalog->library->catalog_service->find_catalog(library_catalog->catalog_id);
assets_per_path.add(catalog->path, LibraryAsset{library_ref, asset});
return true;
});
}