lkml.org 
[lkml]   [2010]   [Jul]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] Fix off-by-one bug in mbind() syscall implementation
Andi Kleen wrote:
> On Mon, Jul 26, 2010 at 12:23:10PM +0200, Andre Przywara wrote:
>> Andi Kleen wrote:
>>> On Mon, Jul 26, 2010 at 11:28:18AM +0200, Andre Przywara wrote:
>>>> When the mbind() syscall implementation processes the node mask
>>>> provided by the user, the last node is accidentally masked out.
>>>> This is present since the dawn of time (aka Before Git), I guess
>>>> nobody realized that because libnuma as the most prominent user of
>>>> mbind() uses large masks (sizeof(long)) and nobody cared if the
>>>> 64th node is not handled properly. But if the user application
>>>> defers the masking to the kernel and provides the number of valid bits
>>>> in maxnodes, there is always the last node missing.
>>>> However this also affect the special case with maxnodes=0, the manpage
>>>> reads that mbind(ptr, len, MPOL_DEFAULT, &some_long, 0, 0); should
>>>> reset the policy to the default one, but in fact it returns EINVAL.
>>>> This patch just removes the decrease-by-one statement, I hope that
>>>> there is no workaround code in the wild that relies on the bogus
>>>> behavior.
>>> Actually libnuma and likely most existing users rely on it.
>> If grep didn't fool me, then the only users in libnuma aware of that
>> bug are the test implementations in numactl-2.0.3/test, namely
>> /test/tshm.c (NUMA_MAX_NODES+1) and test/mbind_mig_pages.c
>> (old_nodes->size + 1).
>
> At least libnuma 1 (which is the libnuma most distributions use today)
> explicitely knows about it and will break if you change it.
Please define most distributions. I just did some research:
Old libnuma with the workaround active:
* OpenSuse 11.0 (recently EOL)
* Fedora 9 (EOL for about a year)
* SLES10 (still supported, but unlikey to get a vanilla kernel update)
* CentOS 5.5 (same as SLES10)
First version with a safe libnuma:
* OpenSuse 11.1
* Fedora 10
* SLES11
Didn't check others, but I guess that looks similar. If they get an
official kernel update, they likely get the corresponding library fixes
along with it.
Also I found that numactl-1.0.3 already had the bug fix.

So how big is the chance the anyone with these old distros will use a
2.6.36+ kernel with it? If someone does so, then I'd guess he'd be on
his own and will probably also update other parts of the system (or
better upgrade the whole setup).
I see that this is a general question and should not be answered with
probability arguments, but I would like to hear other statements on this
policy. After all this is a clear kernel bug and should be fixed. Recent
library implementation will trigger this bug.
Also I would like to know whether we support any older library with
newer kernels. I guess there is no such promise (thinking of modutils,
udev, ...)
Is the stable syscall interface defined by documentation or by (possibly
buggy) de facto implementation?

>
>> Has this bug been known before?
>
> Yes (and you can argue whether it's a problem or not)
OK, I will:
1. It's not documented, neither in the kernel nor in libnuma.
2. The default interface for large bitmaps (consisting of a number of
longs) is to pass the number of valid bits. A variant would be passing
the highest valid bit number. The number of bits plus one is not in the
list.
3. There is a special case in the syscall interface for resetting the
policy. It says you need to pass either a NULL pointer or 0 for the
number of bits (along with MPOL_DEFAULT). This simply does not work.
Instead you have to pass a NULL pointer or _1_. Also that means that
passing 1 intentionally triggers the special case.
3. libnuma changed the behavior from work-arounding to ignoring some 18
month or so before. This bug will lead to the 64th node (or the 128th
node, the 192th node, ...) to be ignored. And please don't argument that
nobody will ever have 64 nodes...
4. If one use mbind() directly and lets the kernel do the masking by
passing the number of valid bits (and not the size of the buffer) then
the last node will always be masked off.

So I strongly opt for fixing this by removing the line and maybe add
some documentation about the old behavior.

Regards,
Andre.

--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448-3567-12



\
 
 \ /
  Last update: 2010-07-29 11:03    [W:0.044 / U:0.136 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site