From: Daniel Sands <[email protected]>
The cause is that the xdr_putlong uses a long to store the
converted value, then passes it to fwrite as a byte buffer.
Only the first 4 bytes are written, which is okay for a LE
system after byteswapping, but writes all zeroes on BE systems.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1261738
Signed-off-by: Steve Dickson <[email protected]>
---
src/xdr_stdio.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/xdr_stdio.c b/src/xdr_stdio.c
index 4410262..b415f61 100644
--- a/src/xdr_stdio.c
+++ b/src/xdr_stdio.c
@@ -103,10 +103,10 @@ xdrstdio_getlong(xdrs, lp)
XDR *xdrs;
long *lp;
{
-
- if (fread(lp, sizeof(int32_t), 1, (FILE *)xdrs->x_private) != 1)
+ u_int32_t mycopy;
+ if (fread(&mycopy, sizeof(u_int32_t), 1, (FILE *)xdrs->x_private) != 1)
return (FALSE);
- *lp = (long)ntohl((u_int32_t)*lp);
+ *lp = (long)ntohl(mycopy);
return (TRUE);
}
@@ -115,9 +115,9 @@ xdrstdio_putlong(xdrs, lp)
XDR *xdrs;
const long *lp;
{
- long mycopy = (long)htonl((u_int32_t)*lp);
+ u_int32_t mycopy = (u_int32_t)htonl((u_int32_t)*lp);
- if (fwrite(&mycopy, sizeof(int32_t), 1, (FILE *)xdrs->x_private) != 1)
+ if (fwrite(&mycopy, sizeof(u_int32_t), 1, (FILE *)xdrs->x_private) != 1)
return (FALSE);
return (TRUE);
}
--
2.17.1
T24gVGh1LCAyMDE4LTA2LTI4IGF0IDExOjI5IC0wNDAwLCBTdGV2ZSBEaWNrc29uIHdyb3RlOg0K
PiBGcm9tOiBEYW5pZWwgU2FuZHMgPGRuc2FuZHNAc2FuZGlhLmdvdj4NCj4gDQo+IFRoZSBjYXVz
ZSBpcyB0aGF0IHRoZSB4ZHJfcHV0bG9uZyB1c2VzIGEgbG9uZyB0byBzdG9yZSB0aGUNCj4gY29u
dmVydGVkIHZhbHVlLCB0aGVuIHBhc3NlcyBpdCB0byBmd3JpdGUgYXMgYSBieXRlIGJ1ZmZlci4N
Cj4gT25seSB0aGUgZmlyc3QgNCBieXRlcyBhcmUgd3JpdHRlbiwgd2hpY2ggaXMgb2theSBmb3Ig
YSBMRQ0KPiBzeXN0ZW0gYWZ0ZXIgYnl0ZXN3YXBwaW5nLCBidXQgd3JpdGVzIGFsbCB6ZXJvZXMg
b24gQkUgc3lzdGVtcy4NCj4gDQo+IEZpeGVzOiBodHRwczovL2J1Z3ppbGxhLnJlZGhhdC5jb20v
c2hvd19idWcuY2dpP2lkPTEyNjE3MzgNCj4gDQo+IFNpZ25lZC1vZmYtYnk6IFN0ZXZlIERpY2tz
b24gPHN0ZXZlZEByZWRoYXQuY29tPg0KPiAtLS0NCj4gIHNyYy94ZHJfc3RkaW8uYyB8IDEwICsr
KysrLS0tLS0NCj4gIDEgZmlsZSBjaGFuZ2VkLCA1IGluc2VydGlvbnMoKyksIDUgZGVsZXRpb25z
KC0pDQo+IA0KPiBkaWZmIC0tZ2l0IGEvc3JjL3hkcl9zdGRpby5jIGIvc3JjL3hkcl9zdGRpby5j
DQo+IGluZGV4IDQ0MTAyNjIuLmI0MTVmNjEgMTAwNjQ0DQo+IC0tLSBhL3NyYy94ZHJfc3RkaW8u
Yw0KPiArKysgYi9zcmMveGRyX3N0ZGlvLmMNCj4gQEAgLTEwMywxMCArMTAzLDEwIEBAIHhkcnN0
ZGlvX2dldGxvbmcoeGRycywgbHApDQo+ICAJWERSICp4ZHJzOw0KPiAgCWxvbmcgKmxwOw0KPiAg
ew0KPiAtDQo+IC0JaWYgKGZyZWFkKGxwLCBzaXplb2YoaW50MzJfdCksIDEsIChGSUxFICopeGRy
cy0+eF9wcml2YXRlKQ0KPiAhPSAxKQ0KPiArCXVfaW50MzJfdCBteWNvcHk7DQo+ICsJaWYgKGZy
ZWFkKCZteWNvcHksIHNpemVvZih1X2ludDMyX3QpLCAxLCAoRklMRSAqKXhkcnMtDQo+ID54X3By
aXZhdGUpICE9IDEpDQo+ICAJCXJldHVybiAoRkFMU0UpOw0KPiAtCSpscCA9IChsb25nKW50b2hs
KCh1X2ludDMyX3QpKmxwKTsNCj4gKwkqbHAgPSAobG9uZyludG9obChteWNvcHkpOw0KPiAgCXJl
dHVybiAoVFJVRSk7DQo+ICB9DQo+ICANCj4gQEAgLTExNSw5ICsxMTUsOSBAQCB4ZHJzdGRpb19w
dXRsb25nKHhkcnMsIGxwKQ0KPiAgCVhEUiAqeGRyczsNCj4gIAljb25zdCBsb25nICpscDsNCj4g
IHsNCj4gLQlsb25nIG15Y29weSA9IChsb25nKWh0b25sKCh1X2ludDMyX3QpKmxwKTsNCj4gKwl1
X2ludDMyX3QgbXljb3B5ID0gKHVfaW50MzJfdClodG9ubCgodV9pbnQzMl90KSpscCk7DQo+ICAN
Cj4gLQlpZiAoZndyaXRlKCZteWNvcHksIHNpemVvZihpbnQzMl90KSwgMSwgKEZJTEUgKil4ZHJz
LQ0KPiA+eF9wcml2YXRlKSAhPSAxKQ0KPiArCWlmIChmd3JpdGUoJm15Y29weSwgc2l6ZW9mKHVf
aW50MzJfdCksIDEsIChGSUxFICopeGRycy0NCj4gPnhfcHJpdmF0ZSkgIT0gMSkNCj4gIAkJcmV0
dXJuIChGQUxTRSk7DQo+ICAJcmV0dXJuIChUUlVFKTsNCj4gIH0NCg0KSG1tLi4uIEdpdmVuIHRo
YXQgbW9zdCBzZXR1cHMgdG9kYXkgdGVuZCB0byBiZSA2NC1iaXQsIHNob3VsZG4ndCB0aGVyZQ0K
YWxzbyBiZSBib3VuZHMgY2hlY2tpbmcgaW4gdGhlIGFib3ZlICd4ZHJzdGRpb19wdXRsb25nKCkn
IGluIG9yZGVyIHRvDQptYWtlIGl0IHNhZmU/DQoNClNvbWV0aGluZyBsaWtlDQoNCmlmICgobG9u
ZykodV9pbnQzMl90KSpscCAhPSAqbHApIHJldHVybiAoRkFMU0UpOw0KDQotLSANClRyb25kIE15
a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVyLCBIYW1tZXJzcGFjZQ0KdHJvbmQu
bXlrbGVidXN0QGhhbW1lcnNwYWNlLmNvbQ0KDQo=
On 06/28/2018 12:17 PM, Trond Myklebust wrote:
> On Thu, 2018-06-28 at 11:29 -0400, Steve Dickson wrote:
>> From: Daniel Sands <[email protected]>
>>
>> The cause is that the xdr_putlong uses a long to store the
>> converted value, then passes it to fwrite as a byte buffer.
>> Only the first 4 bytes are written, which is okay for a LE
>> system after byteswapping, but writes all zeroes on BE systems.
>>
>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1261738
>>
>> Signed-off-by: Steve Dickson <[email protected]>
>> ---
>> src/xdr_stdio.c | 10 +++++-----
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/xdr_stdio.c b/src/xdr_stdio.c
>> index 4410262..b415f61 100644
>> --- a/src/xdr_stdio.c
>> +++ b/src/xdr_stdio.c
>> @@ -103,10 +103,10 @@ xdrstdio_getlong(xdrs, lp)
>> XDR *xdrs;
>> long *lp;
>> {
>> -
>> - if (fread(lp, sizeof(int32_t), 1, (FILE *)xdrs->x_private)
>> != 1)
>> + u_int32_t mycopy;
>> + if (fread(&mycopy, sizeof(u_int32_t), 1, (FILE *)xdrs-
>>> x_private) != 1)
>> return (FALSE);
>> - *lp = (long)ntohl((u_int32_t)*lp);
>> + *lp = (long)ntohl(mycopy);
>> return (TRUE);
>> }
>>
>> @@ -115,9 +115,9 @@ xdrstdio_putlong(xdrs, lp)
>> XDR *xdrs;
>> const long *lp;
>> {
>> - long mycopy = (long)htonl((u_int32_t)*lp);
>> + u_int32_t mycopy = (u_int32_t)htonl((u_int32_t)*lp);
>>
>> - if (fwrite(&mycopy, sizeof(int32_t), 1, (FILE *)xdrs-
>>> x_private) != 1)
>> + if (fwrite(&mycopy, sizeof(u_int32_t), 1, (FILE *)xdrs-
>>> x_private) != 1)
>> return (FALSE);
>> return (TRUE);
>> }
>
> Hmm... Given that most setups today tend to be 64-bit, shouldn't there
> also be bounds checking in the above 'xdrstdio_putlong()' in order to
> make it safe?
>
> Something like
>
> if ((long)(u_int32_t)*lp != *lp) return (FALSE);
>
Sorry... I'm not following this... why is this necessary
and what are you making safe?
steved.
T24gVGh1LCAyMDE4LTA2LTI4IGF0IDEyOjQwIC0wNDAwLCBTdGV2ZSBEaWNrc29uIHdyb3RlOg0K
PiANCj4gT24gMDYvMjgvMjAxOCAxMjoxNyBQTSwgVHJvbmQgTXlrbGVidXN0IHdyb3RlOg0KPiA+
IE9uIFRodSwgMjAxOC0wNi0yOCBhdCAxMToyOSAtMDQwMCwgU3RldmUgRGlja3NvbiB3cm90ZToN
Cj4gPiA+IEZyb206IERhbmllbCBTYW5kcyA8ZG5zYW5kc0BzYW5kaWEuZ292Pg0KPiA+ID4gDQo+
ID4gPiBUaGUgY2F1c2UgaXMgdGhhdCB0aGUgeGRyX3B1dGxvbmcgdXNlcyBhIGxvbmcgdG8gc3Rv
cmUgdGhlDQo+ID4gPiBjb252ZXJ0ZWQgdmFsdWUsIHRoZW4gcGFzc2VzIGl0IHRvIGZ3cml0ZSBh
cyBhIGJ5dGUgYnVmZmVyLg0KPiA+ID4gT25seSB0aGUgZmlyc3QgNCBieXRlcyBhcmUgd3JpdHRl
biwgd2hpY2ggaXMgb2theSBmb3IgYSBMRQ0KPiA+ID4gc3lzdGVtIGFmdGVyIGJ5dGVzd2FwcGlu
ZywgYnV0IHdyaXRlcyBhbGwgemVyb2VzIG9uIEJFIHN5c3RlbXMuDQo+ID4gPiANCj4gPiA+IEZp
eGVzOiBodHRwczovL2J1Z3ppbGxhLnJlZGhhdC5jb20vc2hvd19idWcuY2dpP2lkPTEyNjE3MzgN
Cj4gPiA+IA0KPiA+ID4gU2lnbmVkLW9mZi1ieTogU3RldmUgRGlja3NvbiA8c3RldmVkQHJlZGhh
dC5jb20+DQo+ID4gPiAtLS0NCj4gPiA+ICBzcmMveGRyX3N0ZGlvLmMgfCAxMCArKysrKy0tLS0t
DQo+ID4gPiAgMSBmaWxlIGNoYW5nZWQsIDUgaW5zZXJ0aW9ucygrKSwgNSBkZWxldGlvbnMoLSkN
Cj4gPiA+IA0KPiA+ID4gZGlmZiAtLWdpdCBhL3NyYy94ZHJfc3RkaW8uYyBiL3NyYy94ZHJfc3Rk
aW8uYw0KPiA+ID4gaW5kZXggNDQxMDI2Mi4uYjQxNWY2MSAxMDA2NDQNCj4gPiA+IC0tLSBhL3Ny
Yy94ZHJfc3RkaW8uYw0KPiA+ID4gKysrIGIvc3JjL3hkcl9zdGRpby5jDQo+ID4gPiBAQCAtMTAz
LDEwICsxMDMsMTAgQEAgeGRyc3RkaW9fZ2V0bG9uZyh4ZHJzLCBscCkNCj4gPiA+ICAJWERSICp4
ZHJzOw0KPiA+ID4gIAlsb25nICpscDsNCj4gPiA+ICB7DQo+ID4gPiAtDQo+ID4gPiAtCWlmIChm
cmVhZChscCwgc2l6ZW9mKGludDMyX3QpLCAxLCAoRklMRSAqKXhkcnMtDQo+ID4gPiA+eF9wcml2
YXRlKQ0KPiA+ID4gIT0gMSkNCj4gPiA+ICsJdV9pbnQzMl90IG15Y29weTsNCj4gPiA+ICsJaWYg
KGZyZWFkKCZteWNvcHksIHNpemVvZih1X2ludDMyX3QpLCAxLCAoRklMRSAqKXhkcnMtDQo+ID4g
PiA+IHhfcHJpdmF0ZSkgIT0gMSkNCj4gPiA+IA0KPiA+ID4gIAkJcmV0dXJuIChGQUxTRSk7DQo+
ID4gPiAtCSpscCA9IChsb25nKW50b2hsKCh1X2ludDMyX3QpKmxwKTsNCj4gPiA+ICsJKmxwID0g
KGxvbmcpbnRvaGwobXljb3B5KTsNCj4gPiA+ICAJcmV0dXJuIChUUlVFKTsNCj4gPiA+ICB9DQo+
ID4gPiAgDQo+ID4gPiBAQCAtMTE1LDkgKzExNSw5IEBAIHhkcnN0ZGlvX3B1dGxvbmcoeGRycywg
bHApDQo+ID4gPiAgCVhEUiAqeGRyczsNCj4gPiA+ICAJY29uc3QgbG9uZyAqbHA7DQo+ID4gPiAg
ew0KPiA+ID4gLQlsb25nIG15Y29weSA9IChsb25nKWh0b25sKCh1X2ludDMyX3QpKmxwKTsNCj4g
PiA+ICsJdV9pbnQzMl90IG15Y29weSA9ICh1X2ludDMyX3QpaHRvbmwoKHVfaW50MzJfdCkqbHAp
Ow0KPiA+ID4gIA0KPiA+ID4gLQlpZiAoZndyaXRlKCZteWNvcHksIHNpemVvZihpbnQzMl90KSwg
MSwgKEZJTEUgKil4ZHJzLQ0KPiA+ID4gPiB4X3ByaXZhdGUpICE9IDEpDQo+ID4gPiANCj4gPiA+
ICsJaWYgKGZ3cml0ZSgmbXljb3B5LCBzaXplb2YodV9pbnQzMl90KSwgMSwgKEZJTEUgKil4ZHJz
LQ0KPiA+ID4gPiB4X3ByaXZhdGUpICE9IDEpDQo+ID4gPiANCj4gPiA+ICAJCXJldHVybiAoRkFM
U0UpOw0KPiA+ID4gIAlyZXR1cm4gKFRSVUUpOw0KPiA+ID4gIH0NCj4gPiANCj4gPiBIbW0uLi4g
R2l2ZW4gdGhhdCBtb3N0IHNldHVwcyB0b2RheSB0ZW5kIHRvIGJlIDY0LWJpdCwgc2hvdWxkbid0
DQo+ID4gdGhlcmUNCj4gPiBhbHNvIGJlIGJvdW5kcyBjaGVja2luZyBpbiB0aGUgYWJvdmUgJ3hk
cnN0ZGlvX3B1dGxvbmcoKScgaW4gb3JkZXINCj4gPiB0bw0KPiA+IG1ha2UgaXQgc2FmZT8NCj4g
PiANCj4gPiBTb21ldGhpbmcgbGlrZQ0KPiA+IA0KPiA+IGlmICgobG9uZykodV9pbnQzMl90KSps
cCAhPSAqbHApIHJldHVybiAoRkFMU0UpOw0KPiA+IA0KPiANCj4gU29ycnkuLi4gSSdtIG5vdCBm
b2xsb3dpbmcgdGhpcy4uLiB3aHkgaXMgdGhpcyBuZWNlc3NhcnkNCj4gYW5kIHdoYXQgYXJlIHlv
dSBtYWtpbmcgc2FmZT8NCj4gDQpBIGxvbmcgaW50ZWdlciBvbiBtb3N0IDY0LWJpdCBzeXN0ZW1z
IGlzIDY0LWJpdCBsb25nLCBidXQgaW4gdGhlIGNvZGUNCmFib3ZlLCB5b3UgYXJlIHB1c2hpbmcg
dGhhdCB2YWx1ZSBpbnRvIGEgMzItYml0IGJpZy1lbmRpYW4gaW50ZWdlci4gU28NCmEgc2FmZSBp
bXBsZW1lbnRhdGlvbiB3b3VsZCBub3JtYWxseSBjaGVjayBmb3Igd2hldGhlciBvciBub3QgdGhl
IDY0LQ0KYml0IHZhbHVlIGlzIGdldHRpbmcgdHJ1bmNhdGVkIHdoZW4gaXQgZ2V0cyBjYXN0IHRv
IHVfaW50MzJfdCwgYW5kDQp3b3VsZCB0aHJvdyBhbiBlcnJvciBpbiB0aGUgY2FzZSB3aGVyZSB0
aGUgbG9uZyByZWFsbHkgZG9lcyBub3QgZml0DQppbnRvIHRoYXQgMzItYml0IGludGVnZXIuDQoN
Ci0tIA0KVHJvbmQgTXlrbGVidXN0DQpDVE8sIEhhbW1lcnNwYWNlIEluYw0KNDMwMCBFbCBDYW1p
bm8gUmVhbCwgU3VpdGUgMTA1DQpMb3MgQWx0b3MsIENBIDk0MDIyDQp3d3cuaGFtbWVyLnNwYWNl
DQoNCg==