Skip to contentSkip to conversation

net: nfs: Drop CONFIG_NFS_READ_SIZE

PATCH In review Tom Rini 2 participants 7 messages Aug 21, 2017 – Sep 7, 2017 Networking

lore
Share & tools
lore

Tom Rini

author
#0 lore raw

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 <[email protected]>
Signed-off-by Tom Rini <[email protected]>
---
 net/nfs.h | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)
net/nfs.h +5 -8
index 70a1a6d554be..1aa06e8fb90f 100644
@@ -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 {
Signature · 1 line
1.9.1
author
#1 lore raw

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

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 <[email protected]>
Signed-off-by Tom Rini <[email protected]>
---
 net/nfs.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
net/nfs.h +4 -3
index 1aa06e8fb90f..b23b4088d825 100644
@@ -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));
Signature · 1 line
1.9.1
#2 lore raw

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

Quoted 10 lines · 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 <[email protected]>
Signed-off-by Tom Rini <[email protected]>
Acked-by Joe Hershberger <[email protected]>
#3 lore raw

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

In reply to Tom Rini

Hi Tom,

Quoted 10 lines · 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 <[email protected]>
Signed-off-by Tom Rini <[email protected]>

This seems to be breaking one of the targets...
https://travis-ci.org/jhershbe/u-boot/jobs/269330530

Thanks,
-Joe

NAK
#4 lore raw

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

In reply to Tom Rini
Quoted 31 lines · 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 <[email protected]>
Signed-off-by Tom Rini <[email protected]>
---
 net/nfs.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
net/nfs.h +3 -2
index 1aa06e8fb90f..b23b4088d825 100644
@@ -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 */

net/nfs.h struct rpc_t { · line 77

Quoted 5 lines
net/nfs.h +1 -1
@@ -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 <[email protected]>
Quoted 10 lines
net/nfs.h +0 -1
                } reply;
        } u;
 } __attribute__((packed));
--

1.9.1

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

#5 lore raw

PATCH 1/2 net: nfs: Drop CONFIG_NFS_READ_SIZE

In reply to Tom Rini
Quoted 7 lines · 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 <[email protected]>
Signed-off-by Tom Rini <[email protected]>
Acked-by Joe Hershberger <[email protected]>