Re: Bit of a rant, bug in PNM

becka@rz.uni-duesseldorf.de
Wed, 9 Dec 1998 01:30:03 +0100 (MET)

Hi !

> 2. The PNM backend seems to be a quick hack,

Well - it was kind of the "reference implementation", that was used in the
early days to have something to talk about.

If it is broken, it is probably largely my fault.

> I hoped to use it to test my install of SANE,

That's actually what it was meant for. Testing and reference implementation.

> but instead I have spent a good hour finding that the
> problem initially thought to be the result of my mistakes is a BUG
> in the PNM backend!

Ouch. So we got to squish it.

> The major bug in the PNM device is that it seems to think the
> maximum length parameter to read_sane() is just a suggestion!

Oh - WHAT ?

In what branch does that happen ?

The sane-read request branches, if the "modifying" options are set.
If neither grayify nor three-pass-emulation are set, all should be well:

else
/* Suck in as much of the file as possible, since it's already in the
correct format. */
len = fread (data, 1, max_length, infile);

Now let's have a look at the other branch ...

Ah - I see it. There is a buffer allocated and blindly filled with data
which is the given out again. However due to the dynamic sizing of the
buffer, this can overflow the frontend buffers, in case the frontend
uses different sized read requests.

O.K. - nice someone has found that finally, and even nicer, that I am sure
I didn't write that code. So who's the culprit ? David ? ;-)

O.K. - how to fix that ...

as a quickfix, I assume replacing the ;p < rgbend; in the for loops with
;p < rgbend && max_length--; looks tempting, but we would have to take
care for a _biiig_ overflow buffer.

Another (a bit better) one would be to disable the if (rgbbuf == 0 ||
rgblength < 3 * max_length) condition, or the yet best I see is to do

- len = fread (q, 1, rgblength - rgbleftover[0], infile);
+ len = fread (q, 1, 3*max_length - rgbleftover[0], infile);

Yeah - that should fix it.

Could you test/confirm that ? Otherwise we'd need to look for a "real" fix.

> If the PNM device has more data to send, it just writes past the end
> of the buffer,

Hey - it's not that bad. Just some overly clever code, which misbehaves
when used in a somewhat unusual way. Who didn't make such a mistake once
upon a time ?

> I can't tell if this bug exists for any other SANE devices.

It shouldn't. It looks like it has been introduced with the test festures.

Can you confirm, that my suspicion is correct ?

I.e. it only happens, when either grayify or 3-pass-emulation is on, and the
frontend uses varying sizes for the read()s - right ?

Does my suggested fix kill the bug ?

CU, ANdy

-- 
= Andreas Beck                    |  Email :  <andreas.beck@ggi-project.org> =

--
Source code, list archive, and docs: http://www.mostang.com/sane/
To unsubscribe: echo unsubscribe sane-devel | mail majordomo@mostang.com