lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 29 May 2019 17:38:15 +0200 From: Auger Eric <eric.auger@...hat.com> To: Christoph Hellwig <hch@...radead.org> Cc: eric.auger.pro@...il.com, joro@...tes.org, iommu@...ts.linux-foundation.org, linux-kernel@...r.kernel.org, dwmw2@...radead.org, robin.murphy@....com, jean-philippe.brucker@....com, alex.williamson@...hat.com Subject: Re: [PATCH v5 1/7] iommu: Fix a leak in iommu_insert_resv_region Hi Christoph, On 5/29/19 8:17 AM, Christoph Hellwig wrote: >> } else if ((start >= a) && (end <= b)) { >> if (new->type == type) >> - goto done; >> + return 0; >> else >> pos = pos->next; > > Please remove the pointless else after the return statement. Sorry I don't get your point here. For both cases, we only return if both types match. Otherwise we continue parsing the list as there may be a region of the same type as @new following the current interval that may need to be merged with @new. Let's consider the insertion of those resv regions: add_resv(head, 0xa001000, 0xa001FFF, IOMMU_RESV_TYPE1); head -> 0x000000000a001000 0x000000000a001fff type1 add_resv(head, 0xa003000, 0xa003FFF, IOMMU_RESV_TYPE1); head -> 0x000000000a001000 0x000000000a001fff type1 -> 0x000000000a003000 0x000000000a003fff type1 add_resv(head, 0xa002000, 0xa004FFF, IOMMU_RESV_TYPE2); head -> 0x000000000a001000 0x000000000a001fff type1 -> 0x000000000a003000 0x000000000a003fff type1 -> 0x000000000a002000 0x000000000a004fff type2 -> add_resv(head, 0xa0010FF, 0xa003000, IOMMU_RESV_TYPE2); head -> 0x000000000a001000 0x000000000a001fff type1 -> 0x000000000a003000 0x000000000a003fff type1 -> 0x000000000a0010ff 0x000000000a004fff type2 <- merge Or maybe I simply missed your point. Please let me know. Thanks Eric > >> } else { >> if (new->type == type) { >> phys_addr_t new_start = min(a, start); >> phys_addr_t new_end = max(b, end); >> + int ret; >> >> list_del(&entry->list); >> entry->start = new_start; >> entry->length = new_end - new_start + 1; >> - iommu_insert_resv_region(entry, regions); >> + ret = iommu_insert_resv_region(entry, regions); >> + kfree(entry); >> + return ret; >> } else { >> pos = pos->next; >> } > > Same here. >
Powered by blists - more mailing lists