[OE-core] [PATCH] rpm: fix a endian incompatible error in generating tag
Ming Liu
ming.liu at windriver.com
Fri Feb 7 05:07:33 UTC 2014
On 01/29/2014 10:51 PM, Mark Hatle wrote:
> On 1/28/14, 9:27 PM, Ming Liu wrote:
>> 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 at 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 at 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.
>
> The general byte order of the RPM package should be little endian from
> my memory. So we'll want to make sure that on a load we do le32toh,
> and then on a write we do htole32.
>
> My confusion with the math is:
>
> if the tag is stored as 0x0000138C (tag 5004) (little endian):
>
> 1100 1000 0011 0001 0000 0000 0000 0000
>
> C 8 3 1 0 0 0 0
>
> and then it's & 0x3fffffff (on a little endian machine), it stays as:
>
> 1100 1000 0011 0001 0000 0000 0000 0000
>
> but if it's & 0x3fffffff on a big endian machine, it would suddenly
> become:
>
>
> 0000 1000 0011 0001 0000 0000 0000 0000
>
> Which is incorrect. This is why the math needs to happen before the
> endian convert.
>
> The | makes it even worse, as sets bit '30'.
>
> So what I think needs to happen is that on read of the 4 bytes of
> data, they need to be converted to -from little endian- to host
> format. Then the tag function can remain as it is... when written
> back to the data struction it needs to be converted from host back to
> little endian.
>
> Looking at the code, I expect that there is a byte swap when other
> tags are generated and/or the tag is written into the file.
>
> I've not found the code, but the issue may be that the
> rpmDigestUpdate/rpmDigestFinal may already be doing an endian
> conversion for you. Then when you copy the key out it needs to be
> converted back to the native endian. If this is the case then le32toh
> would be the correct conversion at this point.
>
> Looking at the underlying implementation (for big endian machines):
>
> # define htole32(x) __bswap_32 (x)
>
> # define le32toh(x) __bswap_32 (x)
>
> So they are the same function in reality. So perhaps the only real
> issue with the patch is that it should be written as "le32toh" in
> order to avoid confusion when people read the code. As the conversion
> is identical.
>
> So my suggestion would be:
>
> + if (digest && digestlen > 4) {
> ++ /* The tag is stored in little endian byte order for
> cross-endian compatibility.
> ++ Swap to host order if appropriate. */
> + memcpy(&tag, digest + (digestlen - 4), 4);
> ++ tag = le32toh(tag);
>
> (generated code is the same, but this now makes sense to me, and the
> math works.)
Thanks for your explaination! That really makes sense, I will send a new
patch as you suggested.
//Ming Liu
>
> --Mark
>
>> //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
>>>>
>>>
>>>
>>>
>>
>
>
>
More information about the Openembedded-core
mailing list