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]>
---
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)
+ 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
> On Jul 10, 2018, at 10:44 AM, Steve Dickson <[email protected]> wrote:
>=20
> From: Daniel Sands <[email protected]>
>=20
> 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.
>=20
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=3D1261738
>=20
> Signed-off-by: Steve Dickson <[email protected]>
> ---
> v3: Reworked the bounds checking
>=20
> v2: Added bounds checking
> Changed from unsigned to signed
>=20
> src/xdr_stdio.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>=20
> 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;
>=20
> - if (fread(lp, sizeof(int32_t), 1, (FILE *)xdrs->x_private) !=3D =
1)
> + if (fread(&mycopy, sizeof(int32_t), 1, (FILE *)xdrs->x_private) =
!=3D 1)
> return (FALSE);
> - *lp =3D (long)ntohl((u_int32_t)*lp);
> +
> + *lp =3D (long)ntohl(mycopy);
> return (TRUE);
> }
>=20
> @@ -115,8 +117,14 @@ xdrstdio_putlong(xdrs, lp)
> XDR *xdrs;
> const long *lp;
> {
> - long mycopy =3D (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 <[email protected]>
> + if ((*lp > INT32_MAX) || (*lp < INT32_MIN))
> + return (FALSE);
> +#endif
>=20
> + mycopy =3D (int32_t)htonl((int32_t)*lp);
> if (fwrite(&mycopy, sizeof(int32_t), 1, (FILE *)xdrs->x_private) =
!=3D 1)
> return (FALSE);
> return (TRUE);
> --=20
> 2.17.1
>=20
>=20
> =
--------------------------------------------------------------------------=
----
> 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
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/libtirpc-devel
--
Chuck Lever
On 07/10/2018 10:49 AM, Chuck Lever wrote:
>
>
>> On Jul 10, 2018, at 10:44 AM, Steve Dickson <[email protected]> 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]>
>> ---
>> 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 <[email protected]>
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?
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
>> [email protected]
>> https://lists.sourceforge.net/lists/listinfo/libtirpc-devel
>
> --
> Chuck Lever
>
>
>
> On Jul 10, 2018, at 11:43 AM, Steve Dickson <[email protected]> wrote:
>=20
>=20
>=20
> On 07/10/2018 10:49 AM, Chuck Lever wrote:
>>=20
>>=20
>>> On Jul 10, 2018, at 10:44 AM, Steve Dickson <[email protected]> =
wrote:
>>>=20
>>> From: Daniel Sands <[email protected]>
>>>=20
>>> 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.
>>>=20
>>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=3D1261738
>>>=20
>>> Signed-off-by: Steve Dickson <[email protected]>
>>> ---
>>> v3: Reworked the bounds checking
>>>=20
>>> v2: Added bounds checking
>>> Changed from unsigned to signed
>>>=20
>>> src/xdr_stdio.c | 14 +++++++++++---
>>> 1 file changed, 11 insertions(+), 3 deletions(-)
>>>=20
>>> 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;
>>>=20
>>> - if (fread(lp, sizeof(int32_t), 1, (FILE *)xdrs->x_private) !=3D =
1)
>>> + if (fread(&mycopy, sizeof(int32_t), 1, (FILE *)xdrs->x_private) =
!=3D 1)
>>> return (FALSE);
>>> - *lp =3D (long)ntohl((u_int32_t)*lp);
>>> +
>>> + *lp =3D (long)ntohl(mycopy);
>>> return (TRUE);
>>> }
>>>=20
>>> @@ -115,8 +117,14 @@ xdrstdio_putlong(xdrs, lp)
>>> XDR *xdrs;
>>> const long *lp;
>>> {
>>> - long mycopy =3D (long)htonl((u_int32_t)*lp);
>>> + int32_t mycopy;
>>> +
>>> +#if defined(_LP64)
>>=20
>> Hi Steve, _LP64 is gcc-specific. Hope that's OK.
>>=20
>> Reviewed-by: Chuck Lever <[email protected]>
> Here the reason I left it:
> =
https://stackoverflow.com/questions/685124/how-to-identify-a-64-bit-build-=
on-linux-using-the-preprocessor
>=20
> 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.
> steved.
>=20
>>=20
>>=20
>>> + if ((*lp > INT32_MAX) || (*lp < INT32_MIN))
>>> + return (FALSE);
>>> +#endif
>>>=20
>>> + mycopy =3D (int32_t)htonl((int32_t)*lp);
>>> if (fwrite(&mycopy, sizeof(int32_t), 1, (FILE *)xdrs->x_private) =
!=3D 1)
>>> return (FALSE);
>>> return (TRUE);
>>> --=20
>>> 2.17.1
>>>=20
>>>=20
>>> =
--------------------------------------------------------------------------=
----
>>> 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
>>> [email protected]
>>> https://lists.sourceforge.net/lists/listinfo/libtirpc-devel
>>=20
>> --
>> Chuck Lever
--
Chuck Lever
On 07/10/2018 11:51 AM, Chuck Lever wrote:
>
>
>> On Jul 10, 2018, at 11:43 AM, Steve Dickson <[email protected]> wrote:
>>
>>
>>
>> On 07/10/2018 10:49 AM, Chuck Lever wrote:
>>>
>>>
>>>> On Jul 10, 2018, at 10:44 AM, Steve Dickson <[email protected]> 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]>
>>>> ---
>>>> 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 <[email protected]>
>> 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
>>>> [email protected]
>>>> https://lists.sourceforge.net/lists/listinfo/libtirpc-devel
>>>
>>> --
>>> Chuck Lever
>
> --
> Chuck Lever
>
>
>