LIBATA SCSI command validation changed in 2.6.24
commit 607126c2a21cd6e9bb807fdd415c1a992f7b9009 changed command validation to allow short commands in 16-byte CDBs, but it also made checking more strict. Before the change, a 10-byte SG_IO command could have its length set to 9 and still work. Now it fails. Not sure if this is a bug, but it has caused at least one application to fail that used to work (qpxtool.)
Quoted 7 lines · Chuck Ebbert
commit
607126c2a21cd6e9bb807fdd415c1a992f7b9009changed command validation to allow short commands in 16-byte CDBs, but it also made checking more strict. Before the change, a 10-byteSG_IOcommand could have its length set to 9 and still work. Now it fails. Not sure if this is a bug, but it has caused at least one application to fail that used to work (qpxtool.)
Can you get us an example CDB? Its unclear if the hexdump in the bug report is a returned mode page or the CDB or what...?
Jeff
Quoted 9 lines · Jeff Garzik
Quoted 8 lines · Chuck Ebbert
commit
607126c2a21cd6e9bb807fdd415c1a992f7b9009changed command validation
to allow short commands in 16-byte CDBs, but it also made checking more strict. Before the change, a 10-byteSG_IOcommand could have its length set
to 9 and still work. Now it fails. Not sure if this is a bug, but it has caused at least one application to fail that used to work (qpxtool.)
Quoted 5 lines
Quoted 1 line
Can you get us an example CDB? Its unclear if the hexdump in the bug report is a returned mode page or the CDB or what...?
Not easily, but the maintainer of that program forced the length of the MODE_SENSE(10) command to 10 and that command started working.
By looking at the source I could tell that it was setting the command length to (1 + the index of the last byte written to the CDB) and only wrote up to offset 8 when building the command, so it must have been sending the command with a length of 9. (It zeroed the whole CDB first and only wrote what it needed to.)
(And it used the C++ operator [] to build the command, that was fun to trace...)
Quoted 18 lines · Chuck Ebbert
Quoted 9 lines · Jeff Garzik
Quoted 8 lines · Chuck Ebbert
commit
607126c2a21cd6e9bb807fdd415c1a992f7b9009changed command validation
to allow short commands in 16-byte CDBs, but it also made checking more strict. Before the change, a 10-byteSG_IOcommand could have its length set
to 9 and still work. Now it fails. Not sure if this is a bug, but it has caused at least one application to fail that used to work (qpxtool.)Quoted 4 lines
Quoted 1 line
Can you get us an example CDB? Its unclear if the hexdump in the bug report is a returned mode page or the CDB or what...?
Not easily, but the maintainer of that program forced the length of the
MODE_SENSE(10)command to 10 and that command started working.
Quoted 8 lines
By looking at the source I could tell that it was setting the command length to (1 + the index of the last byte written to the CDB) and only wrote up to offset 8 when building the command, so it must have been sending the command with a length of 9. (It zeroed the whole CDB first and only wrote what it needed to.)
(And it used the C++ operator [] to build the command, that was fun to trace...)
Even if allocation length is present in the CDB, the CDB may be missing important information that is required to process the command. So it may have caught a bug in the program... depending on the CDB.
Jeff
Quoted 26 lines · Jeff Garzik
Quoted 25 lines · Chuck Ebbert
Quoted 10 lines · Jeff Garzik
Quoted 9 lines · Chuck Ebbert
commit
607126c2a21cd6e9bb807fdd415c1a992f7b9009changed command validation
to allow short commands in 16-byte CDBs, but it also made checking more strict. Before the change, a 10-byteSG_IOcommand could have its length set
to 9 and still work. Now it fails. Not sure if this is a bug, but it has
caused at least one application to fail that used to work (qpxtool.)Quoted 4 lines
Quoted 1 line
Can you get us an example CDB? Its unclear if the hexdump in the bug report is a returned mode page or the CDB or what...?
Not easily, but the maintainer of that program forced the length of the
MODE_SENSE(10)command to 10 and that command started working.By looking at the source I could tell that it was setting the command length to (1 + the index of the last byte written to the CDB) and only wrote up to offset 8 when building the command, so it must have been sending the command with a length of 9. (It zeroed the whole CDB first and only wrote what it needed to.)
Quoted 7 lines
Quoted 2 lines
(And it used the C++ operator [] to build the command, that was fun to trace...)
Even if allocation length is present in the CDB, the CDB may be missing important information that is required to process the command. So it may have caught a bug in the program... depending on the CDB.
Yeah, the change is probably good. We should have been validating that the length was at least as long the expected length all along. But some programs are going to break because of this...