[U-Boot] [PATCH 1/2] net: nfs: Drop CONFIG_NFS_READ_SIZE

In the general case, CONFIG_NFS_READ_SIZE is unchanged from the default of 1024. There are in fact no in-tree users that increase this size. Adjust the comment to reflect what could be done in the future in conjunction with CONFIG_IP_DEFRAG.
Cc: Joe Hershberger joe.hershberger@ni.com Signed-off-by: Tom Rini trini@konsulko.com --- net/nfs.h | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/net/nfs.h b/net/nfs.h index 70a1a6d554be..1aa06e8fb90f 100644 --- a/net/nfs.h +++ b/net/nfs.h @@ -36,16 +36,13 @@ #define NFSERR_ISDIR 21 #define NFSERR_INVAL 22
-/* Block size used for NFS read accesses. A RPC reply packet (including all +/* + * Block size used for NFS read accesses. A RPC reply packet (including all * headers) must fit within a single Ethernet frame to avoid fragmentation. - * However, if CONFIG_IP_DEFRAG is set, the config file may want to use a - * bigger value. In any case, most NFS servers are optimized for a power of 2. + * However, if CONFIG_IP_DEFRAG is set, a bigger value could be used. In any + * case, most NFS servers are optimized for a power of 2. */ -#ifdef CONFIG_NFS_READ_SIZE -#define NFS_READ_SIZE CONFIG_NFS_READ_SIZE -#else -#define NFS_READ_SIZE 1024 /* biggest power of two that fits Ether frame */ -#endif +#define NFS_READ_SIZE 1024 /* biggest power of two that fits Ether frame */
/* Values for Accept State flag on RPC answers (See: rfc1831) */ enum rpc_accept_stat {

In rpc_t we declare data to be a uint8_t of size 2048, for a final size of 2048. We also however declare the reply part of the union to have a uint32_t data field of NFS_READ_SIZE (1024) for a final size of 4096+24=4120 and an overrun. Expand the comment above the struct to note that if NFS_READ_SIZE is increased then the data buf must also be increased and correct the declaration to be uint8_t.
Reported-by: Coverity (CID: 152888) Cc: Joe Hershberger joe.hershberger@ni.com Signed-off-by: Tom Rini trini@konsulko.com --- net/nfs.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/nfs.h b/net/nfs.h index 1aa06e8fb90f..b23b4088d825 100644 --- a/net/nfs.h +++ b/net/nfs.h @@ -39,8 +39,9 @@ /* * Block size used for NFS read accesses. A RPC reply packet (including all * headers) must fit within a single Ethernet frame to avoid fragmentation. - * However, if CONFIG_IP_DEFRAG is set, a bigger value could be used. In any - * case, most NFS servers are optimized for a power of 2. + * However, if CONFIG_IP_DEFRAG is set, a bigger value could be used, so long + * as rpc_t->u->data is incrased to match. In any case, most NFS servers are + * optimized for a power of 2. */ #define NFS_READ_SIZE 1024 /* biggest power of two that fits Ether frame */
@@ -73,7 +74,7 @@ struct rpc_t { uint32_t verifier; uint32_t v2; uint32_t astatus; - uint32_t data[NFS_READ_SIZE]; + uint8_t data[NFS_READ_SIZE]; } reply; } u; } __attribute__((packed));

On Sun, Aug 20, 2017 at 9:40 PM, Tom Rini trini@konsulko.com wrote:
In rpc_t we declare data to be a uint8_t of size 2048, for a final size of 2048. We also however declare the reply part of the union to have a uint32_t data field of NFS_READ_SIZE (1024) for a final size of 4096+24=4120 and an overrun. Expand the comment above the struct to note that if NFS_READ_SIZE is increased then the data buf must also be increased and correct the declaration to be uint8_t.
Reported-by: Coverity (CID: 152888) Cc: Joe Hershberger joe.hershberger@ni.com Signed-off-by: Tom Rini trini@konsulko.com
Acked-by: Joe Hershberger joe.hershberger@ni.com

Hi Tom,
On Sun, Aug 20, 2017 at 9:40 PM, Tom Rini trini@konsulko.com wrote:
In rpc_t we declare data to be a uint8_t of size 2048, for a final size of 2048. We also however declare the reply part of the union to have a uint32_t data field of NFS_READ_SIZE (1024) for a final size of 4096+24=4120 and an overrun. Expand the comment above the struct to note that if NFS_READ_SIZE is increased then the data buf must also be increased and correct the declaration to be uint8_t.
Reported-by: Coverity (CID: 152888) Cc: Joe Hershberger joe.hershberger@ni.com Signed-off-by: Tom Rini trini@konsulko.com
This seems to be breaking one of the targets... https://travis-ci.org/jhershbe/u-boot/jobs/269330530
Thanks, -Joe

On Sun, Aug 20, 2017 at 9:40 PM, Tom Rini trini@konsulko.com wrote:
In rpc_t we declare data to be a uint8_t of size 2048, for a final size of 2048. We also however declare the reply part of the union to have a uint32_t data field of NFS_READ_SIZE (1024) for a final size of 4096+24=4120 and an overrun. Expand the comment above the struct to note that if NFS_READ_SIZE is increased then the data buf must also be increased and correct the declaration to be uint8_t.
Reported-by: Coverity (CID: 152888) Cc: Joe Hershberger joe.hershberger@ni.com Signed-off-by: Tom Rini trini@konsulko.com
net/nfs.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/nfs.h b/net/nfs.h index 1aa06e8fb90f..b23b4088d825 100644 --- a/net/nfs.h +++ b/net/nfs.h @@ -39,8 +39,9 @@ /*
- Block size used for NFS read accesses. A RPC reply packet (including all
- headers) must fit within a single Ethernet frame to avoid fragmentation.
- However, if CONFIG_IP_DEFRAG is set, a bigger value could be used. In any
- case, most NFS servers are optimized for a power of 2.
- However, if CONFIG_IP_DEFRAG is set, a bigger value could be used, so long
- as rpc_t->u->data is incrased to match. In any case, most NFS servers are
*/
- optimized for a power of 2.
#define NFS_READ_SIZE 1024 /* biggest power of two that fits Ether frame */
@@ -73,7 +74,7 @@ struct rpc_t { uint32_t verifier; uint32_t v2; uint32_t astatus;
uint32_t data[NFS_READ_SIZE];
uint8_t data[NFS_READ_SIZE];
All of the pointer math would also need to be updated. Didn't notice that at first so,
Nacked-by: Joe Hershberger joe.hershberger@ni.com
} reply; } u;
} __attribute__((packed));
1.9.1
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot

On Sun, Aug 20, 2017 at 9:40 PM, Tom Rini trini@konsulko.com wrote:
In the general case, CONFIG_NFS_READ_SIZE is unchanged from the default of 1024. There are in fact no in-tree users that increase this size. Adjust the comment to reflect what could be done in the future in conjunction with CONFIG_IP_DEFRAG.
Cc: Joe Hershberger joe.hershberger@ni.com Signed-off-by: Tom Rini trini@konsulko.com
Acked-by: Joe Hershberger joe.hershberger@ni.com

Hi Tom,
https://patchwork.ozlabs.org/patch/803831/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git
Thanks! -Joe
participants (2)
-
Joe Hershberger
-
Tom Rini