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