Dangerous usage of read() in ReadBlob()

Post any defects you find in the released or beta versions of the ImageMagick software here. Include the ImageMagick version, OS, and any command-line required to reproduce the problem. Got a patch for a bug? Post it here.
Post Reply
FrBrGeorge
Posts: 2
Joined: 2013-04-25T04:19:21-07:00
Authentication code: 6789

Dangerous usage of read() in ReadBlob()

Post by FrBrGeorge » 2013-04-25T04:46:16-07:00

This code in magick/blob.c can legally return any count as a result:

Code: Select all

2789     case StandardStream:
2790     {
2791       count=read(fileno(image->blob->file_info.file),q,length);
2792       break;
2793     }
According to manual:
It is not an error if this number is smaller than the number of bytes requested
This command line performs on GNU/Linux successfully for lower N, always fails for higher N and behaves randomly on N between 50 and 200, depending on system speed. See two convert calls that work with same data, but gain different results:

Code: Select all

$ export N=150; head -c $(($N*$N*3)) /dev/urandom | tee img | convert -depth 8 -size ${N}x${N} rgb:- o.raw
convert: unexpected end-of-file `-': No such file or directory @ error/rgb.c/ReadRGBImage/231.
$ cat img | convert -depth 8 -size ${N}x${N} rgb:- o.raw
$ ls -l o.raw
-rw-r--r-- 1 george george 67500 Apr 25 15:43 o.raw
Actually this can happen independent of I/O type, only it must be slow enough. I see no solution but using fread() here.

User avatar
magick
Site Admin
Posts: 11064
Joined: 2003-05-31T11:32:55-07:00

Re: Dangerous usage of read() in ReadBlob()

Post by magick » 2013-04-25T05:09:44-07:00

We can't use fread because its buffered and that causes other sorts of problems, but we can put the read in a loop until the read completes. We'll get a patch in ImageMagick 6.8.5-1 Beta within a few days.

FrBrGeorge
Posts: 2
Joined: 2013-04-25T04:19:21-07:00
Authentication code: 6789

Re: Dangerous usage of read() in ReadBlob()

Post by FrBrGeorge » 2013-04-29T01:20:28-07:00

magick wrote:we can put the read in a loop until the read completes
Read loop, yes. By the way, I've reverted that very part of code, but got the same error even on file I/O (on heavy loaded system). Probably, more thorough read() check is needed.

UPD Alert clear, that was my fault. Read loop seems to be enough.

Post Reply