| Message ID | 1269500806-15383-4-git-send-email-amit.salecha@qlogic.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Amit Kumar Salecha <amit.salecha@qlogic.com> Date: Thu, 25 Mar 2010 00:06:45 -0700 > Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com> > > Return value of ioremap is not checked, NULL check added. You're not even thinking when you make these changes. Let's try again ok? These ioremap() calls store the pointers into local variables. So it doesn't work at all to just jump to err_out to clean them up and iounmap them. The unamp code you end up calling in netxen_pci_map() only does an iounmap() on pointers stored in the netxen_adapter struct. At this point they haven't been stored there yet. So they still leak. Can you tell my patience on this patch set is very much running out? If you put half of the effort writing these patches as I am putting into reviewing them, we wouldn't have to go back and forth on this so many times. Double check your work, and resubmit this whole patch set once you've fixed this one, I think the other ones look fine. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
David, Sorry for inconvenience caused. I'll take care in future work. I have submitted reworked patches. -Amit -----Original Message----- From: David Miller [mailto:davem@davemloft.net] Sent: Friday, March 26, 2010 12:35 AM To: Amit Salecha Cc: netdev@vger.kernel.org; Ameen Rahman Subject: Re: [PATCHv1 3/4] netxen: added sanity check for pci map From: Amit Kumar Salecha <amit.salecha@qlogic.com> Date: Thu, 25 Mar 2010 00:06:45 -0700 > Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com> > > Return value of ioremap is not checked, NULL check added. You're not even thinking when you make these changes. Let's try again ok? These ioremap() calls store the pointers into local variables. So it doesn't work at all to just jump to err_out to clean them up and iounmap them. The unamp code you end up calling in netxen_pci_map() only does an iounmap() on pointers stored in the netxen_adapter struct. At this point they haven't been stored there yet. So they still leak. Can you tell my patience on this patch set is very much running out? If you put half of the effort writing these patches as I am putting into reviewing them, we wouldn't have to go back and forth on this so many times. Double check your work, and resubmit this whole patch set once you've fixed this one, I think the other ones look fine. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/netxen/netxen_nic_main.c b/drivers/net/netxen/netxen_nic_main.c index 9a7a0f3..cf06a51 100644 --- a/drivers/net/netxen/netxen_nic_main.c +++ b/drivers/net/netxen/netxen_nic_main.c @@ -636,10 +636,24 @@ netxen_setup_pci_map(struct netxen_adapter *adapter) mem_ptr2 = ioremap(mem_base + THIRD_PAGE_GROUP_START, THIRD_PAGE_GROUP_SIZE); pci_len0 = FIRST_PAGE_GROUP_SIZE; + + if (mem_ptr0 == NULL || mem_ptr1 == NULL || mem_ptr2 == NULL) { + dev_err(&pdev->dev, "failed to map PCI bar 0\n"); + err = -EIO; + goto err_out; + } + } else if (mem_len == NETXEN_PCI_32MB_SIZE) { mem_ptr1 = ioremap(mem_base, SECOND_PAGE_GROUP_SIZE); mem_ptr2 = ioremap(mem_base + THIRD_PAGE_GROUP_START - SECOND_PAGE_GROUP_START, THIRD_PAGE_GROUP_SIZE); + + if (mem_ptr1 == NULL || mem_ptr2 == NULL) { + dev_err(&pdev->dev, "failed to map PCI bar 0\n"); + err = -EIO; + goto err_out; + } + } else if (mem_len == NETXEN_PCI_2MB_SIZE) { mem_ptr0 = pci_ioremap_bar(pdev, 0);
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com> Return value of ioremap is not checked, NULL check added. --- drivers/net/netxen/netxen_nic_main.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)