From 9880d49bca65ad332b1ee77b1098bd201862b764 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= Date: Tue, 23 Apr 2024 08:12:45 +0200 Subject: [PATCH] Fix calculating a difference between two same-version RPM transacations Resolves: RHEL-17494 --- ...n-Calculate-RPM-difference-between-t.patch | 180 ++++++++++++++++++ ...n-Fix-invalid-memory-access-when-dro.patch | 94 +++++++++ libdnf.spec | 8 +- 3 files changed, 281 insertions(+), 1 deletion(-) create mode 100644 0012-MergedTransaction-Calculate-RPM-difference-between-t.patch create mode 100644 0013-MergedTransaction-Fix-invalid-memory-access-when-dro.patch diff --git a/0012-MergedTransaction-Calculate-RPM-difference-between-t.patch b/0012-MergedTransaction-Calculate-RPM-difference-between-t.patch new file mode 100644 index 0000000..0f9d492 --- /dev/null +++ b/0012-MergedTransaction-Calculate-RPM-difference-between-t.patch @@ -0,0 +1,180 @@ +From 3ab9afbfa28f83f0e2b737aaaa9e8d36993f540a Mon Sep 17 00:00:00 2001 +From: Jan Kolarik +Date: Mon, 26 Feb 2024 09:58:33 +0000 +Subject: [PATCH] MergedTransaction: Calculate RPM difference between two same + versions as no-op +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Upstream commit: 54823d82a1369c25ba1a68c18ea2a67c41f4fbe7 + +If a package of a particular version is installed and would still be installed after a list of transactions, it's more user friendly to treat the whole situation as "do nothing". + +Resolves: https://issues.redhat.com/browse/RHEL-17494 +Signed-off-by: Petr Písař +--- + libdnf/transaction/MergedTransaction.cpp | 38 ++++++++++++------- + libdnf/transaction/MergedTransaction.hpp | 6 +-- + .../transaction/MergedTransactionTest.cpp | 7 +--- + 3 files changed, 28 insertions(+), 23 deletions(-) + +diff --git a/libdnf/transaction/MergedTransaction.cpp b/libdnf/transaction/MergedTransaction.cpp +index a8d878cb..8f26882f 100644 +--- a/libdnf/transaction/MergedTransaction.cpp ++++ b/libdnf/transaction/MergedTransaction.cpp +@@ -192,7 +192,7 @@ static bool transaction_item_sort_function(const std::shared_ptr (new action) = (merged action) + * +- * Erase/Obsolete -> Install/Obsoleting = Reinstall/Downgrade/Upgrade ++ * Erase/Obsolete -> Install/Obsoleting = Downgrade/Upgrade + * + * Reinstall/Reason change -> (new action) = (new action) + * +@@ -210,6 +210,9 @@ static bool transaction_item_sort_function(const std::shared_ptr + MergedTransaction::getItems() +@@ -261,13 +264,16 @@ getItemIdentifier(ItemPtr item) + + /** + * Resolve the difference between RPMs in the first and second transaction item +- * and create a ItemPair of Upgrade, Downgrade or reinstall. ++ * and create a ItemPair of Upgrade, Downgrade or drop the item from the merged ++ * transaction set in case of both packages are of the same version. + * Method is called when original package is being removed and than installed again. ++ * \param itemPairMap merged transaction set + * \param previousItemPair original item pair + * \param mTransItem new transaction item + */ + void +-MergedTransaction::resolveRPMDifference(ItemPair &previousItemPair, ++MergedTransaction::resolveRPMDifference(ItemPairMap &itemPairMap, ++ ItemPair &previousItemPair, + TransactionItemBasePtr mTransItem) + { + auto firstItem = previousItemPair.first->getItem(); +@@ -277,11 +283,10 @@ MergedTransaction::resolveRPMDifference(ItemPair &previousItemPair, + auto secondRPM = std::dynamic_pointer_cast< RPMItem >(secondItem); + + if (firstRPM->getVersion() == secondRPM->getVersion() && +- firstRPM->getEpoch() == secondRPM->getEpoch()) { +- // reinstall +- mTransItem->setAction(TransactionItemAction::REINSTALL); +- previousItemPair.first = mTransItem; +- previousItemPair.second = nullptr; ++ firstRPM->getEpoch() == secondRPM->getEpoch() && ++ firstRPM->getRelease() == secondRPM->getRelease()) { ++ // Drop the item from merged transaction ++ itemPairMap.erase(getItemIdentifier(firstItem)); + return; + } else if ((*firstRPM) < (*secondRPM)) { + // Upgrade to secondRPM +@@ -296,7 +301,9 @@ MergedTransaction::resolveRPMDifference(ItemPair &previousItemPair, + } + + void +-MergedTransaction::resolveErase(ItemPair &previousItemPair, TransactionItemBasePtr mTransItem) ++MergedTransaction::resolveErase(ItemPairMap &itemPairMap, ++ ItemPair &previousItemPair, ++ TransactionItemBasePtr mTransItem) + { + /* + * The original item has been removed - it has to be installed now unless the rpmdb +@@ -306,7 +313,7 @@ MergedTransaction::resolveErase(ItemPair &previousItemPair, TransactionItemBaseP + if (mTransItem->getAction() == TransactionItemAction::INSTALL) { + if (mTransItem->getItem()->getItemType() == ItemType::RPM) { + // resolve the difference between RPM packages +- resolveRPMDifference(previousItemPair, mTransItem); ++ resolveRPMDifference(itemPairMap, previousItemPair, mTransItem); + } else { + // difference between comps can't be resolved + mTransItem->setAction(TransactionItemAction::REINSTALL); +@@ -323,11 +330,14 @@ MergedTransaction::resolveErase(ItemPair &previousItemPair, TransactionItemBaseP + * transaction - new package is used to complete the pair. Items are stored in pairs (Upgrade, + * Upgrade) or (Downgraded, Downgrade). With complete transaction pair we need to get the new + * Upgrade/Downgrade item and compare its version with the original item from the pair. ++ * \param itemPairMap merged transaction set + * \param previousItemPair original item pair + * \param mTransItem new transaction item + */ + void +-MergedTransaction::resolveAltered(ItemPair &previousItemPair, TransactionItemBasePtr mTransItem) ++MergedTransaction::resolveAltered(ItemPairMap &itemPairMap, ++ ItemPair &previousItemPair, ++ TransactionItemBasePtr mTransItem) + { + auto newState = mTransItem->getAction(); + auto firstState = previousItemPair.first->getAction(); +@@ -369,7 +379,7 @@ MergedTransaction::resolveAltered(ItemPair &previousItemPair, TransactionItemBas + } else { + if (mTransItem->getItem()->getItemType() == ItemType::RPM) { + // resolve the difference between RPM packages +- resolveRPMDifference(previousItemPair, mTransItem); ++ resolveRPMDifference(itemPairMap, previousItemPair, mTransItem); + } else { + // difference between comps can't be resolved + previousItemPair.second->setAction(TransactionItemAction::REINSTALL); +@@ -405,7 +415,7 @@ MergedTransaction::mergeItem(ItemPairMap &itemPairMap, TransactionItemBasePtr mT + switch (firstState) { + case TransactionItemAction::REMOVE: + case TransactionItemAction::OBSOLETED: +- resolveErase(previousItemPair, mTransItem); ++ resolveErase(itemPairMap, previousItemPair, mTransItem); + break; + case TransactionItemAction::INSTALL: + // the original package has been installed -> it may be either Removed, or altered +@@ -432,7 +442,7 @@ MergedTransaction::mergeItem(ItemPairMap &itemPairMap, TransactionItemBasePtr mT + case TransactionItemAction::UPGRADE: + case TransactionItemAction::UPGRADED: + case TransactionItemAction::OBSOLETE: +- resolveAltered(previousItemPair, mTransItem); ++ resolveAltered(itemPairMap, previousItemPair, mTransItem); + break; + case TransactionItemAction::REINSTALLED: + break; +diff --git a/libdnf/transaction/MergedTransaction.hpp b/libdnf/transaction/MergedTransaction.hpp +index dbb8af11..f85b133a 100644 +--- a/libdnf/transaction/MergedTransaction.hpp ++++ b/libdnf/transaction/MergedTransaction.hpp +@@ -76,9 +76,9 @@ protected: + typedef std::map< std::string, ItemPair > ItemPairMap; + + void mergeItem(ItemPairMap &itemPairMap, TransactionItemBasePtr transItem); +- void resolveRPMDifference(ItemPair &previousItemPair, TransactionItemBasePtr mTransItem); +- void resolveErase(ItemPair &previousItemPair, TransactionItemBasePtr mTransItem); +- void resolveAltered(ItemPair &previousItemPair, TransactionItemBasePtr mTransItem); ++ void resolveRPMDifference(ItemPairMap &itemPairMap, ItemPair &previousItemPair, TransactionItemBasePtr mTransItem); ++ void resolveErase(ItemPairMap &itemPairMap, ItemPair &previousItemPair, TransactionItemBasePtr mTransItem); ++ void resolveAltered(ItemPairMap &itemPairMap, ItemPair &previousItemPair, TransactionItemBasePtr mTransItem); + }; + + } // namespace libdnf +diff --git a/tests/libdnf/transaction/MergedTransactionTest.cpp b/tests/libdnf/transaction/MergedTransactionTest.cpp +index 52507700..35fb4250 100644 +--- a/tests/libdnf/transaction/MergedTransactionTest.cpp ++++ b/tests/libdnf/transaction/MergedTransactionTest.cpp +@@ -822,12 +822,7 @@ MergedTransactionTest::test_downgrade_upgrade_remove() + // test merging trans1, trans2 + merged.merge(trans2); + auto items2 = merged.getItems(); +- CPPUNIT_ASSERT_EQUAL(1, (int)items2.size()); +- auto item2 = items2.at(0); +- CPPUNIT_ASSERT_EQUAL(std::string("tour-4.8-1.noarch"), item2->getItem()->toStr()); +- CPPUNIT_ASSERT_EQUAL(std::string("repo1"), item2->getRepoid()); +- CPPUNIT_ASSERT_EQUAL(TransactionItemAction::REINSTALL, item2->getAction()); +- CPPUNIT_ASSERT_EQUAL(TransactionItemReason::USER, item2->getReason()); ++ CPPUNIT_ASSERT_EQUAL(0, (int)items2.size()); + + // test merging trans1, trans2, trans3 + merged.merge(trans3); +-- +2.44.0 + diff --git a/0013-MergedTransaction-Fix-invalid-memory-access-when-dro.patch b/0013-MergedTransaction-Fix-invalid-memory-access-when-dro.patch new file mode 100644 index 0000000..5357566 --- /dev/null +++ b/0013-MergedTransaction-Fix-invalid-memory-access-when-dro.patch @@ -0,0 +1,94 @@ +From 7ee86de4db48ad48217c7631e800618eff14875b Mon Sep 17 00:00:00 2001 +From: Jan Kolarik +Date: Tue, 23 Apr 2024 14:11:19 +0000 +Subject: [PATCH] MergedTransaction: Fix invalid memory access when dropping + items +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Upstream commit: 90d2ffad964a91a7a798b81e15c16eb1e840f257 + +When an item is dropped from the merged transaction, the `ItemPair` reference becomes invalid and should no longer be used. + +Resolves: https://issues.redhat.com/browse/RHEL-17494 +Signed-off-by: Petr Písař +--- + libdnf/transaction/MergedTransaction.cpp | 18 +++++++++++------- + libdnf/transaction/MergedTransaction.hpp | 2 +- + 2 files changed, 12 insertions(+), 8 deletions(-) + +diff --git a/libdnf/transaction/MergedTransaction.cpp b/libdnf/transaction/MergedTransaction.cpp +index 8f26882f..75d2c1e7 100644 +--- a/libdnf/transaction/MergedTransaction.cpp ++++ b/libdnf/transaction/MergedTransaction.cpp +@@ -264,14 +264,15 @@ getItemIdentifier(ItemPtr item) + + /** + * Resolve the difference between RPMs in the first and second transaction item +- * and create a ItemPair of Upgrade, Downgrade or drop the item from the merged +- * transaction set in case of both packages are of the same version. +- * Method is called when original package is being removed and than installed again. ++ * and create a ItemPair of Upgrade, Downgrade or remove the item from the merged ++ * transaction set in case of both packages are the same. ++ * Method is called when original package is being removed and then installed again. + * \param itemPairMap merged transaction set + * \param previousItemPair original item pair + * \param mTransItem new transaction item ++ * \return true if the original and new transaction item differ + */ +-void ++bool + MergedTransaction::resolveRPMDifference(ItemPairMap &itemPairMap, + ItemPair &previousItemPair, + TransactionItemBasePtr mTransItem) +@@ -287,7 +288,7 @@ MergedTransaction::resolveRPMDifference(ItemPairMap &itemPairMap, + firstRPM->getRelease() == secondRPM->getRelease()) { + // Drop the item from merged transaction + itemPairMap.erase(getItemIdentifier(firstItem)); +- return; ++ return false; + } else if ((*firstRPM) < (*secondRPM)) { + // Upgrade to secondRPM + previousItemPair.first->setAction(TransactionItemAction::UPGRADED); +@@ -298,6 +299,7 @@ MergedTransaction::resolveRPMDifference(ItemPairMap &itemPairMap, + mTransItem->setAction(TransactionItemAction::DOWNGRADE); + } + previousItemPair.second = mTransItem; ++ return true; + } + + void +@@ -308,12 +310,14 @@ MergedTransaction::resolveErase(ItemPairMap &itemPairMap, + /* + * The original item has been removed - it has to be installed now unless the rpmdb + * has changed. Resolve the difference between packages and mark it as Upgrade, +- * Reinstall or Downgrade ++ * Downgrade or remove it from the transaction + */ + if (mTransItem->getAction() == TransactionItemAction::INSTALL) { + if (mTransItem->getItem()->getItemType() == ItemType::RPM) { + // resolve the difference between RPM packages +- resolveRPMDifference(itemPairMap, previousItemPair, mTransItem); ++ if (!resolveRPMDifference(itemPairMap, previousItemPair, mTransItem)) { ++ return; ++ } + } else { + // difference between comps can't be resolved + mTransItem->setAction(TransactionItemAction::REINSTALL); +diff --git a/libdnf/transaction/MergedTransaction.hpp b/libdnf/transaction/MergedTransaction.hpp +index f85b133a..50212159 100644 +--- a/libdnf/transaction/MergedTransaction.hpp ++++ b/libdnf/transaction/MergedTransaction.hpp +@@ -76,7 +76,7 @@ protected: + typedef std::map< std::string, ItemPair > ItemPairMap; + + void mergeItem(ItemPairMap &itemPairMap, TransactionItemBasePtr transItem); +- void resolveRPMDifference(ItemPairMap &itemPairMap, ItemPair &previousItemPair, TransactionItemBasePtr mTransItem); ++ bool resolveRPMDifference(ItemPairMap &itemPairMap, ItemPair &previousItemPair, TransactionItemBasePtr mTransItem); + void resolveErase(ItemPairMap &itemPairMap, ItemPair &previousItemPair, TransactionItemBasePtr mTransItem); + void resolveAltered(ItemPairMap &itemPairMap, ItemPair &previousItemPair, TransactionItemBasePtr mTransItem); + }; +-- +2.44.0 + diff --git a/libdnf.spec b/libdnf.spec index e6f4565..5f9a0cc 100644 --- a/libdnf.spec +++ b/libdnf.spec @@ -58,7 +58,7 @@ Name: libdnf Version: %{libdnf_major_version}.%{libdnf_minor_version}.%{libdnf_micro_version} -Release: 9%{?dist} +Release: 10%{?dist} Summary: Library providing simplified C and Python API to libsolv License: LGPLv2+ URL: https://github.com/rpm-software-management/libdnf @@ -74,6 +74,8 @@ Patch8: 0008-PGP-Set-a-default-creation-SELinux-labels-on-GnuPG-d.patch Patch9: 0009-Replace-assert-by-map_grow.patch Patch10: 0010-subject-py-Fix-memory-leak.patch Patch11: 0011-Add-virtual-destructor-to-TransactionItem.patch +Patch12: 0012-MergedTransaction-Calculate-RPM-difference-between-t.patch +Patch13: 0013-MergedTransaction-Fix-invalid-memory-access-when-dro.patch BuildRequires: cmake @@ -323,6 +325,10 @@ popd %endif %changelog +* Tue Apr 23 2024 Petr Pisar - 0.69.0-10 +- Fix calculating a difference between two same-version RPM transacations + (RHEL-17494) + * Tue Apr 16 2024 Petr Pisar - 0.69.0-9 - Grow memory if applying a query after increasing a number of available packages (RHEL-27657)