Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <51CBBC8F.5050301@nicta.com.au>
Date: Thu, 27 Jun 2013 14:16: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 27/06/13 14:10, Rich Felker wrote:
> On Thu, Jun 27, 2013 at 01:52:01PM +1000, Matthew Fernandez wrote:
>> Hi all,
>>
>> My question refers to the latest git commit at time of writing,
>> b17c75a4d539d7ec5b81cc7ce7ce6b065a87e7a6. My issue was encountered on
>> ARM, but it applies to most 32-bit platforms.
>>
>> The function mseek() accesses a size_t variable, c->size, and casts this
>> to a ssize_t. I know there aren't strong standards on what to expect
>> from ssize_t, but the Musl C constants SIZE_MAX (== UINT32_MAX) and
>> SSIZE_MAX (== LONG_MAX) seem to imply that you would be wise to assume
>> ssize_t is signed and the same width as size_t.
>>
>> As a result, the cast I mentioned produces some unexpected results when
>> operating on a file of size greater than SSIZE_MAX. In my case I had an
>> in-memory file of size SIZE_MAX and was surprised to find I couldn't
>> fseek this file.
>>
>> Is the code in mseek() correct? If so, I would recommend failing
>> fmemopen() when the requested size is greater than SSIZE_MAX. OTOH
>> perhaps I'm misunderstanding something more subtle here. If so, please
>> correct me.
>
> The argument to a function like fmemopen that takes a size_t
> representing the size of an object must actually correspond to the
> size of an object. It's always been the intent in musl that objects
> larger than SSIZE_MAX (which is also PTRDIFF_MAX) not be able to
> exist, since they result in overflow (and undefined behavior) if you
> subtract pointers within them, and implementation-defined behavior if
> you pass their sizes to functions like read, write, etc. The malloc
> implementation in musl explicitly rejects size arguments greater than
> SSIZE_MAX, with the intent of ensuring that such objects don't exist,
> so as far as I can tell, the only remaining "backdoor" to get such an
> object is mmap.
>
> Unless there are strong objections, I'd like to make mmap reject such
> sizes too. On 32-bit archs, the maximum vm size is already bounded by
> 3gb, and once you get the program and a few shared libraries loaded
> too, it's a good bit less, so it would be hard to map more than 2.5gb
> or so anyway.
>
> Once all the doors to obtaining objects of size greater than SSIZE_MAX
> are closed, the issue goes away entirely. If you pass a size greater
> than SSIZE_MAX to fmemopen or similar functions, it can't actually be
> the correct size of the object (since no object that large exists),
> and thus you must be invoking UB by passing an incorrect size, so it
> doesn't matter what the function does.
>
> Does this all make sense and sound reasonable?

Perfectly reasonable to make it UB (and I had assumed it was so
already). It just seemed to me that it would be more user-friendly to
bounds check the size parameter in fmemopen. Is there a reason not to do
this?

________________________________

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.