Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sat, 29 Jun 2013 11:48:59 -0400
From: Rich Felker <dalias@...ifal.cx>
To: musl@...ts.openwall.com
Subject: Re: Use of size_t and ssize_t in mseek

On Sat, Jun 29, 2013 at 04:56:42PM +0200, Jens Gustedt wrote:
> Am Samstag, den 29.06.2013, 10:17 -0400 schrieb Rich Felker:
> > Just that this is one of a multitude of places that such a check could
> > be made, and I question the value of doing it in one place but not
> > others. Examples include snprintf, strnlen, memchr, and basically any
> > interface that takes a size_t representing the size of an already
> > existing object. I'm against adding checks to all these places since
> > it adds bloat and potentially hurts performance and for most of them
> > there's nothing they could do except crash if the check failed. So
> > what I'm questioning is the value of adding such a check to the one
> > interface you ran into trouble with, when there are plenty more widely
> > used functions that won't be checked; this inconsistency does not make
> > sense to me. I'd like to hear what others think, though.
> 
> I think C11 has indentified this sort of specification problems and
> therefore introduces rsize_t and RSIZE_MAX in the not-loved-by-many
> appendix K "bounds-checking intefaces". Interfaces that are specified
> with this type are required to check that the value isn't too large
> for any object.
> 
> If you'd want to go that road (of checking for the size) I'd suggest
> that you'd define and use RSIZE_MAX for such a thing, and maybe even
> change the interfaces to use rsize_t. Since this is only a typedef
> such a change should still be compatible with size_t as in the current
> and future standard(s), and it would clearly mark the intent of bounds
> checking.

The type in the interface can't be changed. It's required by POSIX
(and for many interfaces, required by C). However if rsize_t were
defined as size_t and RSIZE_MAX were SIZE_MAX/2, it would at least
partly convey the intent.

It's the intent in musl that objects larger than the max value of the
signed type corresponding to size_t simply never exist, so they don't
have to be considered.

Matthew was only able to get such a large size_t by 'guessing' - his
bootloader (he ported musl to run on non-Linux) gave him a pointer to
an object of unknown size, and he passed SIZE_MAX to fmemopen to
attempt to read it as a file. This usage (giving size larger than the
actual size of an object) is of course undefined behavior and not
supported, but it's conceivable that his boot loader could have given
him a valid size larger than SIZE_MAX/2 due to the bootloader and musl
disagreeing on whether such objects can exist.

The reason I question just adding a check here is that he could just
as easily have happened to pass this object to snprintf, strnlen,
memchr, etc., so from a standpoint of consistency, it doesn't make
sense to me to add a check in one place but not others (and some of
the others can't even report errors).

My leaning at this point is just to document well that objects larger
than SIZE_MAX/2 are assumed not to exist. I've already fixed the last
two ways (mmap and shmget) they could be created with musl, so it
seems to me the only way they can arise now is when you've done
something like what Matthew has done and ported musl to a different
underlying system, in which case this issue is one of many issues that
should be considered as part of the port.

Rich

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.