linux-kernel - Re: [PATCH 2/2] pci: Repair pci_save/restore_state so we can restore one save many times.

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 12 Mar 2007 15:46:44 -0700
From:	"Kok, Auke" <auke-jan.h.kok@...el.com>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>
CC:	Jeff Garzik <jeff@...zik.org>,
	"Kok, Auke" <auke-jan.h.kok@...el.com>,
	Ingo Molnar <mingo@...e.hu>,
	"Michael S. Tsirkin" <mst@...lanox.co.il>,
	Pavel Machek <pavel@....cz>,
	Jens Axboe <jens.axboe@...cle.com>,
	Adrian Bunk <bunk@...sta.de>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>, linux-pm@...ts.osdl.org,
	Michal Piotrowski <michal.k.k.piotrowski@...il.com>,
	Greg Kroah-Hartman <gregkh@...e.de>,
	linux-pci@...ey.karlin.mff.cuni.cz, michael@...erman.id.au
Subject: Re: [PATCH 2/2] pci: Repair pci_save/restore_state so we can restore
 one save many times.

Eric W. Biederman wrote:
> Because we do not reserve space for the pci-x and pci-e state in struct
> pci dev we need to dynamically allocate it.  However because we need
> to support restore being called multiple times after a single save
> it is never safe to free the buffers we have allocated to hold the
> state.
> 
> So this patch modifies the save routines to first check to see
> if we have already allocated a state buffer before allocating
> a new one.  Then the restore routines are modified to not free
> the state after restoring it.  Simple and it fixes some subtle
> error path handling bugs, that are hard to test for.
> 
> Signed-off-by: Eric W. Biederman <ebiederm@...ssion.com>


I tested this patch and the other 2 in this series:

[PATCH 0/2] Repair pci_restore_state when used with device resets
[PATCH 1/2] msi: Safer state caching.

against e1000 with suspend/resume functionality. Apart from a minor symmetry 
violation in e1000 for which I will send a patch later, these patches appear to 
work fine on my ich8 with 5 msi capable e1000 ports.

Feel free to add my Signed-off-by: Auke Kok <auke-jan.h.kok@...el.com>

Cheers,

Auke



> ---
>  drivers/pci/pci.c   |   12 ++++++------
>  include/linux/pci.h |    5 -----
>  2 files changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 6fb78df..b292c9a 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -551,7 +551,9 @@ static int pci_save_pcie_state(struct pci_dev *dev)
>  	if (pos <= 0)
>  		return 0;
>  
> -	save_state = kzalloc(sizeof(*save_state) + sizeof(u16) * 4, GFP_KERNEL);
> +	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_EXP);
> +	if (!save_state)
> +		save_state = kzalloc(sizeof(*save_state) + sizeof(u16) * 4, GFP_KERNEL);
>  	if (!save_state) {
>  		dev_err(&dev->dev, "Out of memory in pci_save_pcie_state\n");
>  		return -ENOMEM;
> @@ -582,8 +584,6 @@ static void pci_restore_pcie_state(struct pci_dev *dev)
>  	pci_write_config_word(dev, pos + PCI_EXP_LNKCTL, cap[i++]);
>  	pci_write_config_word(dev, pos + PCI_EXP_SLTCTL, cap[i++]);
>  	pci_write_config_word(dev, pos + PCI_EXP_RTCTL, cap[i++]);
> -	pci_remove_saved_cap(save_state);
> -	kfree(save_state);
>  }
>  
>  
> @@ -597,7 +597,9 @@ static int pci_save_pcix_state(struct pci_dev *dev)
>  	if (pos <= 0)
>  		return 0;
>  
> -	save_state = kzalloc(sizeof(*save_state) + sizeof(u16), GFP_KERNEL);
> +	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_EXP);
> +	if (!save_state)
> +		save_state = kzalloc(sizeof(*save_state) + sizeof(u16), GFP_KERNEL);
>  	if (!save_state) {
>  		dev_err(&dev->dev, "Out of memory in pci_save_pcie_state\n");
>  		return -ENOMEM;
> @@ -622,8 +624,6 @@ static void pci_restore_pcix_state(struct pci_dev *dev)
>  	cap = (u16 *)&save_state->data[0];
>  
>  	pci_write_config_word(dev, pos + PCI_X_CMD, cap[i++]);
> -	pci_remove_saved_cap(save_state);
> -	kfree(save_state);
>  }
>  
>  
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 78417e4..481ea06 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -209,11 +209,6 @@ static inline void pci_add_saved_cap(struct pci_dev *pci_dev,
>  	hlist_add_head(&new_cap->next, &pci_dev->saved_cap_space);
>  }
>  
> -static inline void pci_remove_saved_cap(struct pci_cap_saved_state *cap)
> -{
> -	hlist_del(&cap->next);
> -}
> -
>  /*
>   *  For PCI devices, the region numbers are assigned this way:
>   *
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/