[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <51CCED3F.3080303@nicta.com.au>
Date: Fri, 28 Jun 2013 11:56:15 +1000
From: Matthew Fernandez <matthew.fernandez@...ta.com.au>
To: <musl@...ts.openwall.com>
CC: Rich Felker <dalias@...ifal.cx>
Subject: Re: Use of size_t and ssize_t in mseek
On 28/06/13 11:48, Rich Felker wrote:
> On Fri, Jun 28, 2013 at 11:34:23AM +1000, Matthew Fernandez wrote:
>>> On Fri, Jun 28, 2013 at 10:49:41AM +1000, Matthew Fernandez wrote:
>> I suppose this is an option, but this just isolates the problem to
>> 64-bit systems.
>
> Well, on a 32-bit system (albeit not the one stock musl, as intended
> to run on Linux, implements) it's possible to have an object larger
> than 2^31 bytes. It's never possible to have an object larger than
> 2^63 bytes on any system.
Yes, however as I said, nothing prevents me type casting to get around
this. Either way, this is a more or less a philosophical argument at
this point.
>> On x86_64 I would still be able to naïvely call fmemopen
>> with SIZE_MAX and end up being unable to fseek. Not being able to
>
> Ah, I missed the fact that you were just passing SIZE_MAX because you
> don't know the size; I thought you were passing some large value equal
> to the actual size. In general, passing SIZE_MAX for unknown sizes is
> dangerous, even if objects larger than SSIZE_MAX are supported,
> because the implementation of the function might work like:
>
> char *end = base + size;
> while (pos < end) ...
>
> In this case, merely adding base+size invoked UB if size was larger
> than the size of the object; in practice, what happens is that the
> pointer wraps, so end is less than base, and then the loop never runs.
>
> While I'm not opposed to making changes to reject or to support
> objects larger than SSIZE_MAX, I am pretty much opposed to making any
> attempt to accept a length of SIZE_MAX as "unknown/unlimited size".
> This kind of usage, as described above, is flawed and error-prone. If
> you want to replace it with something at least halfway portable,
> instead of passing SIZE_MAX, pass SIZE_MAX-(size_t)base or some
> variant of this concept. This will at least avoid the overflow; it's
> the trick musl uses internally for implementing sprintf in terms of
> snprintf. However, I would really recommend trying to find some better
> approach to obtain and pass the correct size to fmemopen. There is no
> contract that fmemopen is not allowed to read arbitrary parts of the
> buffer passed to it, so passing an incorrect length and expecting it
> to work is depending on the current implementation.
>
> By the way, note that the issue here is not that the size argument is
> greater than SSIZE_MAX. Even if you passed a shorter "huge object"
> length, like SSIZE_MAX or SSIZE_MAX/2, it could still cause overflows
> if the base of the object happened to begin at a high address. The
> issue is merely that the size is not the correct size of the object.
I completely agree with all of this.
>>> Alternatively, I could adjust the arithmetic to just avoid working
>>> with signed values, and perhaps make it more obvious what it's doing
>>> in the process.
>>
>> I would also be happy with this solution. The code in mseek could
>> definitely be clearer. Not that I don't enjoy switch statements written
>> as offsets into stack structs and reverse jumps ;)
>
> Yes, I think this is probably the best solution, even if it makes the
> function a few bytes larger. The code should be more clear.
Thanks, Rich. I appreciate you taking the time to consider this issue.
Apologies that it seems to have steamrolled into all the ways of
constructing invalid objects and possibly bored everyone else on this
list :)
________________________________
The information in this e-mail may be confidential and subject to legal professional privilege and/or copyright. National ICT Australia Limited accepts no liability for any damage caused by this email or its attachments.
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
