linux-kernel - Re: [PATCH 0/5]scsi:scsi_debug: Add error injection for single device

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]

Date:   Thu, 23 Mar 2023 16:25:11 +0000
From:   John Garry <john.g.garry@...cle.com>
To:     "haowenchao (C)" <haowenchao2@...wei.com>,
        "James E . J . Bottomley" <jejb@...ux.ibm.com>,
        "Martin K . Petersen" <martin.petersen@...cle.com>,
        Douglas Gilbert <dgilbert@...erlog.com>,
        linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     linfeilong@...wei.com, louhongxiang@...wei.com
Subject: Re: [PATCH 0/5]scsi:scsi_debug: Add error injection for single device

On 23/03/2023 13:13, haowenchao (C) wrote:
> On 2023/3/23 20:40, John Garry wrote:
>> On 23/03/2023 11:55, Wenchao Hao wrote:
>>> The original error injection mechanism was based on scsi_host which
>>> could not inject fault for a single SCSI device.
>>>
>>> This patchset provides the ability to inject errors for a single
>>> SCSI device. Now we supports inject timeout errors, queuecommand
>>> errors, and hostbyte, driverbyte, statusbyte, and sense data for
>>> specific SCSI Command.
>>
>> There is already a basic mechanism to generate errors - like timeouts 
>> - on "nth" command. Can you say why you want this new interface? What 
>> special scenarios are you trying to test/validate (which could not be 
>> achieved based on the current mechanism)?
>>
> 
> I am testing a new error handle policy which is based on single scsi_device
> without set host to RECOVERY. So I need a method to generate errors for
> single SCSI devices.
> 
> While we can not generate errors for single device with current mechanism
> because it is designed for host-wide error generation.
> 
>> With this series we would have 2x methods to inject errors, which is 
>> less than ideal, and they seem to possibly conflict as well, e.g. I 
>> set timeout for nth command via current interface and then use the new 
>> interface to set timeout for some other cadence. What behavior to 
>> expect ...?
> 
> I did not take this issue in consideration. I now assume the users would
> not use these 2 methods at same time.
> 
> What's more, I don not know where to write the usage of this newly added
> interface, maybe we can explain these in doc?

sysfs entries are described in Documentation/ABI, but please don't add 
elaborate programming interfaces in sysfs files (like in these patches) 
- a sysfs file should be just for reading or writing a single value

> 
>>
>> I'm not saying that I am a huge fan of the current inject mechanism, 
>> but at the very least you need to provide more justification for this 
>> series.
>>>>
>>> The first patch add an sysfs interface to add and inquiry single
>>> device's error injection info; the second patch defined how to remove
>>> an injection which has been added. The following 3 patches use the
>>> injection info and generate the related error type.
>>>
>>> Wenchao Hao (5):
>>>    scsi:scsi_debug: Add sysfs interface to manage scsi devices' error
>>>      injection
>>>    scsi:scsi_debug: Define grammar to remove added error injection
>>>    scsi:scsi_debug: timeout command if the error is injected
>>>    scsi:scsi_debug: Return failed value if the error is injected
>>>    scsi:scsi_debug: set command's result and sense data if the error is
>>>      injected
>>>
>>>   drivers/scsi/scsi_debug.c | 296 ++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 296 insertions(+)
>>>
>>
>>
>