linux-kernel - Re: [PATCH] PCI: Clear Bus Master bit only on kexec reboot

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]

Message-ID: <87ob55my8z.fsf@xmission.com>
Date:	Wed, 27 Nov 2013 11:38:04 -0800
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Khalid Aziz <khalid.aziz@...cle.com>
Cc:	bhelgaas@...gle.com, cl91tp@...il.com, tianyu.lan@...el.com,
	khlebnikov@...nvz.org, gnomes@...rguk.ukuu.org.uk,
	indou.takao@...fujitsu.com, jility09@...il.com, f.otti@....at,
	mjg59@...f.ucam.org, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org, kexec@...ts.infradead.org,
	stable@...r.kernel.org
Subject: Re: [PATCH] PCI: Clear Bus Master bit only on kexec reboot

Khalid Aziz <khalid.aziz@...cle.com> writes:

> Add a flag to tell the PCI subsystem that kernel is shutting down
> in prepapration to kexec a kernel. Add code in PCI subsystem to use
> this flag to clear Bus Master bit on PCI devices only in case of
> kexec reboot. This fixes https://bugzilla.kernel.org/show_bug.cgi?id=63861
> and avoids any other issues caused by clearing Bus Master bit on PCI
> devices in normal shutdown path. This patch is based on discussion at
> http://marc.info/?l=linux-pci&m=138425645204355&w=2

Scratches head.

Given that most devices already call pci_disable_device which clears the
bus master bit how does this change anything meaningful?

Is is the problem here that most drivers are lazy and have a noop
shutdown method?

Eric


> Signed-off-by: Khalid Aziz <khalid.aziz@...cle.com>
> Acked-by: Konstantin Khlebnikov <koct9i@...il.com>
> Cc: stable@...r.kernel.org
> ---
>  drivers/pci/pci-driver.c | 9 ++++++---
>  drivers/pci/pci.h        | 3 +++
>  kernel/kexec.c           | 4 ++++
>  3 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 9042fdb..e920195 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -400,10 +400,13 @@ static void pci_device_shutdown(struct device *dev)
>  	pci_msix_shutdown(pci_dev);
>  
>  	/*
> -	 * Turn off Bus Master bit on the device to tell it to not
> -	 * continue to do DMA. Don't touch devices in D3cold or unknown states.
> +	 * If this is a kexec reboot, turn off Bus Master bit on the
> +	 * device to tell it to not continue to do DMA. Don't touch
> +	 * devices in D3cold or unknown states.
> +	 * If it is not a kexec reboot, firmware will hit the PCI
> +	 * devices with big hammer and stop their DMA any way.
>  	 */
> -	if (pci_dev->current_state <= PCI_D3hot)
> +	if (kexec_in_progress && (pci_dev->current_state <= PCI_D3hot))
>  		pci_clear_master(pci_dev);
>  }
>  
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 9c91ecc..7d85733 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -9,6 +9,9 @@
>  extern const unsigned char pcix_bus_speed[];
>  extern const unsigned char pcie_link_speed[];
>  
> +/* flag to track if kexec reboot is in progress */
> +extern unsigned long kexec_in_progress;
> +
>  /* Functions internal to the PCI core code */
>  
>  int pci_create_sysfs_dev_files(struct pci_dev *pdev);
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index 490afc0..fd2d63e 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
> @@ -47,6 +47,9 @@ u32 vmcoreinfo_note[VMCOREINFO_NOTE_SIZE/4];
>  size_t vmcoreinfo_size;
>  size_t vmcoreinfo_max_size = sizeof(vmcoreinfo_data);
>  
> +/* Flag to indicate we are going to kexec a new kernel */
> +unsigned long kexec_in_progress = 0;
> +
>  /* Location of the reserved area for the crash kernel */
>  struct resource crashk_res = {
>  	.name  = "Crash kernel",
> @@ -1675,6 +1678,7 @@ int kernel_kexec(void)
>  	} else
>  #endif
>  	{
> +		kexec_in_progress = 1;
>  		kernel_restart_prepare(NULL);
>  		printk(KERN_EMERG "Starting new kernel\n");
>  		machine_shutdown();
--
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/