From 212cb7333a3d7d583db21e258de72f52ae1f10f1 Mon Sep 17 00:00:00 2001 From: Ingo Weinhold Date: Sun, 28 Apr 2013 14:39:58 +0200 Subject: [PATCH] packagefs: Fix deadbeef when using a file after package removal When closing a file whose package had been removed, PackageFile::VFSUninit() would crash due to calling Package::Close() on an already destroyed Package object. PackageNode does now hold a reference to the package between VFSInit() and VFSUninit(). --- .../kernel/file_systems/packagefs/PackageFile.cpp | 13 ++++++++++++- .../kernel/file_systems/packagefs/PackageNode.cpp | 3 +++ .../kernel/file_systems/packagefs/PackageNode.h | 12 ++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/add-ons/kernel/file_systems/packagefs/PackageFile.cpp b/src/add-ons/kernel/file_systems/packagefs/PackageFile.cpp index d61f36b533..3074257c90 100644 --- a/src/add-ons/kernel/file_systems/packagefs/PackageFile.cpp +++ b/src/add-ons/kernel/file_systems/packagefs/PackageFile.cpp @@ -9,6 +9,8 @@ #include #include +#include + #include #include @@ -153,6 +155,12 @@ PackageFile::~PackageFile() status_t PackageFile::VFSInit(dev_t deviceID, ino_t nodeID) { + status_t error = PackageNode::VFSInit(deviceID, nodeID); + if (error != B_OK) + return error; + MethodDeleter baseClassUninit(this, + &PackageNode::NonVirtualVFSUninit); + // open the package int fd = fPackage->Open(); if (fd < 0) @@ -164,7 +172,7 @@ PackageFile::VFSInit(dev_t deviceID, ino_t nodeID) if (fDataAccessor == NULL) RETURN_ERROR(B_NO_MEMORY); - status_t error = fDataAccessor->Init(deviceID, nodeID, fd); + error = fDataAccessor->Init(deviceID, nodeID, fd); if (error != B_OK) { delete fDataAccessor; fDataAccessor = NULL; @@ -172,6 +180,7 @@ PackageFile::VFSInit(dev_t deviceID, ino_t nodeID) } packageCloser.Detach(); + baseClassUninit.Detach(); return B_OK; } @@ -184,6 +193,8 @@ PackageFile::VFSUninit() delete fDataAccessor; fDataAccessor = NULL; } + + PackageNode::VFSUninit(); } diff --git a/src/add-ons/kernel/file_systems/packagefs/PackageNode.cpp b/src/add-ons/kernel/file_systems/packagefs/PackageNode.cpp index 585907afe4..06eb88110a 100644 --- a/src/add-ons/kernel/file_systems/packagefs/PackageNode.cpp +++ b/src/add-ons/kernel/file_systems/packagefs/PackageNode.cpp @@ -10,6 +10,7 @@ #include #include "DebugSupport.h" +#include "Package.h" PackageNode::PackageNode(Package* package, mode_t mode) @@ -48,6 +49,7 @@ PackageNode::Init(PackageDirectory* parent, const char* name) status_t PackageNode::VFSInit(dev_t deviceID, ino_t nodeID) { + fPackage->AcquireReference(); return B_OK; } @@ -55,6 +57,7 @@ PackageNode::VFSInit(dev_t deviceID, ino_t nodeID) void PackageNode::VFSUninit() { + fPackage->ReleaseReference(); } diff --git a/src/add-ons/kernel/file_systems/packagefs/PackageNode.h b/src/add-ons/kernel/file_systems/packagefs/PackageNode.h index ec6a57fdc5..8f9ed2f048 100644 --- a/src/add-ons/kernel/file_systems/packagefs/PackageNode.h +++ b/src/add-ons/kernel/file_systems/packagefs/PackageNode.h @@ -28,6 +28,11 @@ public: virtual ~PackageNode(); Package* GetPackage() const { return fPackage; } + // Since PackageNode does only hold a + // reference to the package between + // VFSInit() and VFSUninit(), the caller + // must otherwise make sure the package + // still exists. PackageDirectory* Parent() const { return fParent; } const char* Name() const { return fName; } @@ -36,6 +41,7 @@ public: virtual status_t VFSInit(dev_t deviceID, ino_t nodeID); virtual void VFSUninit(); + // base class versions must be called mode_t Mode() const { return fMode; } @@ -65,6 +71,12 @@ public: inline void* IndexCookieForAttribute(const char* name) const; +protected: + void NonVirtualVFSUninit() + { PackageNode::VFSUninit(); } + // service for derived classes, e.g. for use + // with MethodDeleter + protected: Package* fPackage; PackageDirectory* fParent;