linux-kernel - [git pull] FireWire regression fix

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>] [day] [month] [year] [list]
Message-ID: <20110504134520.0a0aa35e@stein>
Date:	Wed, 4 May 2011 13:45:20 +0200
From:	Stefan Richter <stefanr@...6.in-berlin.de>
To:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Cc:	linux-kernel@...r.kernel.org, linux1394-devel@...ts.sourceforge.net
Subject: [git pull] FireWire regression fix

Linus, please pull from the fixes branch at

    git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git fixes

to receive the following update for the IEEE 1394 (FireWire) subsystem.
Thanks.

B.J. Buchalter (1):
      firewire: Fix for broken configrom updates in quick succession

 drivers/firewire/ohci.c |   39 +++++++++++++++++++++++++--------------
 1 files changed, 25 insertions(+), 14 deletions(-)


commit 2e053a27d9d5ad5e0831e002cbf8043836fb2060
Author: B.J. Buchalter <bj@...abs.com>
Date:   Mon May 2 13:33:42 2011 -0400

    firewire: Fix for broken configrom updates in quick succession
    
    Current implementation of ohci_set_config_rom() uses a deferred
    bus reset via fw_schedule_bus_reset(). If clients add multiple
    unit descriptors to the config_rom in quick succession, the
    deferred bus reset may not have fired before succeeding update
    requests have come in. This can lead to an incorrect partial
    update of the config_rom for both addition and removal of
    config_rom descriptors, as the ohci_set_config_rom() routine
    will return -EBUSY if a previous pending update has not been
    completed yet; the requested update just gets dropped on the floor.
    
    This patch recognizes that the "in-flight" update can be modified
    until it has been processed by the bus-reset, and the locking
    in the bus_reset_tasklet ensures that the update is done atomically
    with respect to modifications made by ohci_set_config_rom(). The
    -EBUSY error case is simply removed.
    
    [Stefan R:  The bug always existed at least theoretically.  But it
    became easy to trigger since 2.6.36 commit 02d37bed188c "firewire: core:
    integrate software-forced bus resets with bus management" which
    introduced long mandatory delays between janitorial bus resets.]
    
    Signed-off-by: Benjamin Buchalter <bj@...abs.com>
    Signed-off-by: Stefan Richter <stefanr@...6.in-berlin.de> (trivial style changes)
    Cc: <stable@...nel.org> # 2.6.36.y and newer

diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index f903d7b6..23d1468 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -2199,7 +2199,6 @@ static int ohci_set_config_rom(struct fw_card *card,
 {
 	struct fw_ohci *ohci;
 	unsigned long flags;
-	int ret = -EBUSY;
 	__be32 *next_config_rom;
 	dma_addr_t uninitialized_var(next_config_rom_bus);
 
@@ -2240,22 +2239,37 @@ static int ohci_set_config_rom(struct fw_card *card,
 
 	spin_lock_irqsave(&ohci->lock, flags);
 
+	/*
+	 * If there is not an already pending config_rom update,
+	 * push our new allocation into the ohci->next_config_rom
+	 * and then mark the local variable as null so that we
+	 * won't deallocate the new buffer.
+	 *
+	 * OTOH, if there is a pending config_rom update, just
+	 * use that buffer with the new config_rom data, and
+	 * let this routine free the unused DMA allocation.
+	 */
+
 	if (ohci->next_config_rom == NULL) {
 		ohci->next_config_rom = next_config_rom;
 		ohci->next_config_rom_bus = next_config_rom_bus;
+		next_config_rom = NULL;
+	}
 
-		copy_config_rom(ohci->next_config_rom, config_rom, length);
+	copy_config_rom(ohci->next_config_rom, config_rom, length);
 
-		ohci->next_header = config_rom[0];
-		ohci->next_config_rom[0] = 0;
+	ohci->next_header = config_rom[0];
+	ohci->next_config_rom[0] = 0;
 
-		reg_write(ohci, OHCI1394_ConfigROMmap,
-			  ohci->next_config_rom_bus);
-		ret = 0;
-	}
+	reg_write(ohci, OHCI1394_ConfigROMmap, ohci->next_config_rom_bus);
 
 	spin_unlock_irqrestore(&ohci->lock, flags);
 
+	/* If we didn't use the DMA allocation, delete it. */
+	if (next_config_rom != NULL)
+		dma_free_coherent(ohci->card.device, CONFIG_ROM_SIZE,
+				  next_config_rom, next_config_rom_bus);
+
 	/*
 	 * Now initiate a bus reset to have the changes take
 	 * effect. We clean up the old config rom memory and DMA
@@ -2263,13 +2277,10 @@ static int ohci_set_config_rom(struct fw_card *card,
 	 * controller could need to access it before the bus reset
 	 * takes effect.
 	 */
-	if (ret == 0)
-		fw_schedule_bus_reset(&ohci->card, true, true);
-	else
-		dma_free_coherent(ohci->card.device, CONFIG_ROM_SIZE,
-				  next_config_rom, next_config_rom_bus);
 
-	return ret;
+	fw_schedule_bus_reset(&ohci->card, true, true);
+
+	return 0;
 }
 
 static void ohci_send_request(struct fw_card *card, struct fw_packet *packet)

-- 
Stefan Richter
-=====-==-== -=-= --=--
http://arcgraph.de/sr/
--
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/