Le samedi 29 septembre 2012 à 13:10 +0000, FFADO a écrit :
#362: ffado-dbus-server crashes when launching ffado-mixer
----------------------------+-----------------------------------------------
   Reporter:  andreeecz     |       Owner:  arnonym          
       Type:  bug           |      Status:  new              
   Priority:  major         |   Milestone:  FFADO 2.x        
  Component:  ffado-mixer   |     Version:  FFADO SVN (trunk)
 Resolution:                |    Keywords:                   
Device_name:  SaffirePro40  |  
----------------------------+-----------------------------------------------
Comment (by jwoithe):

 Your analysis of the dump looks reasonable.  Phil posted this followup to
 the ffado-devel mailing list earlier today:

 > Strange, since getnickname (at least for Focusrite) returns no more than
 16 characters; and ffado-dbus-server effectively returns a 16 characters
 nickname (your previous report). Which way did you use to change the
 nickname at first ?

 It's hard to determine quite where the problem is occurring.  It certainly
 appears that a buffer containing the nickname is not being zero-terminated
 correctly somewhere; but where that might be is far from clear.

 I am far from familiar with the way the DICE devices work.  A casual
 glance through the source code suggests that the nickname string as
 returned by the device may not in fact be explicitly NULL terminated
 before being passed onto other areas of FFADO.  If this is the case, then
 trouble will occur if the device itself does not return a NULL-terminated
 nickname.

 EAP::readRegBlock() clearly just reads "length" bytes values into the
 "data" buffer.  In SaffirePro40::getNickname(), this is essentially all
 that's being done.  Therefore if the Nickname doesn't contain a
 terminating NULL byte, the memory pointed to by "name" in
 SaffirePro40::getNickname() won't be either, unless by chance the "name"
 array included a zero somewhere.  When the result is converted to a
 std::string, there's no telling how long the resulting string would be.

 Working on the assumption that the device has space for a nickname up to
 16 bytes in length, I've pushed r2237 into trunk.  This does two things.
 First and foremost it ensures correct NULL termination of the nickname
 returned by SaffirePro40::getNickname().  Secondly, it ensures that at
 most 16 characters are sent to the device when setting the nickname via
 SaffirePro40::setNickname().  If my assumptions are correct and this
 proves to fix the problem as documented by the above dump, the changes
 will need to be propagated to the other Saffire sub-drivers as well.

Hi Jonathan,

I fully agree: whatever it fixes the bug or not, the way it was used up to now is not safe. After answering the ticket (in the wrong way, as usual !), I was wondering about the setnickname function and the apparent possibility for writing something larger than the size devoted for this in the rom; I would ask your opinion on this before modifying. I didn't think about this everlasting (about 30 years for me) problem of non-zero terminated strings :(

I will propagate the changes for Pro14 and Pro24 and have a look to generic Dice EAP. Just a point: don't you think it would be clearer to introduce a
    #define SAFFIRE_PRO40_NICKNAME_LENGTH 16
and possibly a
    #define SAFFIRE_PRO40_NICKNAME_REG 0x44

in the .h file, so as it will be simpler to modify something in the future ?

Best regards,

Phil

--
Philippe Carriere <la-page-web-of-phil.contact@orange.fr>