[PATCH 1/8] SUNRPC: Use unsigned string lengths in xdr_decode_string_inplace

[PATCH 1/8] SUNRPC: Use unsigned string lengths in xdr_decode_string_inplace

[Chuck Lever]

XDR strings, opaques, and net objects should all use unsigned lengths.
To wit, RFC 4506 says:

4.2.  Unsigned Integer

   An XDR unsigned integer is a 32-bit datum that encodes a non-negative
   integer in the range [0,4294967295].

 ...

4.11.  String

   The standard defines a string of n (numbered 0 through n-1) ASCII
   bytes to be the number n encoded as an unsigned integer (as described
   above), and followed by the n bytes of the string.

After this patch, xdr_decode_string_inplace now matches the other XDR
string and array helpers that take a string length argument.  See:

xdr_encode_opaque_fixed, xdr_encode_opaque, xdr_encode_array

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 include/linux/sunrpc/xdr.h |    3 ++-
 net/sunrpc/xdr.c           |    8 +++++---
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h
index 0751c94..e4057d7 100644
--- a/include/linux/sunrpc/xdr.h
+++ b/include/linux/sunrpc/xdr.h
@@ -112,7 +112,8 @@ struct xdr_buf {
 __be32 *xdr_encode_opaque_fixed(__be32 *p, const void *ptr, unsigned int len);
 __be32 *xdr_encode_opaque(__be32 *p, const void *ptr, unsigned int len);
 __be32 *xdr_encode_string(__be32 *p, const char *s);
-__be32 *xdr_decode_string_inplace(__be32 *p, char **sp, int *lenp, int maxlen);
+__be32 *xdr_decode_string_inplace(__be32 *p, char **sp, unsigned int *lenp,
+			unsigned int maxlen);
 __be32 *xdr_encode_netobj(__be32 *p, const struct xdr_netobj *);
 __be32 *xdr_decode_netobj(__be32 *p, struct xdr_netobj *);
 
diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
index fdc5e6d..31bd346 100644
--- a/net/sunrpc/xdr.c
+++ b/net/sunrpc/xdr.c
@@ -93,11 +93,13 @@ xdr_encode_string(__be32 *p, const char *string)
 }
 
 __be32 *
-xdr_decode_string_inplace(__be32 *p, char **sp, int *lenp, int maxlen)
+xdr_decode_string_inplace(__be32 *p, char **sp,
+			  unsigned int *lenp, unsigned int maxlen)
 {
-	unsigned int	len;
+	u32 len;
 
-	if ((len = ntohl(*p++)) > maxlen)
+	len = ntohl(*p++);
+	if (len > maxlen)
 		return NULL;
 	*lenp = len;
 	*sp = (char *) p;


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 1/8] SUNRPC: Use unsigned string lengths in xdr_decode_string_inplace

[Neil Brown]

Thanks Chuck.
This series all:
   Acked-By: NeilBrown <neilb@suse.de>

Just one question:

> diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
> index fdc5e6d..31bd346 100644
> --- a/net/sunrpc/xdr.c
> +++ b/net/sunrpc/xdr.c
> @@ -93,11 +93,13 @@ xdr_encode_string(__be32 *p, const char *string)
>  }
>  
>  __be32 *
> -xdr_decode_string_inplace(__be32 *p, char **sp, int *lenp, int maxlen)
> +xdr_decode_string_inplace(__be32 *p, char **sp,
> +			  unsigned int *lenp, unsigned int maxlen)
>  {
> -	unsigned int	len;
> +	u32 len;

What is the reason for making this a u32?  You seem to be happy with
"unsigned int" everywhere else.

It's not important, but I'm curious.

NeilBrown

>  
> -	if ((len = ntohl(*p++)) > maxlen)
> +	len = ntohl(*p++);
> +	if (len > maxlen)
>  		return NULL;
>  	*lenp = len;
>  	*sp = (char *) p;

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 1/8] SUNRPC: Use unsigned string lengths in xdr_decode_string_inplace

[Chuck Lever]

[-- Attachment #1: Type: text/plain, Size: 837 bytes --]

Neil Brown wrote:
> Thanks Chuck.
> This series all:
>    Acked-By: NeilBrown <neilb@suse.de>
> 
> Just one question:
> 
>> diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
>> index fdc5e6d..31bd346 100644
>> --- a/net/sunrpc/xdr.c
>> +++ b/net/sunrpc/xdr.c
>> @@ -93,11 +93,13 @@ xdr_encode_string(__be32 *p, const char *string)
>>  }
>>  
>>  __be32 *
>> -xdr_decode_string_inplace(__be32 *p, char **sp, int *lenp, int maxlen)
>> +xdr_decode_string_inplace(__be32 *p, char **sp,
>> +			  unsigned int *lenp, unsigned int maxlen)
>>  {
>> -	unsigned int	len;
>> +	u32 len;
> 
> What is the reason for making this a u32?  You seem to be happy with
> "unsigned int" everywhere else.
> 
> It's not important, but I'm curious.

It boils down to documenting exactly what comes off the wire, 
independent of what the local "unsigned int" is.

[-- Attachment #2: chuck.lever.vcf --]
[-- Type: text/x-vcard, Size: 259 bytes --]

begin:vcard
fn:Chuck Lever
n:Lever;Chuck
org:Oracle Corporation;Corporate Architecture: Linux Projects Group
adr:;;1015 Granger Avenue;Ann Arbor;MI;48104;USA
title:Principal Member of Staff
tel;work:+1 248 614 5091
x-mozilla-html:FALSE
version:2.1
end:vcard


[-- Attachment #3: Type: text/plain, Size: 314 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

[-- Attachment #4: Type: text/plain, Size: 140 bytes --]

_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs