xdrstdio_create buffers do not output encoded values on ppc

PATCH Superseded Steve Dickson 2 participants 4 messages Jun 28, 2018 – Jun 29, 2018

Subsystems (2)

NFS client NFS server

lore

Share & tools

lore

Steve Dickson

author

2018-06-28 15:29 UTC #0 lore raw

PATCH 1/1 xdrstdio_create buffers do not output encoded values on ppc

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(-)

src/xdr_stdio.c +5 -5

index 4410262..b415f61 100644
@@ -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);
 }

Signature · 1 line

2.17.1

Trond Myklebust

2018-06-28 16:17 UTC #1 lore raw

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=

Steve Dickson

author

2018-06-28 16:40 UTC #2 lore raw

Quoted 50 lines · Trond Myklebust

Quoted 46 lines · Steve Dickson
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(-)
src/xdr_stdio.c +3 -3
index 4410262..b415f61 100644
@@ -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);
}
src/xdr_stdio.c +2 -2
@@ -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);
Quoted 2 lines
return (TRUE);
}

Quoted 8 lines

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.

Trond Myklebust

2018-06-29 01:47 UTC #3 lore raw

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==