potential bug in atomicio?

Reimar Döffinger Reimar.Doeffinger at gmx.de
Wed Jul 18 00:01:25 AWST 2018


On 17 July 2018 14:34:16 CEST, Matt Johnston <matt at ucc.asn.au> wrote:
>On Wed, Jul 11, 2018 at 05:26:17PM -0300, Daniel Gutson wrote:
>> Hi,
>> 
>>    considering this:
>> 
>>
>https://github.com/mkj/dropbear/blob/d740dc548924f2faf0934e5f9a4b83d2b5d6902d/atomicio.c#L55
>...
>> What if res is negative less than -1, for example -2 ? Shouldn't be a
>check
>> there that res is > 0 ?
>
>Hi Daniel,
>
>atomicio() is always to be called with f == read or f == write which
>both
>won't return a value <-1. Have you found a platform that doesn't do
>that?
>I'd probably write it as "ret < 0" myself but it was a
>straight copy from OpenSSH's tree.

I'd also caution against checking against < 0 "just because". 
Not applying to this code specifically, but for example the documentation of open() says it on error returns -1 but it doesn't say -2 is an invalid return value. For example WinCE DID in fact return negative file handles.
The implementation very much LOOKS like it was carefully written to allow reading more than 2GB on 32 bit systems. 
Then again, looks can be deceiving, and a liberal use of asserts might have been advisable, but better not to change imported code.

Regards,
Reimar


More information about the Dropbear mailing list