Hi,
On Fri, Apr 17, 2009 at 11:33 AM, Tejun Heo <tj@xxxxxxxxxx> wrote:
> Impact: fix an oops which always triggers
>
> ide_tape_issue_pc() assumed drive->pc isn't NULL on invocation when
> checking for back-to-back request sense issues but drive->pc can be
> NULL and even when it's not NULL, it's not safe to dereference it once
> the previous command is complete because pc could have been freed or
> was on stack. Kill back-to-back REQUEST_SENSE detection.
>
> Signed-off-by: Tejun Heo <tj@xxxxxxxxxx>
> ---
> drivers/ide/ide-tape.c | 6 ------
> 1 files changed, 0 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
> index cb942a9..3a53e08 100644
> --- a/drivers/ide/ide-tape.c
> +++ b/drivers/ide/ide-tape.c
> @@ -614,12 +614,6 @@ static ide_startstop_t ide_tape_issue_pc(ide_drive_t *drive,
> {
> idetape_tape_t *tape = drive->driver_data;
>
> - if (drive->pc->c[0] == REQUEST_SENSE &&
> - pc->c[0] == REQUEST_SENSE) {
> - printk(KERN_ERR "ide-tape: possible ide-tape.c bug - "
> - "Two request sense in serial were issued\n");
> - }
> -
> if (drive->failed_pc == NULL && pc->c[0] != REQUEST_SENSE)
> drive->failed_pc = pc;
>
I hit that too when debugging an ide-tape problem a user has
(http://bugzilla.kernel.org/show_bug.cgi?id=12874). However, this is not the
proper solution since, currently, ide-tape stuffs all packet commands in
rq->buffer or rq->special now after your changes. It has to get them out of
there in the ->do_request callback and set drive->pc to point to the current
packet command in flight through the IRQ handler. And since ide_tape_issue_pc()
is called by the ->do_request callback we should have the drive->pc always
valid.
How about something like that instead:
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 4e6181c..171dbcd 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -792,6 +792,9 @@ static ide_startstop_t
idetape_do_request(ide_drive_t *drive,
struct request *postponed_rq = tape->postponed_rq;
u8 stat;
+ if (rq->cmd_type == REQ_TYPE_SPECIAL)
+ drive->pc = (struct ide_atapi_pc *) rq->buffer;
+
debug_log(DBG_SENSE, "sector: %llu, nr_sectors: %lu,"
" current_nr_sectors: %u\n",
(unsigned long long)rq->sector, rq->nr_sectors,
--
Regards/Gruss,
Boris
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
