Fix potential memory leak in cpu enumeration on Linux

[martin-frbg]

An early return after a failed call to sched_getaffinity would leak the previously allocated cpu_set_t. Wrong calculation of the size argument in that call increased the likelyhood of that failure. Fixes #2003

[jbrandmeyer]

Beware that cpusetp has a pointer type. I think you really want sizeof(*cpusetp)

[martin-frbg]

Beware that cpusetp has a pointer type. I think you really want sizeof(*cpusetp)

I must admit that I am a bit confused about this. Size of the type evaluates to 512 on a mere dualcore laptop and using that it bombs out with a memory management error in an unrelated free() later.

[jbrandmeyer]

At this point, I'm confused, too. I spent some time reading through the glibc headers and sources.

The cpuset macros are clearly operating by allocating a number of unsigned long. CPU_ALLOC(n) does not necessarily return a whole cpu_set_t, despite the return type being a cpu_set_t *.

sched_getaffinity() is just calling into the kernel. From sysdeps/unix/sysv/linux/sched_getaffinity.c, it looks like the kernel is returning the number of bytes written into the cpuset output argument. Then, the syscall wrapper clears any additional memory, according to the size passed in by the caller (ie, openblas).

That suggests that the right argument to pass to sched_getaffinity is the result of calling CPU_ALLOC_SIZE(), and not sizeof(cpu_set_t). It just so happens that CPU_ALLOC_SIZE(n) for n <= 64 is also sizeof(intptr_t), so everything appears to work on commodity hardware.

My suggestion is to just add the CPU_FREE in the error-handling path for the moment. I suspect that its actually the kernel that's buggy in my case when the size is 8. I'm going source-diving to see if I can figure it out in their end.

[jbrandmeyer]

FWIW, tools/testing/selftests/rseq/param_test.c from the kernel sources always allocates a whole cpu_set_t on the stack, rather than use dynamic allocation.

[martin-frbg]

The actual error appears to be in my declaration of cpusetp as a cpu_set_t* which is appropriate for dynamic allocation but pushed an uninitalized struct onto sched_getaffinity otherwise. Not sure yet why CPU_ALLOC returned a wrong value for you, but perhaps this should not be called when the number of processors is well below 1024 (although some documentation suggests otherwise)

[martin-frbg]

@jbrandmeyer could you try the lastest iteration please, just to see if sched_getaffinity is still failing with the cpu set returned by CPU_ALLOC ?

[martin-frbg]

Committing after testing on Ryzen2-2700 with and without taskset.