Re: [PATCH] rpm: fix a endian incompatible error in generating tag

[Ming Liu <ming.liu@windriver.com>]

On 01/10/2014 02:05 AM, Mark Hatle wrote:
> On 1/9/14, 2:49 AM, Ming Liu wrote:
>> A flaw was found in the way rpm generating arbitrary tags, which 
>> leads to a
>> incorrect query result, this issue is introduced by a incompatible 
>> endianess
>> when the generating process is executed on different architectures.
>>
>> This patch resolves it by taking a uniform byte order.
>>
>> Signed-off-by: Ming Liu <ming.liu@windriver.com>
>> ---
>>   .../rpm-tag-generate-endian-conversion-fix.patch   |   29 
>> ++++++++++++++++++++
>>   meta/recipes-devtools/rpm/rpm_5.4.9.bb             |    1 +
>>   2 files changed, 30 insertions(+), 0 deletions(-)
>>   create mode 100644 
>> meta/recipes-devtools/rpm/rpm/rpm-tag-generate-endian-conversion-fix.patch
>>
>> diff --git 
>> a/meta/recipes-devtools/rpm/rpm/rpm-tag-generate-endian-conversion-fix.patch 
>> b/meta/recipes-devtools/rpm/rpm/rpm-tag-generate-endian-conversion-fix.patch 
>>
>> new file mode 100644
>> index 0000000..4379515
>> --- /dev/null
>> +++ 
>> b/meta/recipes-devtools/rpm/rpm/rpm-tag-generate-endian-conversion-fix.patch
>> @@ -0,0 +1,29 @@
>> +fix a endian incompatible error in generating rpm tag
>> +
>> +A flaw was found in the way rpm generating arbitrary tags, which 
>> leads to a
>> +incorrect query result, this issue is introduced by a incompatible 
>> endianess
>> +when the generating process is executed on different architectures.
>> +
>> +This patch resolves it by taking a uniform byte order.
>> +
>> +Upstream-Status: Pending
>> +
>> +Signed-off-by: Ming Liu <ming.liu@windriver.com>
>> +---
>> + tagname.c |    3 +++
>> + 1 file changed, 3 insertions(+)
>> +
>> +diff -urpN a/rpmdb/tagname.c b/rpmdb/tagname.c
>> +--- a/rpmdb/tagname.c
>> ++++ b/rpmdb/tagname.c
>> +@@ -152,7 +152,10 @@ static rpmTag _tagGenerate(const char *s
>> +     xx = rpmDigestUpdate(ctx, s, nb);
>> +     xx = rpmDigestFinal(ctx, &digest, &digestlen, 0);
>> +     if (digest && digestlen > 4) {
>> ++    /* The tag is stored in a uniform byte order for cross-endian 
>> compatibility.
>> ++       Swap to little-endian if appropriate. */
>> +     memcpy(&tag, digest + (digestlen - 4), 4);
>> ++    tag = htole32(tag);
>> +     tag = (rpmTag) (tag & 0x3fffffff);
>> +     tag = (rpmTag) (tag | 0x40000000);
>
> The above code doesn't look right to me.
>
> If this is reading in from the RPM package, it should be an le32toh..
>
> Otherwise if it's generating the digest info.. then the htole32 should 
> be -after- the & and | operations, otherwise the wrong part of the 
> value will be modified.

Hi, Mark:

I just saw your comments, sorry for the late feedback.
I'd like to explain it more that _tagGenerate is called at both reading 
and generating sides, so either of your recommended resolves only one 
side of it based on my test, the tag is being counted as a number with 
later & or | operator, but before that, we should transfer it to a 
number with uniform byte order(big or little endian) after we get it 
from a piece of memory, which in this case is the last 4 bytes of 
digest, please think about the following scenario:

x86 host (generating and querying)  -->  powerpc/mips (querying)

Using le32toh or putting htole32 after the & and | operations both will 
not satisfy it - to get a same result.

//Ming Liu

>
> --Mark
>
>> +     }
>> diff --git a/meta/recipes-devtools/rpm/rpm_5.4.9.bb 
>> b/meta/recipes-devtools/rpm/rpm_5.4.9.bb
>> index 9d376a5..7921f40 100644
>> --- a/meta/recipes-devtools/rpm/rpm_5.4.9.bb
>> +++ b/meta/recipes-devtools/rpm/rpm_5.4.9.bb
>> @@ -89,6 +89,7 @@ SRC_URI = 
>> "http://www.rpm5.org/files/rpm/rpm-5.4/rpm-5.4.9-0.20120508.src.rpm;ex
>>          file://debugedit-valid-file-to-fix-segment-fault.patch \
>>          file://rpm-platform-file-fix.patch \
>>          file://rpm-lsb-compatibility.patch \
>> +       file://rpm-tag-generate-endian-conversion-fix.patch \
>>         "
>>
>>   # Uncomment the following line to enable platform score debugging
>>
>
>
>