Zero-length allocation with posix_memalign()

[Fabian Keil-2]

I recently submitted a patch to the vlc developers that prevents
a crash on FreeBSD 8.0 by not calling posix_memalign() with a
size argument of zero.

A simplified test case would be:

#include <stdlib.h>
int main(int argc, char **argv) {
        void *ptr;
        posix_memalign(&ptr, 16, 0);
        return (0);
}

which triggers:
Assertion failed: (size != 0), function arena_malloc, file /usr/src/lib/libc/stdlib/malloc.c, line 3349.

Rémi Denis-Courmont, one of the vlc developers, pointed out
that passing a zero size to posix_memalign() should actually
work, though:

| In principle, while useless, there is no reason why allocating an empty
| picture should not be possible. posix_memalign() does support zero-length
| allocation anyway:
| http://www.opengroup.org/onlinepubs/9699919799/functions/posix_memalign.html
| | If the size of the space requested is 0, the behavior is
| | implementation-defined; the value returned in memptr shall be either a
| | null pointer or a unique pointer.
http://mailman.videolan.org/pipermail/vlc-devel/2009-July/062299.html

I get the impression that this deviation from the standard could be
easily fixed with something similar to the following, which is mostly
copy and pasted from malloc():

index 5404798..a078d07 100644
--- a/malloc.c
+++ b/malloc.c
@@ -5303,6 +5303,15 @@ posix_memalign(void **memptr, size_t alignment, size_t size)
  int ret;
  void *result;
 
+ if (size == 0) {
+ if (opt_sysv == false)
+ size = 1;
+ else {
+ ret = 0;
+ *memptr = result = NULL;
+ goto RETURN;
+ }
+ }
  if (malloc_init())
  result = NULL;
  else {

I assume the "goto RETURN" isn't entirely compliant either as
it skips the alignment check, but so does the malloc_init()
failure branch.

Fabian

[Jason Evans-2]

Fabian Keil wrote:

> Rémi Denis-Courmont, one of the vlc developers, pointed out
> that passing a zero size to posix_memalign() should actually
> work, though:
>
> | In principle, while useless, there is no reason why allocating an empty
> | picture should not be possible. posix_memalign() does support zero-length
> | allocation anyway:
> | http://www.opengroup.org/onlinepubs/9699919799/functions/posix_memalign.html
> | | If the size of the space requested is 0, the behavior is
> | | implementation-defined; the value returned in memptr shall be either a
> | | null pointer or a unique pointer.

Standards: So many to choose from.  This behavior for posix_memalign was
only defined as of the 2008 standard (see the Issue 7 notes for
posix_memalign):

https://www.opengroup.org/austin/interps/uploads/40/14543/AI-152.txt

Such requirements are unfortunate, because they induce a performance
penalty for every call, just so that programs can avoid proper handling
of edge cases in the rare situations for which such edge cases are a
real possibility.

I will add the pessimization to posix_memalign once the 8.0 freeze is
over.  It will be quite some time before this behavior becomes
ubiquitous, so in the meanwhile it's probably a good idea to modify vlc
to avoid such allocation requests.

Thanks,
Jason
_______________________________________________
freebsd-hackers@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@..."

[Paul B Mahol]

On 7/5/09, Fabian Keil <freebsd-listen@...> wrote:

> I recently submitted a patch to the vlc developers that prevents
> a crash on FreeBSD 8.0 by not calling posix_memalign() with a
> size argument of zero.
>
> A simplified test case would be:
>
> #include <stdlib.h>
> int main(int argc, char **argv) {
> void *ptr;
> posix_memalign(&ptr, 16, 0);
> return (0);
> }
>
> which triggers:
> Assertion failed: (size != 0), function arena_malloc, file
> /usr/src/lib/libc/stdlib/malloc.c, line 3349.

Actually that assertion is triggered only if MALLOC_PRODUCTION is undefined.
(when it is undefined it considerably slows thing down)
'a' flag for malloc.conf looks broken for me ....

>
> Remi Denis-Courmont, one of the vlc developers, pointed out
> that passing a zero size to posix_memalign() should actually
> work, though:
>
> | In principle, while useless, there is no reason why allocating an empty
> | picture should not be possible. posix_memalign() does support zero-length
> | allocation anyway:
> |
> http://www.opengroup.org/onlinepubs/9699919799/functions/posix_memalign.html
> | | If the size of the space requested is 0, the behavior is
> | | implementation-defined; the value returned in memptr shall be either a
> | | null pointer or a unique pointer.
> http://mailman.videolan.org/pipermail/vlc-devel/2009-July/062299.html
>
> I get the impression that this deviation from the standard could be
> easily fixed with something similar to the following, which is mostly
> copy and pasted from malloc():
>
> index 5404798..a078d07 100644
> --- a/malloc.c
> +++ b/malloc.c
> @@ -5303,6 +5303,15 @@ posix_memalign(void **memptr, size_t alignment,
> size_t size)
>   int ret;
>   void *result;
>
> + if (size == 0) {
> + if (opt_sysv == false)
> + size = 1;
> + else {
> + ret = 0;
> + *memptr = result = NULL;
> + goto RETURN;
> + }
> + }
>   if (malloc_init())
>   result = NULL;
>   else {
>
> I assume the "goto RETURN" isn't entirely compliant either as
> it skips the alignment check, but so does the malloc_init()
> failure branch.
>
> Fabian
>

--
Paul
_______________________________________________
freebsd-hackers@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@..."

[Fabian Keil-2]

Jason Evans <jasone@...> wrote:

> Fabian Keil wrote:
> > Rémi Denis-Courmont, one of the vlc developers, pointed out
> > that passing a zero size to posix_memalign() should actually
> > work, though:
> >
> > | In principle, while useless, there is no reason why allocating an empty
> > | picture should not be possible. posix_memalign() does support zero-length
> > | allocation anyway:
> > | http://www.opengroup.org/onlinepubs/9699919799/functions/posix_memalign.html
> > | | If the size of the space requested is 0, the behavior is
> > | | implementation-defined; the value returned in memptr shall be either a
> > | | null pointer or a unique pointer.
>
> Standards: So many to choose from.  This behavior for posix_memalign was
> only defined as of the 2008 standard (see the Issue 7 notes for
> posix_memalign):
>
> https://www.opengroup.org/austin/interps/uploads/40/14543/AI-152.txt
>
> Such requirements are unfortunate, because they induce a performance
> penalty for every call, just so that programs can avoid proper handling
> of edge cases in the rare situations for which such edge cases are a
> real possibility.
>
> I will add the pessimization to posix_memalign once the 8.0 freeze is
> over.  It will be quite some time before this behavior becomes
> ubiquitous, so in the meanwhile it's probably a good idea to modify vlc
> to avoid such allocation requests.

Great, thanks.

I agree and will forward the vlc patch to the maintainer of
the FreeBSD port if getting it committed upstream fails.

Fabian