linux-kernel - Re: "scsi: convert host_busy to atomic_t" series causes regressions for some hardware configurations

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <55ED3D44.5060502@gmail.com>
Date:	Mon, 7 Sep 2015 09:31:16 +0200
From:	Sergio Callegari <sergio.callegari@...il.com>
To:	Christoph Hellwig <hch@...radead.org>
Cc:	Borislav Petkov <bp@...en8.de>, linux-kernel@...r.kernel.org
Subject: Re: "scsi: convert host_busy to atomic_t" series causes regressions
 for some hardware configurations

Hi Christoph (and those reading!),

I wonder if there might be any update or, most important, anything else for me 
to test in order to provide info to address this issue...

I really would like to find out whether it is a bug in my hardware (which would 
be OK, as I know already how to work around it) or something that may happen on 
any hardware as soon as an unfortunate combination of storage equipment is adopted.

Thanks for the help so far,

Best regards,

Sergio

On 30/08/2015 12:54, Sergio Callegari wrote:
> Hi Christoph,
>
> just checked.
>
> Unfortunately, the patch below, applied on top of Linus' v3.17 (which I am 
> using as a test kernel) *does not fix the issue*.
>
> Best regards,
>
> Sergio
>
> On 25/08/2015 13:00, Christoph Hellwig wrote:
>> Hi Sergio,
>>
>> can you give the patch below a try?
>>
>> libata currently completes the SCSI command before freeing the internal
>> command structure, which could lead to various races that mess with
>> the ATA command state, which might cause issues like the one you see.
>>
>> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
>> index 641a61a..92cc156 100644
>> --- a/drivers/ata/libata-scsi.c
>> +++ b/drivers/ata/libata-scsi.c
>> @@ -1772,6 +1772,15 @@ nothing_to_do:
>>       return 1;
>>   }
>>   +static void ata_qc_done(struct ata_queued_cmd *qc)
>> +{
>> +    struct scsi_cmnd *cmd = qc->scsicmd;
>> +    void (*done)(struct scsi_cmnd *) = qc->scsidone;
>> +
>> +    ata_qc_free(qc);
>> +    done(cmd);
>> +}
>> +
>>   static void ata_scsi_qc_complete(struct ata_queued_cmd *qc)
>>   {
>>       struct ata_port *ap = qc->ap;
>> @@ -1810,9 +1819,7 @@ static void ata_scsi_qc_complete(struct ata_queued_cmd 
>> *qc)
>>       if (need_sense && !ap->ops->error_handler)
>>           ata_dump_status(ap->print_id, &qc->result_tf);
>>   -    qc->scsidone(cmd);
>> -
>> -    ata_qc_free(qc);
>> +    ata_qc_done(qc);
>>   }
>>     /**
>> @@ -2611,8 +2618,7 @@ static void atapi_sense_complete(struct ata_queued_cmd 
>> *qc)
>>           ata_gen_passthru_sense(qc);
>>       }
>>   -    qc->scsidone(qc->scsicmd);
>> -    ata_qc_free(qc);
>> +    ata_qc_done(qc);
>>   }
>>     /* is it pointless to prefer PIO for "safety reasons"? */
>> @@ -2707,8 +2713,7 @@ static void atapi_qc_complete(struct ata_queued_cmd *qc)
>>               qc->dev->sdev->locked = 0;
>>             qc->scsicmd->result = SAM_STAT_CHECK_CONDITION;
>> -        qc->scsidone(cmd);
>> -        ata_qc_free(qc);
>> +        ata_qc_done(qc);
>>           return;
>>       }
>>   @@ -2752,8 +2757,7 @@ static void atapi_qc_complete(struct ata_queued_cmd *qc)
>>           cmd->result = SAM_STAT_GOOD;
>>       }
>>   -    qc->scsidone(cmd);
>> -    ata_qc_free(qc);
>> +    ata_qc_done(qc);
>>   }
>>   /**
>>    *    atapi_xlat - Initialize PACKET taskfile
>

--
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/