[OE-core] [PATCH 1/1] util-linux-native: fix qsort_r for CentOS 5.10

[Paul Barker]

On 1 April 2014 02:34, Robert Yang <liezhi.yang at windriver.com> wrote:
>
>
> On 04/01/2014 05:22 AM, Paul Barker wrote:
>>
>> On 26 March 2014 07:01, Robert Yang <liezhi.yang at windriver.com> wrote:
>>>
>>> The qsort_r() was added to glibc in version 2.8, so there is no qsort_r()
>>> on
>>> the host like CentOS 5.x, use qsort() to fix it since they are nearly
>>> identical.
>>>
>>> Signed-off-by: Robert Yang <liezhi.yang at windriver.com>
>>> ---
>>>   .../util-linux/util-linux-native-qsort.patch       |   34
>>> ++++++++++++++++++++
>>>   meta/recipes-core/util-linux/util-linux_2.24.1.bb  |    4 ++-
>>>   2 files changed, 37 insertions(+), 1 deletion(-)
>>>   create mode 100644
>>> meta/recipes-core/util-linux/util-linux/util-linux-native-qsort.patch
>>>
>>> diff --git
>>> a/meta/recipes-core/util-linux/util-linux/util-linux-native-qsort.patch
>>> b/meta/recipes-core/util-linux/util-linux/util-linux-native-qsort.patch
>>> new file mode 100644
>>> index 0000000..1707683
>>> --- /dev/null
>>> +++
>>> b/meta/recipes-core/util-linux/util-linux/util-linux-native-qsort.patch
>>> @@ -0,0 +1,34 @@
>>> +From f220d809be1baa654503bf6ff52f3630b0d7015c Mon Sep 17 00:00:00 2001
>>> +From: Robert Yang <liezhi.yang at windriver.com>
>>> +Date: Wed, 26 Mar 2014 01:30:29 +0000
>>> +Subject: [PATCH] sun.c: use qsort() to instead of qsort_r()
>>> +
>>> +qsort_r() was added to glibc in version 2.8, so there is no qsort_r() on
>>> +the host like CentOS 5.x.
>>> +
>>> +Upstream-Status: Inappropriate [Other]
>>> +
>>> +Signed-off-by: Robert Yang <liezhi.yang at windriver.com>
>>> +---
>>> + libfdisk/src/sun.c | 5 ++---
>>> + 1 file changed, 2 insertions(+), 3 deletions(-)
>>> +
>>> +diff --git a/libfdisk/src/sun.c b/libfdisk/src/sun.c
>>> +index e73c701..f7899ec 100644
>>> +--- a/libfdisk/src/sun.c
>>> ++++ b/libfdisk/src/sun.c
>>> +@@ -427,9 +427,8 @@ static int sun_verify_disklabel(struct fdisk_context
>>> *cxt)
>>> +         else
>>> +             array[i] = -1;
>>> +     }
>>> +-    qsort_r(array,ARRAY_SIZE(array),sizeof(array[0]),
>>> +-        (int (*)(const void *,const void *,void *)) verify_sun_cmp,
>>> +-        verify_sun_starts);
>>> ++    qsort(array,ARRAY_SIZE(array),sizeof(array[0]),
>>> ++        (int (*)(const void *,const void *)) verify_sun_cmp);
>>
>>
>> I've just been looking at this for building util-linux on top of musl
>> (as musl-libc doesn't implement qsort_r)
>> and my solution was to import a qsort_r implementation from ccl
>> (https://ccl.googlecode.com/svn/trunk/qsort_r.c).
>>
>
> Maybe we can do it in YP 1.7.
>
>
>> Are you sure this solution works? verify_sun_cmp takes 3 parameters
>> not 2 and it uses the 3rd parameter (data). From reading this I'd
>> imagine a segfault is likely to occur in verify_sun_cmp if qsort is
>> used instead of qsort_r.
>>
>
> I think it works well since there is a similar patch before we upgrade
> the util-linux-native, I will verify later.
>

The more I look at it the more I don't like this patch. It's probably
a very rarely used code path but it could blow up if it's called. C
provides no guarantees that calling a 3-argument function with only 2
arguments will work. Depending on calling convention it could easily
result in stack corruption on some platforms.

I'd suggest we try reverting the relevant bits of the upstream change
from qsort to
qsort_r: http://git.kernel.org/cgit/utils/util-linux/util-linux.git/commit/?id=c69bbca9c1f6645097bd20fe3a21f5a99a2a0698

I think it should just be the first 3 patch hunks in that commit.

-- 
Paul Barker

Email: paul at paulbarker.me.uk
http://www.paulbarker.me.uk