Loading...
iokit/Kernel/IONVRAMV3Handler.cpp xnu-12377.121.6 xnu-10002.61.3
--- xnu/xnu-12377.121.6/iokit/Kernel/IONVRAMV3Handler.cpp
+++ xnu/xnu-10002.61.3/iokit/Kernel/IONVRAMV3Handler.cpp
@@ -201,7 +201,7 @@
 
 	uint8_t                      *_nvramImage;
 
-	OSSharedPtr<OSDictionary>    _varDict;
+	OSSharedPtr<OSDictionary>    &_varDict;
 
 	uint32_t                     _commonSize;
 	uint32_t                     _systemSize;
@@ -212,14 +212,10 @@
 	uint32_t                     _currentOffset;
 
 	OSSharedPtr<OSArray>         _varEntries;
-
-	IORWLock                     *_variableLock;
-	IOLock                       *_controllerLock;
 
 	IOReturn unserializeImage(const uint8_t *image, IOByteCount length);
 	IOReturn reclaim(void);
 	uint32_t findCurrentBank(void);
-	size_t   getAppendSize(void);
 
 	static bool convertObjectToProp(uint8_t *buffer, uint32_t *length, const char *propSymbol, OSObject *propObject);
 	static bool convertPropToObject(const uint8_t *propName, uint32_t propNameLength, const uint8_t *propData, uint32_t propDataLength,
@@ -232,18 +228,22 @@
 	void findExistingEntry(const uuid_t varGuid, const char *varName, struct nvram_v3_var_entry **existing, unsigned int *existingIndex);
 	IOReturn syncRaw(void);
 	IOReturn syncBlock(void);
+
 public:
 	virtual
 	~IONVRAMV3Handler() APPLE_KEXT_OVERRIDE;
-	IONVRAMV3Handler();
+	IONVRAMV3Handler(OSSharedPtr<OSDictionary> &varDict);
+
 	static bool isValidImage(const uint8_t *image, IOByteCount length);
-	static  IONVRAMV3Handler *init(IODTNVRAM *provider, const uint8_t *image, IOByteCount length);
+
+	static  IONVRAMV3Handler *init(IODTNVRAM *provider, const uint8_t *image, IOByteCount length,
+	    OSSharedPtr<OSDictionary> &varDict);
 
 	virtual bool     getNVRAMProperties(void) APPLE_KEXT_OVERRIDE;
 	virtual IOReturn unserializeVariables(void) APPLE_KEXT_OVERRIDE;
 	virtual IOReturn setVariable(const uuid_t varGuid, const char *variableName, OSObject *object) APPLE_KEXT_OVERRIDE;
 	virtual bool     setController(IONVRAMController *controller) APPLE_KEXT_OVERRIDE;
-	virtual IOReturn sync(void) APPLE_KEXT_OVERRIDE;
+	virtual bool     sync(void) APPLE_KEXT_OVERRIDE;
 	virtual IOReturn flush(const uuid_t guid, IONVRAMOperation op) APPLE_KEXT_OVERRIDE;
 	virtual void     reload(void) APPLE_KEXT_OVERRIDE;
 	virtual uint32_t getGeneration(void) const APPLE_KEXT_OVERRIDE;
@@ -251,14 +251,14 @@
 	virtual uint32_t getSystemUsed(void) const APPLE_KEXT_OVERRIDE;
 	virtual uint32_t getCommonUsed(void) const APPLE_KEXT_OVERRIDE;
 	virtual bool     getSystemPartitionActive(void) const APPLE_KEXT_OVERRIDE;
-	virtual IOReturn getVarDict(OSSharedPtr<OSDictionary> &varDictCopy) APPLE_KEXT_OVERRIDE;
 };
 
 IONVRAMV3Handler::~IONVRAMV3Handler()
 {
 }
 
-IONVRAMV3Handler::IONVRAMV3Handler()
+IONVRAMV3Handler::IONVRAMV3Handler(OSSharedPtr<OSDictionary> &varDict) :
+	_varDict(varDict)
 {
 }
 
@@ -275,21 +275,16 @@
 }
 
 IONVRAMV3Handler*
-IONVRAMV3Handler::init(IODTNVRAM *provider, const uint8_t *image, IOByteCount length)
+IONVRAMV3Handler::init(IODTNVRAM *provider, const uint8_t *image, IOByteCount length,
+    OSSharedPtr<OSDictionary> &varDict)
 {
 	OSSharedPtr<IORegistryEntry> entry;
 	OSSharedPtr<OSObject>        prop;
 	bool                         propertiesOk;
 
-	IONVRAMV3Handler *handler = new IONVRAMV3Handler();
+	IONVRAMV3Handler *handler = new IONVRAMV3Handler(varDict);
 
 	handler->_provider = provider;
-
-	handler->_variableLock = IORWLockAlloc();
-	require(handler->_variableLock != nullptr, exit);
-
-	handler->_controllerLock = IOLockAlloc();
-	require(handler->_controllerLock != nullptr, exit);
 
 	propertiesOk = handler->getNVRAMProperties();
 	require_action(propertiesOk, exit, DEBUG_ERROR("Unable to get NVRAM properties\n"));
@@ -351,7 +346,6 @@
 
 	DEBUG_INFO("flushSystem=%d, flushCommon=%d\n", flushSystem, flushCommon);
 
-	NVRAMWRITELOCK(_variableLock);
 	if (flushSystem || flushCommon) {
 		const OSSymbol                    *canonicalKey;
 		OSSharedPtr<OSDictionary>         dictCopy;
@@ -373,7 +367,7 @@
 
 			clear = ((flushSystem && (uuid_compare(varGuid, gAppleSystemVariableGuid) == 0)) ||
 			    (flushCommon && (uuid_compare(varGuid, gAppleSystemVariableGuid) != 0))) &&
-			    verifyPermission(op, varGuid, varName, getSystemPartitionActive(), true);
+			    verifyPermission(op, varGuid, varName, getSystemPartitionActive());
 
 			if (clear) {
 				DEBUG_INFO("Clearing entry for %s:%s\n", uuidString, varName);
@@ -389,7 +383,6 @@
 	DEBUG_INFO("_commonUsed %#x, _systemUsed %#x\n", _commonUsed, _systemUsed);
 
 exit:
-	NVRAMRWUNLOCK(_variableLock);
 	return ret;
 }
 
@@ -404,17 +397,15 @@
 	const struct v3_var_header   *storeVar;
 	OSData                       *entryContainer;
 
-	NVRAMLOCKASSERTHELD(_controllerLock);
-
 	controllerBank = findCurrentBank();
 
 	if (_currentBank != controllerBank) {
-		DEBUG_ERROR("_currentBank %#x != controllerBank %#x\n", _currentBank, controllerBank);
+		DEBUG_ERROR("_currentBank %#x != controllerBank %#x", _currentBank, controllerBank);
 	}
 
 	_currentBank = controllerBank;
 
-	controllerImage = (uint8_t *)IOMallocZeroData(_bankSize);
+	controllerImage = (uint8_t *)IOMallocData(_bankSize);
 
 	_nvramController->select(_currentBank);
 	_nvramController->read(0, controllerImage, _bankSize);
@@ -433,7 +424,6 @@
 	// as VAR_NEW_STATE_NONE meaning no action needed
 	// Otherwise if the data is different or it is not found on the controller image we mark it as VAR_NEW_STATE_APPEND
 	// which will have us invalidate the existing entry if there is one and append it on the next save
-	NVRAMREADLOCK(_variableLock);
 	for (unsigned int i = 0; i < _varEntries->getCount(); i++) {
 		uint32_t offset = sizeof(struct v3_store_header);
 		uint32_t latestOffset;
@@ -452,7 +442,7 @@
 				uint8_t state = prevVarHeader->state & VAR_DELETED & VAR_IN_DELETED_TRANSITION;
 
 				ret = _nvramController->write(prevOffset + offsetof(struct v3_var_header, state), &state, sizeof(state));
-				require_noerr_action(ret, unlock, DEBUG_ERROR("existing state w fail, ret=%#x\n", ret));
+				require_noerr_action(ret, exit, DEBUG_ERROR("existing state w fail, ret=%#x\n", ret));
 			}
 
 			prevOffset = latestOffset;
@@ -490,12 +480,15 @@
 			}
 		}
 	}
+
 	ret = find_current_offset_in_image(controllerImage, _bankSize, &_currentOffset);
-	require_noerr_action(ret, unlock, DEBUG_ERROR("Unidentified bytes in image\n"));
+	if (ret != kIOReturnSuccess) {
+		DEBUG_ERROR("Unidentified bytes in image, reclaiming\n");
+		ret = reclaim();
+		require_noerr_action(ret, exit, DEBUG_ERROR("Reclaim byte recovery failed, invalid controller state!!! ret=%#x\n", ret));
+	}
 	DEBUG_INFO("New _currentOffset=%#x\n", _currentOffset);
 
-unlock:
-	NVRAMRWUNLOCK(_variableLock);
 exit:
 	IOFreeData(controllerImage, _bankSize);
 	return ret;
@@ -515,9 +508,6 @@
 	OSSharedPtr<const OSSymbol> canonicalKey;
 	const char                  *variableName;
 	uint32_t                    variableSize;
-
-	// Anyone calling setEntryForRemove should've already held the lock for write.
-	NVRAMRWLOCKASSERTEXCLUSIVE(_variableLock);
 
 	require_action(v3Entry != nullptr, exit, DEBUG_INFO("remove with no entry\n"));
 
@@ -532,7 +522,7 @@
 
 		v3Entry->new_state = VAR_NEW_STATE_REMOVE;
 
-		_varDict->removeObject(canonicalKey.get());
+		_provider->_varDict->removeObject(canonicalKey.get());
 
 		if (system) {
 			if (_systemUsed < variableSize) {
@@ -549,7 +539,8 @@
 		if (_provider->_diags) {
 			_provider->_diags->logVariable(getPartitionTypeForGUID(v3Entry->header.guid),
 			    kIONVRAMOperationDelete,
-			    variableName);
+			    variableName,
+			    nullptr);
 		}
 	}
 
@@ -732,16 +723,15 @@
 		    propSymbol, propObject)) {
 			OSSharedPtr<const OSSymbol> canonicalKey = keyWithGuidAndCString(v3Entry->header.guid, (const char *)v3Entry->header.name_data_buf);
 
-			DEBUG_INFO("adding %s, variableLength=%zu, dataLength=%u, system=%d\n",
-			    canonicalKey->getCStringNoCopy(), variable_length(header), v3Entry->header.dataSize, system);
+			DEBUG_INFO("adding %s, dataLength=%u, system=%d\n",
+			    canonicalKey->getCStringNoCopy(), v3Entry->header.dataSize, system);
 
 			_varDict->setObject(canonicalKey.get(), propObject.get());
 
 			if (_provider->_diags) {
 				_provider->_diags->logVariable(getPartitionTypeForGUID(v3Entry->header.guid),
 				    kIONVRAMOperationInit, propSymbol.get()->getCStringNoCopy(),
-				    (void *)(uintptr_t)v3Entry->header.dataSize,
-				    (void *)(uintptr_t)offset);
+				    (void *)(uintptr_t)(header->name_data_buf + header->nameSize));
 			}
 		}
 		IOFreeData(v3Entry, nvram_v3_var_container_size(header));
@@ -752,10 +742,6 @@
 	_currentOffset = (uint32_t)offset;
 
 	DEBUG_ALWAYS("_commonSize %#x, _systemSize %#x, _currentOffset %#x\n", _commonSize, _systemSize, _currentOffset);
-
-	ret = handleEphDM();
-	verify_noerr_action(ret, panic("handleEphDM failed with ret=%08x", ret));
-
 	DEBUG_INFO("_commonUsed %#x, _systemUsed %#x\n", _commonUsed, _systemUsed);
 
 	_newData = true;
@@ -791,9 +777,6 @@
 	size_t                      newEntrySize;
 	uuid_string_t               uuidString;
 
-	// Anyone calling setVariableInternal should've already held the lock for write.
-	NVRAMRWLOCKASSERTEXCLUSIVE(_variableLock);
-
 	system = (uuid_compare(varGuid, gAppleSystemVariableGuid) == 0);
 	canonicalKey = keyWithGuidAndCString(varGuid, variableName);
 
@@ -889,14 +872,6 @@
 IONVRAMV3Handler::setVariable(const uuid_t varGuid, const char *variableName, OSObject *object)
 {
 	uuid_t destGuid;
-	IOReturn ret = kIOReturnError;
-
-	if (strcmp(variableName, "reclaim-int") == 0) {
-		NVRAMLOCK(_controllerLock);
-		ret = reclaim();
-		NVRAMUNLOCK(_controllerLock);
-		return ret;
-	}
 
 	if (getSystemPartitionActive()) {
 		// System region case, if they're using the GUID directly or it's on the system allow list
@@ -915,11 +890,7 @@
 		}
 	}
 
-	NVRAMWRITELOCK(_variableLock);
-	ret = setVariableInternal(destGuid, variableName, object);
-	NVRAMRWUNLOCK(_variableLock);
-
-	return ret;
+	return setVariableInternal(destGuid, variableName, object);
 }
 
 uint32_t
@@ -929,8 +900,6 @@
 	uint32_t               maxGen = 0;
 	uint32_t               currentBank = 0;
 
-	NVRAMLOCKASSERTHELD(_controllerLock);
-
 	for (unsigned int i = 0; i < _bankCount; i++) {
 		_nvramController->select(i);
 		_nvramController->read(0, (uint8_t *)&storeHeader, sizeof(storeHeader));
@@ -941,7 +910,7 @@
 		}
 	}
 
-	DEBUG_ALWAYS("currentBank=%#x, gen=%#x\n", currentBank, maxGen);
+	DEBUG_ALWAYS("currentBank=%#x, gen=%#x", currentBank, maxGen);
 
 	return currentBank;
 }
@@ -950,8 +919,6 @@
 IONVRAMV3Handler::setController(IONVRAMController *controller)
 {
 	IOReturn ret = kIOReturnSuccess;
-
-	NVRAMLOCK(_controllerLock);
 
 	if (_nvramController == NULL) {
 		_nvramController = controller;
@@ -964,39 +931,34 @@
 	if (_resetData) {
 		_resetData = false;
 		DEBUG_ERROR("_resetData set, issuing reclaim recovery\n");
-		goto reclaim;
-	}
-
-	if (reloadInternal() == kIOReturnSuccess) {
+		ret = reclaim();
+		require_noerr_action(ret, exit, DEBUG_ERROR("Reclaim recovery failed, invalid controller state!!! ret=%#x\n", ret));
 		goto exit;
 	}
 
-reclaim:
-	ret = reclaim();
-	require_noerr_action(ret, exit, DEBUG_ERROR("Reclaim recovery failed, invalid controller state!!! ret=%#x\n", ret));
+	ret = reloadInternal();
+	if (ret != kIOReturnSuccess) {
+		DEBUG_ERROR("Invalid image found, issuing reclaim recovery\n");
+		ret = reclaim();
+		require_noerr_action(ret, exit, DEBUG_ERROR("Reclaim recovery failed, invalid controller state!!! ret=%#x\n", ret));
+	}
+
 exit:
-	NVRAMUNLOCK(_controllerLock);
 	return ret == kIOReturnSuccess;
 }
 
 IOReturn
 IONVRAMV3Handler::reclaim(void)
 {
-	IOReturn             ret;
-	struct               v3_store_header newStoreHeader;
-	struct               v3_var_header *varHeader;
-	struct               nvram_v3_var_entry *varEntry;
-	OSData               *entryContainer;
-	size_t               new_bank_offset = sizeof(struct v3_store_header);
-	uint32_t             next_bank = (_currentBank + 1) % _bankCount;
-	uint8_t              *bankData;
-	OSSharedPtr<OSArray> remainingEntries;
+	IOReturn ret;
+	struct   v3_store_header newStoreHeader;
+	struct   v3_var_header *varHeader;
+	struct   nvram_v3_var_entry *varEntry;
+	OSData   *entryContainer;
+	size_t   new_bank_offset = sizeof(struct v3_store_header);
+	uint32_t next_bank = (_currentBank + 1) % _bankCount;
 
 	DEBUG_INFO("called\n");
-	NVRAMLOCKASSERTHELD(_controllerLock);
-
-	bankData = (uint8_t *)IOMallocZeroData(_bankSize);
-	require_action(bankData != nullptr, exit, ret = kIOReturnNoMemory);
 
 	ret = _nvramController->select(next_bank);
 	verify_noerr_action(ret, DEBUG_INFO("select of bank %#08x failed\n", next_bank));
@@ -1005,10 +967,6 @@
 	verify_noerr_action(ret, DEBUG_INFO("eraseBank failed, ret=%#08x\n", ret));
 
 	_currentBank = next_bank;
-
-	NVRAMREADLOCK(_variableLock);
-
-	remainingEntries = OSArray::withCapacity(_varEntries->getCapacity());
 
 	for (unsigned int i = 0; i < _varEntries->getCount(); i++) {
 		entryContainer = OSDynamicCast(OSData, _varEntries->getObject(i));
@@ -1018,19 +976,16 @@
 		DEBUG_INFO("entry %u %s, new_state=%#x, e_offset=%#lx, state=%#x\n",
 		    i, varEntry->header.name_data_buf, varEntry->new_state, varEntry->existing_offset, varHeader->state);
 
-		if ((varEntry->new_state == VAR_NEW_STATE_NONE) ||
-		    (varEntry->new_state == VAR_NEW_STATE_APPEND)) {
-			varHeader->state = VAR_ADDED;
-
-			memcpy(bankData + new_bank_offset, (uint8_t *)varHeader, variable_length(varHeader));
-
-			varEntry->new_state = VAR_NEW_STATE_NONE;
+		if (varEntry->new_state == VAR_NEW_STATE_NONE) {
+			ret = _nvramController->write(new_bank_offset, (uint8_t *)varHeader, variable_length(varHeader));
+			require_noerr_action(ret, exit, DEBUG_ERROR("var write failed, ret=%08x\n", ret));
+
 			varEntry->existing_offset = new_bank_offset;
 			new_bank_offset += variable_length(varHeader);
-
-			remainingEntries->setObject(entryContainer);
 		} else {
-			// entryContainer not added to remainingEntries, entry dropped
+			// Set existing offset to 0 so that they will either be appended
+			// or any remaining removals will be dropped
+			varEntry->existing_offset = 0;
 		}
 	}
 
@@ -1040,162 +995,130 @@
 
 	newStoreHeader.generation = _generation;
 
-	memcpy(bankData, (uint8_t *)&newStoreHeader, sizeof(newStoreHeader));
-
-	ret = _nvramController->write(0, bankData, new_bank_offset);
-	require_noerr_action(ret, unlock, DEBUG_ERROR("reclaim bank write failed, ret=%08x\n", ret));
+	ret = _nvramController->write(0, (uint8_t *)&newStoreHeader, sizeof(newStoreHeader));
+	require_noerr_action(ret, exit, DEBUG_ERROR("store header write failed, ret=%08x\n", ret));
 
 	_currentOffset = (uint32_t)new_bank_offset;
 
-	DEBUG_INFO("Reclaim complete, _currentBank=%u _generation=%u, _currentOffset=%#x\n", _currentBank, _generation, _currentOffset);
-
-	_newData = false;
-	_varEntries.reset(remainingEntries.get(), OSRetain);
-
-unlock:
-	NVRAMRWUNLOCK(_variableLock);
+	DEBUG_INFO("Reclaim complete, _generation=%u, _currentOffset=%#x\n", _generation, _currentOffset);
+
 exit:
-	IOFreeData(bankData, _bankSize);
-
 	return ret;
 }
 
-size_t
-IONVRAMV3Handler::getAppendSize(void)
-{
-	struct nvram_v3_var_entry *varEntry;
-	struct v3_var_header      *varHeader;
-	OSData                    *entryContainer;
-	size_t                    appendSize = 0;
-
-	NVRAMRWLOCKASSERTHELD(_variableLock);
+IOReturn
+IONVRAMV3Handler::syncRaw(void)
+{
+	IOReturn             ret = kIOReturnSuccess;
+	size_t               varEndOffset;
+	size_t               varStartOffset;
+	struct               nvram_v3_var_entry *varEntry;
+	struct               v3_var_header *varHeader;
+	OSData               *entryContainer;
+	OSSharedPtr<OSArray> remainingEntries;
+
+	require_action(_nvramController != nullptr, exit, DEBUG_INFO("No _nvramController\n"));
+	require_action(_newData == true, exit, DEBUG_INFO("No _newData to sync\n"));
+	require_action(_bankSize != 0, exit, DEBUG_INFO("No nvram size info\n"));
+
+	DEBUG_INFO("_varEntries->getCount()=%#x\n", _varEntries->getCount());
+
+	remainingEntries = OSArray::withCapacity(_varEntries->getCapacity());
 
 	for (unsigned int i = 0; i < _varEntries->getCount(); i++) {
+		size_t space_needed = 0;
+		uint8_t state;
+
 		entryContainer = OSDynamicCast(OSData, _varEntries->getObject(i));
 		varEntry = (struct nvram_v3_var_entry *)entryContainer->getBytesNoCopy();
 		varHeader = &varEntry->header;
 
+		DEBUG_INFO("%s new_state=%d, e_off=%#lx, c_off=%#x, uuid=%x%x, nameSize=%#x, dataSize=%#x\n",
+		    varEntry->header.name_data_buf,
+		    varEntry->new_state, varEntry->existing_offset, _currentOffset,
+		    varHeader->guid[0], varHeader->guid[1],
+		    varHeader->nameSize, varHeader->dataSize);
+
 		if (varEntry->new_state == VAR_NEW_STATE_APPEND) {
-			appendSize += variable_length(varHeader);
-		}
-	}
-
-	return appendSize;
-}
-
-IOReturn
-IONVRAMV3Handler::syncRaw(void)
-{
-	IOReturn                  ret = kIOReturnSuccess;
-	struct nvram_v3_var_entry *varEntry;
-	struct v3_var_header      *varHeader;
-	OSData                    *entryContainer;
-	OSSharedPtr<OSArray>      remainingEntries;
-	uint8_t                   *appendBuffer = nullptr;
-	size_t                    appendBufferOffset = 0;
-	size_t                    *invalidateOffsets = nullptr;
-	size_t                    invalidateOffsetsCount = 0;
-	size_t                    invalidateOffsetIndex = 0;
-
-	require_action(_nvramController != nullptr, exit, DEBUG_INFO("No _nvramController\n"));
-	require_action(_newData == true, exit, DEBUG_INFO("No _newData to sync\n"));
-	require_action(_bankSize != 0, exit, DEBUG_INFO("No nvram size info\n"));
-
-	NVRAMREADLOCK(_variableLock);
-	DEBUG_INFO("_varEntries->getCount()=%#x\n", _varEntries->getCount());
-
-	if (getAppendSize() + _currentOffset < _bankSize) {
-		// No reclaim, build append and invalidate list
-		remainingEntries = OSArray::withCapacity(_varEntries->getCapacity());
-
-		appendBuffer = (uint8_t *)IOMallocZeroData(_bankSize);
-		require_action(appendBuffer, unlock, ret = kIOReturnNoMemory);
-
-		invalidateOffsetsCount = _varEntries->getCount();
-		invalidateOffsets = (size_t *)IOMallocZeroData(invalidateOffsetsCount * sizeof(size_t));
-		require_action(invalidateOffsets, unlock, ret = kIOReturnNoMemory);
-
-		for (unsigned int i = 0; i < _varEntries->getCount(); i++) {
-			entryContainer = OSDynamicCast(OSData, _varEntries->getObject(i));
-			varEntry = (struct nvram_v3_var_entry *)entryContainer->getBytesNoCopy();
-			varHeader = &varEntry->header;
-
-			DEBUG_INFO("entry %s, new_state=%#02x state=%#02x, existing_offset=%#zx\n",
-			    varEntry->header.name_data_buf, varEntry->new_state, varEntry->header.state, varEntry->existing_offset);
-
-			if (varEntry->new_state == VAR_NEW_STATE_APPEND) {
-				size_t varSize = variable_length(varHeader);
-				size_t prevOffset = varEntry->existing_offset;
-
-				varHeader->state = VAR_ADDED;
-				varEntry->existing_offset = _currentOffset + appendBufferOffset;
-				varEntry->new_state = VAR_NEW_STATE_NONE;
-
-				DEBUG_INFO("Appending %s in append buffer offset %#zx, actual offset %#zx, prevOffset %#zx, varsize=%#zx\n",
-				    varEntry->header.name_data_buf, appendBufferOffset, varEntry->existing_offset, prevOffset, varSize);
-
-				// Write to append buffer
-				memcpy(appendBuffer + appendBufferOffset, (uint8_t *)varHeader, varSize);
-				appendBufferOffset += varSize;
-
-				if (prevOffset) {
-					invalidateOffsets[invalidateOffsetIndex++] = prevOffset;
+			space_needed = variable_length(varHeader);
+
+			// reclaim if needed
+			if ((_currentOffset + space_needed) > _bankSize) {
+				ret = reclaim();
+				require_noerr_action(ret, exit, DEBUG_ERROR("reclaim fail, ret=%#x\n", ret));
+
+				// Check after reclaim...
+				if ((_currentOffset + space_needed) > _bankSize) {
+					DEBUG_ERROR("nvram full!\n");
+					goto exit;
 				}
 
-				remainingEntries->setObject(entryContainer);
-			} else if (varEntry->new_state == VAR_NEW_STATE_REMOVE) {
-				if (varEntry->existing_offset) {
-					DEBUG_INFO("marking entry at offset %#lx deleted\n", varEntry->existing_offset);
-					invalidateOffsets[invalidateOffsetIndex++] = varEntry->existing_offset;
-				} else {
-					DEBUG_INFO("No existing_offset , removing\n");
-				}
-
-				// not re-added to remainingEntries
+				DEBUG_INFO("%s AFTER reclaim new_state=%d, e_off=%#lx, c_off=%#x, uuid=%x%x, nameSize=%#x, dataSize=%#x\n",
+				    varEntry->header.name_data_buf,
+				    varEntry->new_state, varEntry->existing_offset, _currentOffset,
+				    varHeader->guid[0], varHeader->guid[1],
+				    varHeader->nameSize, varHeader->dataSize);
+			}
+
+			if (varEntry->existing_offset) {
+				// Mark existing entry as VAR_IN_DELETED_TRANSITION
+				state = varHeader->state & VAR_IN_DELETED_TRANSITION;
+				DEBUG_INFO("invalidating with state=%#x\n", state);
+
+				ret = _nvramController->write(varEntry->existing_offset + offsetof(struct v3_var_header, state), &state, sizeof(state));
+				require_noerr_action(ret, exit, DEBUG_ERROR("new state w fail, ret=%#x\n", ret));
+			}
+
+			varStartOffset = _currentOffset;
+			varEndOffset = _currentOffset;
+
+			// Append new entry as VAR_ADDED
+			varHeader->state = VAR_ADDED;
+
+			ret = _nvramController->write(varStartOffset, (uint8_t *)varHeader, variable_length(varHeader));
+			require_noerr_action(ret, exit, DEBUG_ERROR("variable write fail, ret=%#x\n", ret); );
+
+			varEndOffset += variable_length(varHeader);
+
+			if (varEntry->existing_offset) {
+				// Mark existing entry as VAR_DELETED
+				state = varHeader->state & VAR_DELETED & VAR_IN_DELETED_TRANSITION;
+
+				ret = _nvramController->write(varEntry->existing_offset + offsetof(struct v3_var_header, state), &state, sizeof(state));
+				require_noerr_action(ret, exit, DEBUG_ERROR("existing state w fail, ret=%#x\n", ret));
+			}
+
+			varEntry->existing_offset = varStartOffset;
+			varEntry->new_state = VAR_NEW_STATE_NONE;
+
+			_currentOffset = (uint32_t)varEndOffset;
+
+			remainingEntries->setObject(entryContainer);
+		} else if (varEntry->new_state == VAR_NEW_STATE_REMOVE) {
+			if (varEntry->existing_offset) {
+				DEBUG_INFO("marking entry at offset %#lx deleted\n", varEntry->existing_offset);
+
+				// Mark existing entry as VAR_IN_DELETED_TRANSITION
+				state = varHeader->state & VAR_DELETED & VAR_IN_DELETED_TRANSITION;
+
+				ret = _nvramController->write(varEntry->existing_offset + offsetof(struct v3_var_header, state), &state, sizeof(state));
+				require_noerr_action(ret, exit, DEBUG_ERROR("existing state w fail, ret=%#x\n", ret));
 			} else {
-				DEBUG_INFO("skipping\n");
-				remainingEntries->setObject(entryContainer);
-			}
-		}
-
-		if (appendBufferOffset > 0) {
-			// Write appendBuffer
-			DEBUG_INFO("Appending append buffer size=%#zx at offset=%#x\n", appendBufferOffset, _currentOffset);
-			ret = _nvramController->write(_currentOffset, appendBuffer, appendBufferOffset);
-			require_noerr_action(ret, unlock, DEBUG_ERROR("could not re-append, ret=%#x\n", ret));
-
-			_currentOffset += appendBufferOffset;
+				DEBUG_INFO("No existing, removing\n");
+			}
+
+			// not re-added to remainingEntries
 		} else {
-			DEBUG_INFO("No entries to append\n");
-		}
-
-		if (invalidateOffsetIndex > 0) {
-			// Invalidate Entries
-			for (unsigned int i = 0; i < invalidateOffsetIndex; i++) {
-				uint8_t state = VAR_ADDED & VAR_DELETED & VAR_IN_DELETED_TRANSITION;
-
-				ret = _nvramController->write(invalidateOffsets[i] + offsetof(struct v3_var_header, state), &state, sizeof(state));
-				require_noerr_action(ret, unlock, DEBUG_ERROR("unable to invalidate at offset %#zx, ret=%#x\n", invalidateOffsets[i], ret));
-				DEBUG_INFO("Invalidated entry at offset=%#zx\n", invalidateOffsets[i]);
-			}
-		} else {
-			DEBUG_INFO("No entries to invalidate\n");
-		}
-
-		_newData = false;
-		_varEntries.reset(remainingEntries.get(), OSRetain);
-unlock:
-		NVRAMRWUNLOCK(_variableLock);
-	} else {
-		// Will need to reclaim, rebuild store and write everything at once
-		NVRAMRWUNLOCK(_variableLock);
-		ret = reclaim();
-	}
+			DEBUG_INFO("skipping\n");
+			remainingEntries->setObject(entryContainer);
+		}
+	}
+
+	_varEntries.reset(remainingEntries.get(), OSRetain);
+
+	_newData = false;
 
 exit:
-	IOFreeData(appendBuffer, _bankSize);
-	IOFreeData(invalidateOffsets, invalidateOffsetsCount * sizeof(size_t));
-
 	return ret;
 }
 
@@ -1218,9 +1141,8 @@
 	require_action(_newData == true, exit, DEBUG_INFO("No _newData to sync\n"));
 	require_action(_bankSize != 0, exit, DEBUG_INFO("No nvram size info\n"));
 
-	block = (uint8_t *)IOMallocZeroData(_bankSize);
-
-	NVRAMREADLOCK(_variableLock);
+	block = (uint8_t *)IOMallocData(_bankSize);
+
 	remainingEntries = OSArray::withCapacity(_varEntries->getCapacity());
 
 	ret = _nvramController->select(next_bank);
@@ -1244,12 +1166,12 @@
 		varEntry = (struct nvram_v3_var_entry *)entryContainer->getBytesNoCopy();
 		varHeader = &varEntry->header;
 
+		varHeader->state = VAR_ADDED;
+
 		DEBUG_INFO("entry %u %s, new_state=%#x, e_offset=%#lx, state=%#x\n",
 		    i, varEntry->header.name_data_buf, varEntry->new_state, varEntry->existing_offset, varHeader->state);
 
 		if (varEntry->new_state != VAR_NEW_STATE_REMOVE) {
-			varHeader->state = VAR_ADDED;
-
 			memcpy(block + new_bank_offset, (uint8_t *)varHeader, variable_length(varHeader));
 
 			varEntry->existing_offset = new_bank_offset;
@@ -1262,17 +1184,12 @@
 		}
 	}
 
-	// 0xFF out the remaining space, this will allow banks to switch between append mode and
-	// block mode if ever needed
-	memset(block + new_bank_offset, 0xFF, _bankSize - (uint32_t)new_bank_offset);
-
 	ret = _nvramController->write(0, block, _bankSize);
 	verify_noerr_action(ret, DEBUG_ERROR("w fail, ret=%#x\n", ret));
 
 	_nvramController->sync();
 
 	_varEntries.reset(remainingEntries.get(), OSRetain);
-	NVRAMRWUNLOCK(_variableLock);
 
 	_newData = false;
 
@@ -1284,20 +1201,15 @@
 	return ret;
 }
 
-IOReturn
+bool
 IONVRAMV3Handler::sync(void)
 {
 	IOReturn ret;
-
-	NVRAMLOCK(_controllerLock);
 
 	if (_reload) {
 		ret = reloadInternal();
-		if (ret != kIOReturnSuccess) {
-			DEBUG_ERROR("Reload failed, ret=%#x, reclaiming\n", ret);
-			ret = reclaim();
-			require_noerr_action(ret, exit, DEBUG_ERROR("Reclaim recovery failed, ret=%#x\n", ret));
-		}
+		require_noerr_action(ret, exit, DEBUG_ERROR("Reload failed, ret=%#x", ret));
+
 		_reload = false;
 	}
 
@@ -1306,15 +1218,18 @@
 
 		if (ret != kIOReturnSuccess) {
 			ret = reclaim();
-			require_noerr_action(ret, exit, DEBUG_ERROR("Reclaim recovery failed, ret=%#x\n", ret));
+			require_noerr_action(ret, exit, DEBUG_ERROR("Reclaim recovery failed, ret=%#x", ret));
+
+			// Attempt to save again (will rewrite the variables still in APPEND) on the new bank
+			ret = syncRaw();
+			require_noerr_action(ret, exit, DEBUG_ERROR("syncRaw retry failed, ret=%#x", ret));
 		}
 	} else {
 		ret = syncBlock();
 	}
 
 exit:
-	NVRAMUNLOCK(_controllerLock);
-	return ret;
+	return ret == kIOReturnSuccess;
 }
 
 uint32_t
@@ -1508,22 +1423,3 @@
 
 	return true;
 }
-
-IOReturn
-IONVRAMV3Handler::getVarDict(OSSharedPtr<OSDictionary> &varDictCopy)
-{
-	IOReturn ret = kIOReturnNotFound;
-
-	NVRAMREADLOCK(_variableLock);
-	if (_varDict) {
-		varDictCopy = OSDictionary::withDictionary(_varDict.get());
-		if (varDictCopy) {
-			if (OSDictionary::withCapacity(varDictCopy->getCount()) != nullptr) {
-				ret = kIOReturnSuccess;
-			}
-		}
-	}
-	NVRAMRWUNLOCK(_variableLock);
-
-	return ret;
-}