On 07/10/2018 11:51 AM, Chuck Lever wrote: > > >> On Jul 10, 2018, at 11:43 AM, Steve Dickson <SteveD@xxxxxxxxxx> wrote: >> >> >> >> On 07/10/2018 10:49 AM, Chuck Lever wrote: >>> >>> >>>> On Jul 10, 2018, at 10:44 AM, Steve Dickson <SteveD@xxxxxxxxxx> 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> >>>> --- >>>> v3: Reworked the bounds checking >>>> >>>> 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) >>> >>> Hi Steve, _LP64 is gcc-specific. Hope that's OK. >>> >>> Reviewed-by: Chuck Lever <chuck.lever@xxxxxxxxxx> >> Here the reason I left it: >> https://stackoverflow.com/questions/685124/how-to-identify-a-64-bit-build-on-linux-using-the-preprocessor >> >> Should it be removed? > > Not at all, I'm just pointing out that if libtirpc is built > with a non-gcc compiler, _LP64 is not guaranteed to be > defined. > > That stackoverflow article suggests a couple of more portable > ways of checking for 64-bit. > > Are there distributions that might build libtirpc with a > non-gcc compiler (like Clang) ? If no, then this isn't an > issue at all. Lets cross that bridge if/when it comes... Thanks for the time! steved. > > >> steved. >> >>> >>> >>>> + if ((*lp > INT32_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); >>>> -- >>>> 2.17.1 >>>> >>>> >>>> ------------------------------------------------------------------------------ >>>> Check out the vibrant tech community on one of the world's most >>>> engaging tech sites, Slashdot.org! http://sdm.link/slashdot >>>> _______________________________________________ >>>> Libtirpc-devel mailing list >>>> Libtirpc-devel@xxxxxxxxxxxxxxxxxxxxx >>>> https://lists.sourceforge.net/lists/listinfo/libtirpc-devel >>> >>> -- >>> Chuck Lever > > -- > 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