Skip to content

Commit

Permalink
firewire: ohci: fix race in AR split packet handling
Browse files Browse the repository at this point in the history
When handling an AR buffer that has been completely filled, we assumed
that its descriptor will not be read by the controller and can be
overwritten.  However, when the last received packet happens to end at
the end of the buffer, the controller might not yet have moved on to the
next buffer and might read the branch address later.  If we overwrite
and free the page before that, the DMA context will either go dead
because of an invalid Z value, or go off into some random memory.

To fix this, ensure that the descriptor does not get overwritten by
using only the actual buffer instead of the entire page for reassembling
the split packet.  Furthermore, to avoid freeing the page too early,
move on to the next buffer only when some data in it guarantees that the
controller has moved on.

This should eliminate the remaining firewire-net problems.

Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Cc: 2.6.22-2.6.36 <stable@kernel.org>
Tested-by: Maxim Levitsky <maximlevitsky@gmail.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
  • Loading branch information
Clemens Ladisch authored and Stefan Richter committed Oct 30, 2010
1 parent 85f7ffd commit a1f805e
Showing 1 changed file with 21 additions and 14 deletions.
35 changes: 21 additions & 14 deletions drivers/firewire/ohci.c
Original file line number Diff line number Diff line change
Expand Up @@ -750,20 +750,19 @@ static void ar_context_tasklet(unsigned long data)
*/

offset = offsetof(struct ar_buffer, data);
start = buffer = ab;
start = ab;
start_bus = le32_to_cpu(ab->descriptor.data_address) - offset;
buffer = ab->data;

ab = ab->next;
d = &ab->descriptor;
size = buffer + PAGE_SIZE - ctx->pointer;
size = start + PAGE_SIZE - ctx->pointer;
/* valid buffer data in the next page */
rest = le16_to_cpu(d->req_count) - le16_to_cpu(d->res_count);
/* what actually fits in this page */
size2 = min(rest, (size_t)PAGE_SIZE - size);
size2 = min(rest, (size_t)PAGE_SIZE - offset - size);
memmove(buffer, ctx->pointer, size);
memcpy(buffer + size, ab->data, size2);
ctx->current_buffer = ab;
ctx->pointer = (void *) ab->data + rest;

while (size > 0) {
void *next = handle_ar_packet(ctx, buffer);
Expand All @@ -782,22 +781,30 @@ static void ar_context_tasklet(unsigned long data)
size -= pktsize;
/* fill up this page again */
size3 = min(rest - size2,
(size_t)PAGE_SIZE - size - size2);
(size_t)PAGE_SIZE - offset - size - size2);
memcpy(buffer + size + size2,
(void *) ab->data + size2, size3);
size2 += size3;
}

/* handle the packets that are fully in the next page */
buffer = (void *) ab->data + (buffer - (start + size));
end = (void *) ab->data + rest;
if (rest > 0) {
/* handle the packets that are fully in the next page */
buffer = (void *) ab->data +
(buffer - (start + offset + size));
end = (void *) ab->data + rest;

while (buffer < end)
buffer = handle_ar_packet(ctx, buffer);
while (buffer < end)
buffer = handle_ar_packet(ctx, buffer);

ctx->current_buffer = ab;
ctx->pointer = end;

dma_free_coherent(ohci->card.device, PAGE_SIZE,
start, start_bus);
ar_context_add_page(ctx);
dma_free_coherent(ohci->card.device, PAGE_SIZE,
start, start_bus);
ar_context_add_page(ctx);
} else {
ctx->pointer = start + PAGE_SIZE;
}
} else {
buffer = ctx->pointer;
ctx->pointer = end =
Expand Down

0 comments on commit a1f805e

Please sign in to comment.