On Dec 22 Lubomir Rintel wrote:
> This makes it possible to debug kernel over FireWire without the need to
> recompile it.
>
> Cc: Stefan Richter <stef...@s5r6.in-berlin.de>
> Cc: Dave Hansen <dave.han...@linux.intel.com>
> Signed-off-by: Lubomir Rintel <lkund...@v3.sk>
Looks good to me. A load-time option is preferable over a compile-time
option not only from the POV of the debugging use case, but also from the
maintenance POV.
It weakens security in two scenarios though, AFAICS:
A)
- There are firewire-ohci and firewire-sbp2 installed on the machine,
- the attacker cannot upload code
- but can load kernel modules
- and has physical access to a 1394 port
- and is not able to run a minimal SBP-2 target on the remote 1394 end.
B)
- There is firewire-ohci but not firewire-sbp2 installed on the machine,
- the attacker cannot upload code
- but can load kernel modules
- and has physical access to a 1394 port.
(In both scenarios, the attacker additionally has to be able to /un/load
kernel modules if firewire-ohci was loaded already before the attack.)
That's both quite specific. Hence the security impact of this patch is
negligible in my opinion. Any other opinions or insights into it?
> ---
> Documentation/debugging-via-ohci1394.txt | 4 +---
> drivers/firewire/ohci.c | 19 +++++++++++--------
> lib/Kconfig.debug | 11 -----------
> 3 files changed, 12 insertions(+), 22 deletions(-)
>
> diff --git a/Documentation/debugging-via-ohci1394.txt
> b/Documentation/debugging-via-ohci1394.txt
> index 14d1944..73473aa 100644
> --- a/Documentation/debugging-via-ohci1394.txt
> +++ b/Documentation/debugging-via-ohci1394.txt
> @@ -38,9 +38,7 @@ Drivers
>
> The firewire-ohci driver in drivers/firewire uses filtered physical
> DMA by default, which is more secure but not suitable for remote debugging.
> -Compile the driver with CONFIG_FIREWIRE_OHCI_REMOTE_DMA (Kernel hacking menu:
> -Remote debugging over FireWire with firewire-ohci) to get unfiltered physical
> -DMA.
> +Pass the remote_dma=1 parameter to the driver to get unfiltered physical DMA.
>
> Because the firewire-ohci driver depends on the PCI enumeration to be
> completed, an initialization routine which runs pretty early has been
> diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
> index 6aa8a86..6313735 100644
> --- a/drivers/firewire/ohci.c
> +++ b/drivers/firewire/ohci.c
> @@ -370,6 +370,10 @@ MODULE_PARM_DESC(debug, "Verbose logging (default = 0"
> ", busReset events = " __stringify(OHCI_PARAM_DEBUG_BUSRESETS)
> ", or a combination, or all = -1)");
>
> +static bool param_remote_dma;
> +module_param_named(remote_dma, param_remote_dma, bool, 0444);
Permissions could also be 0644; then it would not be required to unload
firewire-ohci before altering the mode. A hot change of this parameter
would only become effective after a 1394 bus reset though. 0444 keep
things simpler to understand though.
> +MODULE_PARM_DESC(remote_dma, "Enable unfiltered remote DMA (default = 0)");
Trivial comment: I suggest the wording 'default = N' here.
0/1, n/y, or N/Y can be used to set the parameter value, but N/Y will be
shown as value in sysfs.
> +
> static void log_irqs(struct fw_ohci *ohci, u32 evt)
> {
> if (likely(!(param_debug &
> @@ -2050,10 +2054,10 @@ static void bus_reset_work(struct work_struct *work)
> be32_to_cpu(ohci->next_header));
> }
>
> -#ifdef CONFIG_FIREWIRE_OHCI_REMOTE_DMA
> - reg_write(ohci, OHCI1394_PhyReqFilterHiSet, ~0);
> - reg_write(ohci, OHCI1394_PhyReqFilterLoSet, ~0);
> -#endif
> + if (param_remote_dma) {
> + reg_write(ohci, OHCI1394_PhyReqFilterHiSet, ~0);
> + reg_write(ohci, OHCI1394_PhyReqFilterLoSet, ~0);
> + }
>
> spin_unlock_irq(&ohci->lock);
>
> @@ -2587,13 +2591,13 @@ static int ohci_cancel_packet(struct fw_card *card,
> struct fw_packet *packet)
> static int ohci_enable_phys_dma(struct fw_card *card,
> int node_id, int generation)
> {
> -#ifdef CONFIG_FIREWIRE_OHCI_REMOTE_DMA
> - return 0;
> -#else
> struct fw_ohci *ohci = fw_ohci(card);
> unsigned long flags;
> int n, ret = 0;
>
> + if (param_remote_dma)
> + return 0;
> +
> /*
> * FIXME: Make sure this bitmask is cleared when we clear the busReset
> * interrupt bit. Clear physReqResourceAllBuses on bus reset.
> @@ -2622,7 +2626,6 @@ static int ohci_enable_phys_dma(struct fw_card *card,
> spin_unlock_irqrestore(&ohci->lock, flags);
>
> return ret;
> -#endif /* CONFIG_FIREWIRE_OHCI_REMOTE_DMA */
> }
>
> static u32 ohci_read_csr(struct fw_card *card, int csr_offset)
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index db25707..dc30284 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1547,17 +1547,6 @@ config PROVIDE_OHCI1394_DMA_INIT
>
> See Documentation/debugging-via-ohci1394.txt for more information.
>
> -config FIREWIRE_OHCI_REMOTE_DMA
> - bool "Remote debugging over FireWire with firewire-ohci"
> - depends on FIREWIRE_OHCI
> - help
> - This option lets you use the FireWire bus for remote debugging
> - with help of the firewire-ohci driver. It enables unfiltered
> - remote DMA in firewire-ohci.
> - See Documentation/debugging-via-ohci1394.txt for more information.
> -
> - If unsure, say N.
> -
> config BUILD_DOCSRC
> bool "Build targets in Documentation/ tree"
> depends on HEADERS_CHECK
--
Stefan Richter
-=====-===-= ==-- =-===
http://arcgraph.de/sr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
