[k3b] [Bug 382941] Segfault from getSupportedWriteSpeedsVia2A

[Thomas Schmitt]

https://bugs.kde.org/show_bug.cgi?id=382941

--- Comment #16 from Thomas Schmitt <scdbackup at gmx.net> ---
Hi,

Mark's workaround justifies itself by mere arithmetics. The new "if"
prevents computation from being performed if it would yield 0 or less.

The remaining question is why this computation is made and whether
it is correct.
And if it is correct, then why can it become negative at all ?

So it would be interesting to see the SCSI transaction around MODE SENSE
for page 2A from the drive which caused the segfault:

  cdrskin -V dev=/dev/sr0 2>&1 >/tmp/cdrskin_scsi_log

This yields for me with a CD-RW medium in /tmp/cdrskin_scsi_log

  MODE SENSE
  5a 00 2a 00 00 00 00 00 4c 00 
  From drive: 76b
  00 4a 21 00 00 00 00 00 2a 42 3f 37 f1 77 29 23 1b 90 01 00
  0f e0 10 8a 00 10 06 e4 06 e4 00 01 00 00 00 00 06 e4 00 01
  00 00 06 e4 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 

Please post the whole file. There will be more than one MODE SENSE in it.

------------------------------------------------------------------------
Now let's see what K3B is doing this time what it should leave to burn
programs. :))

getSupportedWriteSpeedsVia2A() requests from the drive Mode Page 2A
"MM Capabilities and Mechanical Status Page". It is obsoleted since MMC-4,
but afaik still supported by all drives. (More modern is command
ACh GET PERFORMANCE in K3b::Device::Device::getPerformance().)

If a Mode Page is fetched by command 5Ah MODE SENSE(10), then it is preceeded
by a header struct of 8 bytes (SPC-3, table 240). This header tells in its
bytes 0 to 2 the number of bytes minus 2 which can at most be replied for
the given mode page.

From
  mm_cap_page_2A* mm = (mm_cap_page_2A*)&data[8];
i conclude that "data" is the raw reply from the drive which now gets
overlaid by a C++ struct. (So i can only guess the mapping of specs to code,
from here on.)

Mode page 2A is described in MMC-3, 6.3.11.
In page 2A the number of speed descriptors is recorded in bytes 30 an 31
of the replied page. Their size is four bytes each. So in above example
from cdrskin there are
  0x4a + 2 - 8 - 32 = 36
descriptor bytes.

I never encountered drives which tell descriptor bytes rather than
descriptor count. Can this be a rumor caused by other problems ?

There is some contradiction in
        if( data.size() > 32 ) {
            // we have descriptors
versus
                numDesc = (data.size() - 32 - 8) / 4;

The "if" assumes that data.size() is the net mode page size.
The "numDesc" computation assumes that it is the combined size of header
and page.


Looking at k3bdevice_mmc.cpp line 676
  bool K3b::Device::Device::modeSense( UByteArray& pageData, int page ) const
i get the impression that it is a generic function of the array and
reflects its size as recorded by

  pageLen = 8;
  if( cmd.transport( TR_DIR_READ, header, 8 ) == 0 )
      pageLen = from2Byte( header ) + 2;
  ...
  pageData.resize( pageLen );

So this is wrong in getSupportedWriteSpeedsVia2A() :

        if( data.size() > 32 ) {
            // we have descriptors
            unsigned int numDesc = from2Byte( mm->num_wr_speed_des );

It should be

        if( data.size() > 8 + 32 ) {
            // we have descriptors
            unsigned int numDesc = from2Byte( mm->num_wr_speed_des );

With the wrong comparison and if there are no descriptors, the value numDesc
might be taken from bytes which were not allocated by modeSense().
(Doesn't the fancy UByteArray have an overflow protection ?)

I now ponder whether this might be the reason for the rumor:
  // Some CDs writer returns the number of bytes that contain
  // the descriptors rather than the number of descriptors


Have a nice day :)

Thomas

-- 
You are receiving this mail because:
You are the assignee for the bug.