From 64549e9357e5222a73e41aa87372b37abb047720 Mon Sep 17 00:00:00 2001 From: Michael Buesch Date: Sun, 19 Jul 2009 21:40:39 +0200 Subject: ieee1394: raw1394: Do not leak memory on failed trylock. Do not leak the allocated memory in case the mutex_trylock() failed to acquire the lock. Signed-off-by: Michael Buesch This bug does not happen in practice: All raw1394 clients use libraw1394, and accesses to a libraw1394 handle need to be serialized by the client. This is documented in libraw1394's API reference. Signed-off-by: Stefan Richter diff --git a/drivers/ieee1394/raw1394.c b/drivers/ieee1394/raw1394.c index da5f882..0bc3d78 100644 --- a/drivers/ieee1394/raw1394.c +++ b/drivers/ieee1394/raw1394.c @@ -2272,8 +2272,10 @@ static ssize_t raw1394_write(struct file *file, const char __user * buffer, return -EFAULT; } - if (!mutex_trylock(&fi->state_mutex)) + if (!mutex_trylock(&fi->state_mutex)) { + free_pending_request(req); return -EAGAIN; + } switch (fi->state) { case opened: -- cgit v0.10.2 From 928ec5f148e729076e9202e7c78babede628a50c Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sun, 6 Sep 2009 18:49:17 +0200 Subject: firewire: ohci: fix Self ID Count register mask (safeguard against buffer overflow) The selfIDSize field of Self ID Count is 9 bits wide, and we are only interested in the high 8 bits. Fix the mask accordingly. The previously too large mask didn't do damage though because the next few bits in the register are reserved and therefore zero with presently existing hardware. Also, check for the maximum possible self ID count of 252 (according to OHCI 1.1 clause 11.2 and IEEE 1394a-2000 clause 4.3.4.1, i.e. up to four self IDs of up to 63 nodes, even though IEEE 1394 up to edition 2008 defines only up to three self IDs per node). More than 252 self IDs would only happen if the self ID receive DMA unit malfunctioned, which would likely be caught by other self ID buffer checks. However, check it early to be sure. More than 253 quadlets would overflow the Topology Map CSR. Reported-By: PaX Team Signed-off-by: Stefan Richter diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 76b321b..5d52425 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -1279,8 +1279,8 @@ static void bus_reset_tasklet(unsigned long data) * the inverted quadlets and a header quadlet, we shift one * bit extra to get the actual number of self IDs. */ - self_id_count = (reg >> 3) & 0x3ff; - if (self_id_count == 0) { + self_id_count = (reg >> 3) & 0xff; + if (self_id_count == 0 || self_id_count > 252) { fw_notify("inconsistent self IDs\n"); return; } -- cgit v0.10.2 From 18668ff9a3232d5f942a2f7abc1ad67d2760dcdf Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sun, 6 Sep 2009 18:49:48 +0200 Subject: firewire: core: header file cleanup fw_card_get, fw_card_put, fw_card_release are currently not exported for use outside the firewire-core. Move their definitions/ declarations from the subsystem header file to the core header file. Signed-off-by: Stefan Richter diff --git a/drivers/firewire/core.h b/drivers/firewire/core.h index 6052816..7ff6e75 100644 --- a/drivers/firewire/core.h +++ b/drivers/firewire/core.h @@ -96,6 +96,20 @@ int fw_core_initiate_bus_reset(struct fw_card *card, int short_reset); int fw_compute_block_crc(u32 *block); void fw_schedule_bm_work(struct fw_card *card, unsigned long delay); +static inline struct fw_card *fw_card_get(struct fw_card *card) +{ + kref_get(&card->kref); + + return card; +} + +void fw_card_release(struct kref *kref); + +static inline void fw_card_put(struct fw_card *card) +{ + kref_put(&card->kref, fw_card_release); +} + /* -cdev */ diff --git a/include/linux/firewire.h b/include/linux/firewire.h index 192d1e43..7e1d4de 100644 --- a/include/linux/firewire.h +++ b/include/linux/firewire.h @@ -134,20 +134,6 @@ struct fw_card { u32 topology_map[(CSR_TOPOLOGY_MAP_END - CSR_TOPOLOGY_MAP) / 4]; }; -static inline struct fw_card *fw_card_get(struct fw_card *card) -{ - kref_get(&card->kref); - - return card; -} - -void fw_card_release(struct kref *kref); - -static inline void fw_card_put(struct fw_card *card) -{ - kref_put(&card->kref, fw_card_release); -} - struct fw_attribute_group { struct attribute_group *groups[2]; struct attribute_group group; -- cgit v0.10.2 From b171e204b32b69e241af994d6e9be559e33535c1 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sun, 6 Sep 2009 18:50:29 +0200 Subject: firewire: core: fix race with parallel PCI device probe The config ROM buffer received from generate_config_rom is a globally shared static buffer. Extend the card_mutex protection in fw_add_card until after the config ROM was copied into the card driver's buffer. Otherwise, parallelized card driver probes may end up with ROM contents that were meant for a different card. firewire-ohci's card->driver->enable hook is safe to be called within the card_mutex. Furthermore, it is safe to reorder card_list update versus card enable, which simplifies the code a little. Signed-off-by: Stefan Richter diff --git a/drivers/firewire/core-card.c b/drivers/firewire/core-card.c index f74edae..e4864e8 100644 --- a/drivers/firewire/core-card.c +++ b/drivers/firewire/core-card.c @@ -444,16 +444,13 @@ int fw_card_add(struct fw_card *card, card->guid = guid; mutex_lock(&card_mutex); - config_rom = generate_config_rom(card, &length); - list_add_tail(&card->link, &card_list); - mutex_unlock(&card_mutex); + config_rom = generate_config_rom(card, &length); ret = card->driver->enable(card, config_rom, length); - if (ret < 0) { - mutex_lock(&card_mutex); - list_del(&card->link); - mutex_unlock(&card_mutex); - } + if (ret == 0) + list_add_tail(&card->link, &card_list); + + mutex_unlock(&card_mutex); return ret; } -- cgit v0.10.2 From 85cb9b68640cf467a99d4b6d518f1293222dbc9e Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Tue, 8 Sep 2009 01:13:53 +0200 Subject: firewire: core: fix topology map response handler This register is 1 kBytes large. Adjust topology_map.length to prevent registration of other response handlers in this region and to make sure that we respond to requests to the upper half of the register. Signed-off-by: Stefan Richter diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c index 479b22f..da628c7 100644 --- a/drivers/firewire/core-transaction.c +++ b/drivers/firewire/core-transaction.c @@ -834,7 +834,7 @@ static void handle_topology_map(struct fw_card *card, struct fw_request *request } static struct fw_address_handler topology_map = { - .length = 0x200, + .length = 0x400, .address_callback = handle_topology_map, }; -- cgit v0.10.2 From 094614fc14966bc6a6259ade55f051fe17f36122 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sun, 6 Sep 2009 18:51:27 +0200 Subject: firewire: sbp2: fix status reception Per SBP-2 clause 5.3, a target shall store 8...32 bytes of status information. Trailing zeros after the first 8 bytes don't need to be stored, they are implicit. Fix the status write handler to clear all unwritten status data. Signed-off-by: Stefan Richter diff --git a/drivers/firewire/sbp2.c b/drivers/firewire/sbp2.c index e5df822..8f83bff 100644 --- a/drivers/firewire/sbp2.c +++ b/drivers/firewire/sbp2.c @@ -425,19 +425,20 @@ static void sbp2_status_write(struct fw_card *card, struct fw_request *request, struct sbp2_logical_unit *lu = callback_data; struct sbp2_orb *orb; struct sbp2_status status; - size_t header_size; unsigned long flags; if (tcode != TCODE_WRITE_BLOCK_REQUEST || - length == 0 || length > sizeof(status)) { + length < 8 || length > sizeof(status)) { fw_send_response(card, request, RCODE_TYPE_ERROR); return; } - header_size = min(length, 2 * sizeof(u32)); - fw_memcpy_from_be32(&status, payload, header_size); - if (length > header_size) - memcpy(status.data, payload + 8, length - header_size); + status.status = be32_to_cpup(payload); + status.orb_low = be32_to_cpup(payload + 4); + memset(status.data, 0, sizeof(status.data)); + if (length > 8) + memcpy(status.data, payload + 8, length - 8); + if (STATUS_GET_SOURCE(status) == 2 || STATUS_GET_SOURCE(status) == 3) { fw_notify("non-orb related status write, not handled\n"); fw_send_response(card, request, RCODE_COMPLETE); -- cgit v0.10.2 From 3c5f80357c3fb3170e39e5d0ae87ddd6652f36ac Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sun, 6 Sep 2009 19:33:50 +0200 Subject: firewire: sbp2: remove a workaround for Momobay FX-3A The inquiry delay does more harm than good in tests on a recent kernel. Signed-off-by: Stefan Richter diff --git a/drivers/firewire/sbp2.c b/drivers/firewire/sbp2.c index 8f83bff..50f0176 100644 --- a/drivers/firewire/sbp2.c +++ b/drivers/firewire/sbp2.c @@ -354,8 +354,7 @@ static const struct { /* DViCO Momobay FX-3A with TSB42AA9A bridge */ { .firmware_revision = 0x002800, .model = 0x000000, - .workarounds = SBP2_WORKAROUND_DELAY_INQUIRY | - SBP2_WORKAROUND_POWER_CONDITION, + .workarounds = SBP2_WORKAROUND_POWER_CONDITION, }, /* Initio bridges, actually only needed for some older ones */ { .firmware_revision = 0x000200, -- cgit v0.10.2 From 625f0850a8e27b6a8d6fdb95056f35bc22d92b55 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sun, 6 Sep 2009 19:34:17 +0200 Subject: ieee1394: sbp2: remove a workaround for Momobay FX-3A The inquiry delay is not necessary anymore in tests on a recent kernel. Signed-off-by: Stefan Richter diff --git a/drivers/ieee1394/sbp2.c b/drivers/ieee1394/sbp2.c index 52b25f8..f199896 100644 --- a/drivers/ieee1394/sbp2.c +++ b/drivers/ieee1394/sbp2.c @@ -372,8 +372,7 @@ static const struct { /* DViCO Momobay FX-3A with TSB42AA9A bridge */ { .firmware_revision = 0x002800, .model = 0x000000, - .workarounds = SBP2_WORKAROUND_DELAY_INQUIRY | - SBP2_WORKAROUND_POWER_CONDITION, + .workarounds = SBP2_WORKAROUND_POWER_CONDITION, }, /* Initio bridges, actually only needed for some older ones */ { .firmware_revision = 0x000200, -- cgit v0.10.2