[Patch] Segfault on unaligned lseek() on /dev/sdX (was: [ITP] ddrescue 1.3)

[Pedro Alves]

Christian Franke escreveu:
> Pedro Alves wrote:
>> Christopher Faylor escreveu:
>>> On Fri, May 18, 2007 at 09:02:15PM +0200, Christian Franke wrote:
>>>> Hi,
>>>>
>>>> Cygwin 1.5.24-2 segfaults on unaligned lseek() on raw block devices 
>>>> with sector size >512 bytes.
>>>>
>>>> Testcases:
>>>> $ dd skip=1000 bs=2047 if=/dev/scd0 of=/dev/null
>>>>
>>>> $ ddrescue -c 1 /dev/scd0 file.iso
>>>>
>>>>
>>>> This is due to a fixed 512 byte buffer in fhandler_dev_floppy::lseek().
>>>> It is still present in HEAD revision.
>>>>
>>>> The attached patch should fix. It should work for any sector size.
>>>> (Smoke-)tested with 1.5.24-2 (too busy to test with current CVS, 
>>>> sorry).
>>>>
>>>> 2007-05-18  Christian Franke <franke@computer.org>
>>>>
>>>>     * fhandler_floppy.cc (fhandler_dev_floppy::lseek): Fixed 
>>>> segfault on
>>>>     unaligned seek due to fixed size buffer.
>>>>
>>>
>>> It seems like this could be done without the heavyweight use of malloc,
>>> like use an automatic array of length 512 + 4 and calculate an aligned
>>> address from that.
>>>
>>
>> Or use alloca instead?
>>
>> -  char buf[512];
>> +  char *buf = (char *) alloca (512);
>>
> 
> Yes, thanks.
> 
> Makes the new patch really simple, see attachment.
> 

Actually, I also thought you were talking about memory alignment,
and since alloca has the same alignment guaranties as malloc, it would
avoid doing the + 4 + alignment calc.

I'm just looking at fhandler_floopy.cc for the first time,
but, isn't there the possibility that bytes_left can be a bit too big
for alloca?  It looks like that the raw_read call is there to
advance the position by the needed amount (moving back is forbidden
a bit above).  Perhaps it would be better to read in a loop with
read amount limited by the size of the buffer:

while more bytes
do
     read minimum of bytes left or size of buffer
     if couldn't read, bail out. (oooops internal state broken now).
done

Cheers,
Pedro Alves