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

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

[Tom Rini]

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 {
-- 
1.9.1

[U-Boot] [PATCH 2/2] net: nfs: Correct type of rpc_t->u.reply->data

[Tom Rini]

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));
-- 
1.9.1

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

[Joe Hershberger]

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>

[U-Boot] [PATCH 2/2] net: nfs: Correct type of rpc_t->u.reply->data

[Joe Hershberger]

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>

[U-Boot] [PATCH 2/2] net: nfs: Correct type of rpc_t->u.reply->data

[Joe Hershberger]

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

[U-Boot] [PATCH 2/2] net: nfs: Correct type of rpc_t->u.reply->data

[Joe Hershberger]

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 at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

[U-Boot] net: nfs: Drop CONFIG_NFS_READ_SIZE

[Joe Hershberger]

Hi Tom,

https://patchwork.ozlabs.org/patch/803831/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe