> On Jun 29, 2018, at 3:29 PM, Trond Myklebust <trondmy@xxxxxxxxxxxxxxx> wrote: > > On Fri, 2018-06-29 at 14:42 -0400, Steve Dickson wrote: >> From: Daniel Sands <dnsands@xxxxxxxxxx> >> >> 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 <steved@xxxxxxxxxx> >> --- >> v2: Added bounds checking >> Changed from unsigned to signed >> >> src/xdr_stdio.c | 14 +++++++++++--- >> 1 file changed, 11 insertions(+), 3 deletions(-) >> >> diff --git a/src/xdr_stdio.c b/src/xdr_stdio.c >> index 4410262..b902acd 100644 >> --- a/src/xdr_stdio.c >> +++ b/src/xdr_stdio.c >> @@ -103,10 +103,12 @@ xdrstdio_getlong(xdrs, lp) >> XDR *xdrs; >> long *lp; >> { >> + int32_t mycopy; >> >> - if (fread(lp, sizeof(int32_t), 1, (FILE *)xdrs->x_private) >> != 1) >> + if (fread(&mycopy, sizeof(int32_t), 1, (FILE *)xdrs- >>> x_private) != 1) >> return (FALSE); >> - *lp = (long)ntohl((u_int32_t)*lp); >> + >> + *lp = (long)ntohl(mycopy); >> return (TRUE); >> } >> >> @@ -115,8 +117,14 @@ xdrstdio_putlong(xdrs, lp) >> XDR *xdrs; >> const long *lp; >> { >> - long mycopy = (long)htonl((u_int32_t)*lp); >> + int32_t mycopy; >> + >> +#if defined(_LP64) >> + if ((*lp > INT32_MAX) || (*lp < INT32_MIN)) >> + return (FALSE); > > The will work just fine if your cast is: > int a; > > lp = (long)a; > > ...but it won't do the right thing for > > unsigned int b = UINT32_MAX; > > lp = (long)b; /* set lp to UINT32_MAX */ mycopy is an int32_t now. Does that help? > Since there us no dedicated xdrstdio_putulong() to use for unsigned > integers, we need the check to work for both cases. > For that reason, I think the above check either has to be: > > if (*lp == (long)((u_int32_t)*lp)) > return (FALSE); > > ...or something uglier like > > if (*lp > UINT32_MAX || *lp < INT32_MIN) > return (FALSE); > >> +#endif >> >> + mycopy = (int32_t)htonl((int32_t)*lp); >> if (fwrite(&mycopy, sizeof(int32_t), 1, (FILE *)xdrs- >>> x_private) != 1) >> return (FALSE); >> return (TRUE); > -- > Trond Myklebust > CTO, Hammerspace Inc > 4300 El Camino Real, Suite 105 > Los Altos, CA 94022 > www.hammer.space > > N‹§ēæėrļ›yúčšØbēXŽķĮ§vØ^–)Þš{.nĮ+‰·ĨŠ{ąû"žØ^n‡rĄöĶzËëh™ĻčÚ&Ēø ŪGŦéhŪ(階ŠÝĒj"úķm§ĸïęäzđÞ–ŠāþfĢĒ·hšˆ§~ˆm -- Chuck Lever -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
