John Snow <js...@redhat.com> writes:
> We're a little too lenient with what we'll let an ATAPI drive handle.
> Clamp down on the IDE command execution table to remove CD_OK permissions
> from commands that are not and have never been ATAPI commands.
>
> For ATAPI command validity, please see:
> - ATA4 Section 6.5 ("PACKET Command feature set")
> - ATA8/ACS Section 4.3 ("The PACKET feature set")
> - ACS3 Section 4.3 ("The PACKET feature set")
>
> ACS3 has a historical command validity table in Table B.4
> ("Historical Command Assignments") that can be referenced to find when
> a command was introduced, deprecated, obsoleted, etc.
>
> The only reference for ATAPI command validity is by checking that
> version's PACKET feature set section.
>
> ATAPI was introduced by T13 into ATA4, all commands retired prior to ATA4
> therefore are assumed to have never been ATAPI commands.
>
> Mandatory commands, as listed in ATA8-ACS3, are:
>
> - DEVICE RESET
> - EXECUTE DEVICE DIAGNOSTIC
> - IDENTIFY DEVICE
> - IDENTIFY PACKET DEVICE
> - NOP
> - PACKET
> - READ SECTOR(S)
> - SET FEATURES
>
> Optional commands as listed in ATA8-ACS3, are:
>
> - FLUSH CACHE
> - READ LOG DMA EXT
> - READ LOG EXT
> - WRITE LOG DMA EXT
> - WRITE LOG EXT
>
> All other commands are illegal to send to an ATAPI device and should
> be rejected by the device.
We could perhaps argue about "should be rejected by the device", but I
think the weaker "a device is free to reject it" still suffices to
support your patch.
> CD_OK removal justifications:
>
> 0x06 WIN_DSM Defined in ACS2. Not valid for ATAPI.
> 0x21 WIN_READ_ONCE Retired in ATA5. Not ATAPI in ATA4.
> 0x94 WIN_STANDBYNOW2 Retired in ATA4. Did not coexist with ATAPI.
> 0x95 WIN_IDLEIMMEDIATE2 Retired in ATA4. Did not coexist with ATAPI.
> 0x96 WIN_STANDBY2 Retired in ATA4. Did not coexist with ATAPI.
> 0x97 WIN_SETIDLE2 Retired in ATA4. Did not coexist with ATAPI.
> 0x98 WIN_CHECKPOWERMODE2 Retired in ATA4. Did not coexist with ATAPI.
> 0x99 WIN_SLEEPNOW2 Retired in ATA4. Did not coexist with ATAPI.
> 0xE0 WIN_STANDBYNOW1 Not part of ATAPI in ATA4, ACS or ACS3.
> 0xE1 WIN_IDLEIMMDIATE Not part of ATAPI in ATA4, ACS or ACS3.
> 0xE2 WIN_STANDBY Not part of ATAPI in ATA4, ACS or ACS3.
> 0xE3 WIN_SETIDLE1 Not part of ATAPI in ATA4, ACS or ACS3.
> 0xE4 WIN_CHECKPOWERMODE1 Not part of ATAPI in ATA4, ACS or ACS3.
> 0xE5 WIN_SLEEPNOW1 Not part of ATAPI in ATA4, ACS or ACS3.
> 0xF8 WIN_READ_NATIVE_MAX Obsoleted in ACS3. Not ATAPI in ATA4 or ACS.
Actual patch matches this list.
> This patch fixes a divide by zero fault that can be caused by sending
> the WIN_READ_NATIVE_MAX command to an empty ATAPI drive, which causes
> it to attempt to use zeroed CHS values to perform sector arithmetic.
>
> Reported-by: Qinghao Tang <luodalon...@gmail.com>
> Signed-off-by: John Snow <js...@redhat.com>
I appreciate you going to the root of the problem instead of merely
fixing the narrow bug.
Could a similar argument be made for dropping CFA_OK from some commands?
Do we still need this conditional in cmd_read_native_max()?
/* Refuse if no sectors are addressable (e.g. medium not inserted) */
if (s->nb_sectors == 0) {
ide_abort_command(s);
return true;
}
Why does it fail at guarding the CHS use from empty ATAPI drives before
your patch?
