* [PATCH] rpm: fix a endian incompatible error in generating tag @ 2014-01-09 8:49 Ming Liu 2014-01-09 18:05 ` Mark Hatle 0 siblings, 1 reply; 6+ messages in thread From: Ming Liu @ 2014-01-09 8:49 UTC (permalink / raw) To: openembedded-core 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); + } 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 -- 1.7.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] rpm: fix a endian incompatible error in generating tag 2014-01-09 8:49 [PATCH] rpm: fix a endian incompatible error in generating tag Ming Liu @ 2014-01-09 18:05 ` Mark Hatle 2014-01-29 3:27 ` Ming Liu 0 siblings, 1 reply; 6+ messages in thread From: Mark Hatle @ 2014-01-09 18:05 UTC (permalink / raw) To: Ming Liu, openembedded-core 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. --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 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rpm: fix a endian incompatible error in generating tag 2014-01-09 18:05 ` Mark Hatle @ 2014-01-29 3:27 ` Ming Liu 2014-01-29 8:15 ` Ming Liu 2014-01-29 14:51 ` Mark Hatle 0 siblings, 2 replies; 6+ messages in thread From: Ming Liu @ 2014-01-29 3:27 UTC (permalink / raw) To: Mark Hatle; +Cc: openembedded-core 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 >> > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rpm: fix a endian incompatible error in generating tag 2014-01-29 3:27 ` Ming Liu @ 2014-01-29 8:15 ` Ming Liu 2014-01-29 14:51 ` Mark Hatle 1 sibling, 0 replies; 6+ messages in thread From: Ming Liu @ 2014-01-29 8:15 UTC (permalink / raw) To: Mark Hatle, Patches and discussions about the oe-core layer On 01/29/2014 11:27 AM, 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@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. Despite the discussion above, I found another concern with my patch that 'htole32' is not supported by libc of some system, like CentOS, which leads rpm-native fail to compile with my patch, so please ignore this so far, I will make a new patch soon. //Ming Liu > > //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 >>> >> >> >> > > _______________________________________________ > Openembedded-core mailing list > Openembedded-core@lists.openembedded.org > http://lists.openembedded.org/mailman/listinfo/openembedded-core > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rpm: fix a endian incompatible error in generating tag 2014-01-29 3:27 ` Ming Liu 2014-01-29 8:15 ` Ming Liu @ 2014-01-29 14:51 ` Mark Hatle 2014-02-07 5:07 ` Ming Liu 1 sibling, 1 reply; 6+ messages in thread From: Mark Hatle @ 2014-01-29 14:51 UTC (permalink / raw) To: Ming Liu; +Cc: openembedded-core 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@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. 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.) --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 >>> >> >> >> > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rpm: fix a endian incompatible error in generating tag 2014-01-29 14:51 ` Mark Hatle @ 2014-02-07 5:07 ` Ming Liu 0 siblings, 0 replies; 6+ messages in thread From: Ming Liu @ 2014-02-07 5:07 UTC (permalink / raw) To: Mark Hatle; +Cc: openembedded-core 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@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. > > 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 >>>> >>> >>> >>> >> > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-02-07 5:07 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-01-09 8:49 [PATCH] rpm: fix a endian incompatible error in generating tag Ming Liu 2014-01-09 18:05 ` Mark Hatle 2014-01-29 3:27 ` Ming Liu 2014-01-29 8:15 ` Ming Liu 2014-01-29 14:51 ` Mark Hatle 2014-02-07 5:07 ` Ming Liu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox