Bug 269688 - memalign() produces division by zero if alignment is 0
Summary: memalign() produces division by zero if alignment is 0
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 13.1-RELEASE
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-02-19 16:31 UTC by Paul Floyd
Modified: 2023-09-25 04:34 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Floyd 2023-02-19 16:31:14 UTC
The implementation is to just call aligned_alloc():

	return (aligned_alloc(align, roundup(size, align)));

where roundup is this macro

:#define        roundup(x, y)   ((((x)+((y)-1))/(y))*(y))  /* to any y */

If size is 0 that will be

   (0 + align - 1) / 0 * 0


That gives me

(gdb) r
Starting program: /usr/home/paulf/scratch/valgrind/memcheck/tests/memalign2 

Program received signal SIGFPE, Arithmetic exception.
Integer divide by zero.
0x00000008002f3415 in memalign (align=0, size=<optimized out>) at /usr/src/lib/libc/gen/memalign.c:39
39              return (aligned_alloc(align, roundup(size, align)));

My source code is

91         p = memalign(0, 100);      assert(NULL == p);
Comment 1 Mina Galić freebsd_triage 2023-02-19 16:34:17 UTC
that function is also undocumented, so it's hard to know what the expected behaviour should be.
Comment 2 Paul Floyd 2023-02-19 17:14:00 UTC
On other platforms the behaviour is:

macOS doesn't exist
Linux glibc

just calls malloc

https://elixir.bootlin.com/glibc/glibc-2.37.9000/source/malloc/malloc.c#L3510
  /* If we need less alignment than we give anyway, just relay to malloc.  */
  if (alignment <= MALLOC_ALIGNMENT)
    return __libc_malloc (bytes);

musl

just calls malloc

https://github.com/esmil/musl/blob/master/src/malloc/memalign.c

if (align <= 4*sizeof(size_t)) {
		if (!(mem = malloc(len)))
			return NULL;
		return mem;
	}

illimos

sets EINVAL and returns NULL

https://github.com/illumos/illumos-gate/blob/master/usr/src/lib/libc/port/gen/memalign.c

	/*
	 * check for valid size and alignment parameters
	 * MAX_ALIGN check prevents overflow in later calculation.
	 */
	if (nbytes == 0 || _misaligned(align) || align == 0 ||
	    align > MAX_ALIGN) {
		errno = EINVAL;
		return (NULL);
	}
Comment 3 Paul Floyd 2023-02-19 17:17:18 UTC
And I would just pass it on to aligned_alloc:

void *
memalign(size_t align, size_t size)
{
	return (aligned_alloc(align, size ? roundup(size, align) : size));
}
Comment 4 Ed Maste freebsd_committer freebsd_triage 2023-02-23 00:16:08 UTC
In `p = memalign(0, 100);` align is 0 and size is 100.
Comment 5 Paul Floyd 2023-02-23 06:32:20 UTC
(In reply to Ed Maste from comment #4)

Yes you're right. Sorry for the brain fart. I'll continue on github (with the advantage that I can edit my mistakes more easily).
Comment 6 commit-hook freebsd_committer freebsd_triage 2023-02-24 18:21:58 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=2c709ee70ade9fd8f77b37917a4169d667dda41d

commit 2c709ee70ade9fd8f77b37917a4169d667dda41d
Author:     Paul Floyd <pjfloyd@wanadoo.fr>
AuthorDate: 2023-02-24 16:29:01 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2023-02-24 18:19:06 +0000

    libc: handle zero alignment in memalign()

    For compatibility with glibc. The previous code would trigger a division
    by zero in roundup() and terminate.  Instead, just pass through to
    malloc() for align == 0.

    PR:             269688
    Reviewed by:    imp, mjg
    MFC after:      1 week
    Pull Request:   https://github.com/freebsd/freebsd-src/pull/655

 lib/libc/gen/memalign.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
Comment 7 commit-hook freebsd_committer freebsd_triage 2023-03-03 00:24:10 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=1b1d2aea020e88b3d2076207b13fec3adde42aa9

commit 1b1d2aea020e88b3d2076207b13fec3adde42aa9
Author:     Paul Floyd <pjfloyd@wanadoo.fr>
AuthorDate: 2023-02-24 16:29:01 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2023-03-03 00:23:24 +0000

    libc: handle zero alignment in memalign()

    For compatibility with glibc. The previous code would trigger a division
    by zero in roundup() and terminate.  Instead, just pass through to
    malloc() for align == 0.

    PR:             269688
    Reviewed by:    imp, mjg
    MFC after:      1 week
    Pull Request:   https://github.com/freebsd/freebsd-src/pull/655

    (cherry picked from commit 2c709ee70ade9fd8f77b37917a4169d667dda41d)

 lib/libc/gen/memalign.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
Comment 8 Paul Floyd 2023-09-25 04:34:59 UTC
 libc: handle zero alignment in memalign()

For compatibility with glibc. The previous code would trigger a division
by zero in roundup() and terminate.  Instead, just pass through to
malloc() for align == 0.

PR:		269688
Reviewed by:	imp, mjg
MFC after:	1 week
Pull Request:	#655