Re: memory corruption/panic solved ("FAILURE - ATAPI_IDENTIFY no interrupt")
From: Søren Schmidt (sos_at_DeepCore.dk)
Date: 07/31/04
- Previous message: Oliver Eikemeier: "Re: RFC: Alternate patch to have true new-style rc.d scripts inports (without touching localpkg)"
- In reply to: Nate Lawson: "memory corruption/panic solved ("FAILURE - ATAPI_IDENTIFY no interrupt")"
- Next in thread: Tai-hwa Liang: "Re: memory corruption/panic solved ("FAILURE - ATAPI_IDENTIFY no interrupt")"
- Reply: Tai-hwa Liang: "Re: memory corruption/panic solved ("FAILURE - ATAPI_IDENTIFY no interrupt")"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Date: Sat, 31 Jul 2004 11:10:10 +0200 To: Nate Lawson <nate@root.org>
Nate Lawson wrote:
> I've tracked down the source of the memory corruption in -current that
> results when booting with various CD and DVD drives (especially the ones
> that come with Thinkpads including T23, R32, T41, etc.) The panic is
> obvious when running with INVARIANTS ("memory modified after free") but
> not so obvious in other configurations. For instance, without
> INVARIANTS, part of the rt_info structure is corrupted on my wireless
> card, resulting in a panic during ifconfig on boot. This is likely the
> source of other problems, including phk's ACPI panic (again, only
> triggered when booting with the CD drive in the bay.)
OK, first thanks for digging into this!
> The root problem is that ata_timeout() fires and calls ata_pio_read()
> which overwrites 512 bytes random memory. There are actually two bugs
> here that overwrite memory. The code path is as follows:
>
> 1. ata runs an IDENTIFY command on each drive. It reaches this stack:
>
> ata_getparam()
> ata_identify_devices()
> ata_boot_attach()
>
> 2. ata_getparam() allocates a request and runs it:
> ata_alloc_request()
> loop on retries (2 max)
> fill out an immediate read request for 512 bytes (DEV_BSIZE)
> *** Bug 1: transfersize is 512 bytes but sizeof(struct ata_request) is
> much less (~80 bytes).
> ata_queue_request() starts the request and arms a timeout
Its correct to allocate via ata_alloc_request, the data is *not* put
into the request, but into another memory area (atadev->param) so this
is not a bug.
> 3. Completion for first request results in FAILURE - no interrupt.
> ata_completed() calls ata_generic_interrupt() which calls ata_pio_read()
> *** Bug 1: here's where the 512 - sizeof(struct ata_request) bytes
> following "request" are overwritten.
See above, the data hits the malloc'd buffer (atadev->param) *not* the
request.
> . The completion code also updates request->donecount (an
> upward-counting residual) from 0 to 512 after the request has been
> completed.
>
> 4. ata_getparam runs through the loop again and starts another identify
> *** Bug 2: donecount is now 512 so ata_pio_read() will write on
> request->data + 512 with 512 bytes. Whatever follows request->data +
> 512 is overwritten.
Thats the real bug, donecount should be reset on retry..
> @@ -561,11 +561,12 @@
> request->data = (caddr_t)atadev->param;
> request->bytecount = sizeof(struct ata_params);
> request->transfersize = DEV_BSIZE;
> + request->donecount = 0;
> ata_queue_request(request);
> if (!(error = request->result))
> break;
> }
This part should be all thats needed IMHO...
-Søren
_______________________________________________
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org"
- Previous message: Oliver Eikemeier: "Re: RFC: Alternate patch to have true new-style rc.d scripts inports (without touching localpkg)"
- In reply to: Nate Lawson: "memory corruption/panic solved ("FAILURE - ATAPI_IDENTIFY no interrupt")"
- Next in thread: Tai-hwa Liang: "Re: memory corruption/panic solved ("FAILURE - ATAPI_IDENTIFY no interrupt")"
- Reply: Tai-hwa Liang: "Re: memory corruption/panic solved ("FAILURE - ATAPI_IDENTIFY no interrupt")"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Relevant Pages
|
|
