nfs: Fix showing truncated fsid/dev in /proc/net/nfsfs/volumes

PATCH Superseded Kinglong Mee 2 participants 4 messages Jun 12, 2015 – Jun 13, 2015

Subsystems (2)

NFS client NFS server

lore

Share & tools

lore

Kinglong Mee

author

2015-06-12 13:47 UTC #0 lore raw

PATCH 1/1 nfs: Fix showing truncated fsid/dev in /proc/net/nfsfs/volumes

A truncated fsid showing from /proc/fs/nfsfs/volumes as,

# cat /proc/fs/nfsfs/volumes
NV SERVER   PORT DEV     FSID              FSC
v4 c0a80881  801 0:43    34931f044c2a439b  no

It should be as,

# cat /proc/fs/nfsfs/volumes
NV SERVER   PORT DEV          FSID                              FSC
v4 c0a80881  801 0:43         34931f044c2a439b:954c5d830fa4be8c no

The max buffer length for storing "%llx:%llx" format should be
16 + 1 + 16 + 1 = 34 (16 for %llx, 1 for ':', 1 for '\0').

Also, for storing "%u:%u" of MAJOR() and MINOR() should be
8 + 1 + 3 + 1 = 13 (8 for 2^24, 1 for ':', 3 for 2^8, 1 for '\0').

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/nfs/client.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

fs/nfs/client.c +6 -5

index 892aeff..873f194 100644
@@ -1364,27 +1364,28 @@ static int nfs_volume_list_show(struct seq_file *m, void *v)
 {
 	struct nfs_server *server;
 	struct nfs_client *clp;
-	char dev[8], fsid[17];
+	char dev[13], fsid[34];
 	struct nfs_net *nn = net_generic(seq_file_net(m), nfs_net_id);
 
 	/* display header on line 1 */
 	if (v == &nn->nfs_volume_list) {
-		seq_puts(m, "NV SERVER   PORT DEV     FSID              FSC\n");
+		seq_puts(m, "NV SERVER   PORT DEV          FSID"
+			    "                              FSC\n");
 		return 0;
 	}
 	/* display one transport per line on subsequent lines */
 	server = list_entry(v, struct nfs_server, master_link);
 	clp = server->nfs_client;
 
-	snprintf(dev, 8, "%u:%u",
+	snprintf(dev, 13, "%u:%u",
 		 MAJOR(server->s_dev), MINOR(server->s_dev));
 
-	snprintf(fsid, 17, "%llx:%llx",
+	snprintf(fsid, 34, "%llx:%llx",
 		 (unsigned long long) server->fsid.major,
 		 (unsigned long long) server->fsid.minor);
 
 	rcu_read_lock();
-	seq_printf(m, "v%u %s %s %-7s %-17s %s\n",
+	seq_printf(m, "v%u %s %s %-12s %-33s %s\n",
 		   clp->rpc_ops->version,
 		   rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_HEX_ADDR),
 		   rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_HEX_PORT),

Signature · 1 line

2.4.3

Trond Myklebust

2015-06-12 14:04 UTC #1 lore raw

Quoted 26 lines · Kinglong Mee

A truncated fsid showing from /proc/fs/nfsfs/volumes as,
# cat /proc/fs/nfsfs/volumes
NV SERVER   PORT DEV     FSID              FSC
v4 c0a80881  801 0:43    34931f044c2a439b  no
It should be as,
# cat /proc/fs/nfsfs/volumes
NV SERVER   PORT DEV          FSID                              FSC
v4 c0a80881  801 0:43         34931f044c2a439b:954c5d830fa4be8c no
The max buffer length for storing "%llx:%llx" format should be
16 + 1 + 16 + 1 = 34 (16 for %llx, 1 for ':', 1 for '\0').
Also, for storing "%u:%u" of MAJOR() and MINOR() should be
8 + 1 + 3 + 1 = 13 (8 for 2^24, 1 for ':', 3 for 2^8, 1 for '\0').
Signed-off-by Kinglong Mee <[email protected]>
---
 fs/nfs/client.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
fs/nfs/client.c +0 -0
index 892aeff..873f194 100644

fs/nfs/client.c static int nfs_volume_list_show(struct seq_file *m, void *v) · line 1367

Quoted 5 lines

fs/nfs/client.c +1 -1

@@ -1364,27 +1364,28 @@ static int nfs_volume_list_show(struct seq_file *m, void *v)
 {
        struct nfs_server *server;
        struct nfs_client *clp;
-       char dev[8], fsid[17];
+       char dev[13], fsid[34];

Could use a comment to explain why these particular sizes.

Quoted 5 lines

fs/nfs/client.c +0 -1

        struct nfs_net *nn = net_generic(seq_file_net(m), nfs_net_id);
 
        /* display header on line 1 */
        if (v == &nn->nfs_volume_list) {
-               seq_puts(m, "NV SERVER   PORT DEV     FSID              FSC\n");

fs/nfs/client.c static int nfs_volume_list_show(struct seq_file *m, void *v) · line 1380

Quoted 10 lines

fs/nfs/client.c +3 -1

+               seq_puts(m, "NV SERVER   PORT DEV          FSID"
+                           "                              FSC\n");
                return 0;
        }
        /* display one transport per line on subsequent lines */
        server = list_entry(v, struct nfs_server, master_link);
        clp = server->nfs_client;
 
-       snprintf(dev, 8, "%u:%u",
+       snprintf(dev, 13, "%u:%u",

Is there any reason not to settle for using sizeof(dev) here?

fs/nfs/client.c static int nfs_volume_list_show(struct seq_file *m, void *v) · line 1383

Quoted 4 lines

fs/nfs/client.c +1 -1

                 MAJOR(server->s_dev), MINOR(server->s_dev));
 
-       snprintf(fsid, 17, "%llx:%llx",
+       snprintf(fsid, 34, "%llx:%llx",

ditto

Quoted 12 lines

fs/nfs/client.c +1 -2

                 (unsigned long long) server->fsid.major,
                 (unsigned long long) server->fsid.minor);
 
        rcu_read_lock();
-       seq_printf(m, "v%u %s %s %-7s %-17s %s\n",
+       seq_printf(m, "v%u %s %s %-12s %-33s %s\n",
                   clp->rpc_ops->version,
                   rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_HEX_ADDR),
                   rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_HEX_PORT),
--

2.4.3

Kinglong Mee

author

2015-06-13 00:58 UTC #2 lore raw

Quoted 11 lines · Trond Myklebust

Quoted 10 lines · Kinglong Mee

A truncated fsid showing from /proc/fs/nfsfs/volumes as,

# cat /proc/fs/nfsfs/volumes
NV SERVER   PORT DEV     FSID              FSC
v4 c0a80881  801 0:43    34931f044c2a439b  no

It should be as,

# cat /proc/fs/nfsfs/volumes
NV SERVER   PORT DEV          FSID                              FSC
v4 c0a80881  801 0:43         34931f044c2a439b:954c5d830fa4be8c no

Quoted 5 lines

Quoted 5 lines
The max buffer length for storing "%llx:%llx" format should be
16 + 1 + 16 + 1 = 34 (16 for %llx, 1 for ':', 1 for '\0').
Also, for storing "%u:%u" of MAJOR() and MINOR() should be
8 + 1 + 3 + 1 = 13 (8 for 2^24, 1 for ':', 3 for 2^8, 1 for '\0').

Here is the comments for why buffer size is 13 and 34.

Quoted 32 lines

Quoted 11 lines

Signed-off-by Kinglong Mee <[email protected]>
---
 fs/nfs/client.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
fs/nfs/client.c +0 -0
index 892aeff..873f194 100644

fs/nfs/client.c static int nfs_volume_list_show(struct seq_file *m, void *v) · line 1367

Quoted 5 lines

fs/nfs/client.c +1 -1

@@ -1364,27 +1364,28 @@ static int nfs_volume_list_show(struct seq_file *m, void *v)
 {
        struct nfs_server *server;
        struct nfs_client *clp;
-       char dev[8], fsid[17];
+       char dev[13], fsid[34];

Could use a comment to explain why these particular sizes.

Quoted 13 lines

fs/nfs/client.c +2 -1

        struct nfs_net *nn = net_generic(seq_file_net(m), nfs_net_id);
 
        /* display header on line 1 */
        if (v == &nn->nfs_volume_list) {
-               seq_puts(m, "NV SERVER   PORT DEV     FSID              FSC\n");
+               seq_puts(m, "NV SERVER   PORT DEV          FSID"
+                           "                              FSC\n");
                return 0;
        }
        /* display one transport per line on subsequent lines */
        server = list_entry(v, struct nfs_server, master_link);
        clp = server->nfs_client;

Quoted 4 lines

fs/nfs/client.c static int nfs_volume_list_show(struct seq_file *m, void *v) · line 1380
Quoted 2 lines
fs/nfs/client.c +1 -1
-       snprintf(dev, 8, "%u:%u",
+       snprintf(dev, 13, "%u:%u",
Is there any reason not to settle for using sizeof(dev) here?

No, I just modify it as before.
I will update it as using sizeof(dev) in the new version.

thanks,
Kinglong Mee

Quoted 21 lines

fs/nfs/client.c static int nfs_volume_list_show(struct seq_file *m, void *v) · line 1383

Quoted 5 lines

fs/nfs/client.c +1 -1

                 MAJOR(server->s_dev), MINOR(server->s_dev));
 
-       snprintf(fsid, 17, "%llx:%llx",
+       snprintf(fsid, 34, "%llx:%llx",

ditto

Quoted 12 lines

fs/nfs/client.c +1 -2

                 (unsigned long long) server->fsid.major,
                 (unsigned long long) server->fsid.minor);
 
        rcu_read_lock();
-       seq_printf(m, "v%u %s %s %-7s %-17s %s\n",
+       seq_printf(m, "v%u %s %s %-12s %-33s %s\n",
                   clp->rpc_ops->version,
                   rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_HEX_ADDR),
                   rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_HEX_PORT),
--

2.4.3

Kinglong Mee

author

2015-06-13 02:07 UTC #3 lore raw

PATCH v2 1/1 nfs: Fix showing truncated fsid/dev in, /proc/net/nfsfs/volumes

In reply to Trond Myklebust

A truncated fsid showing from /proc/fs/nfsfs/volumes as,

NV SERVER   PORT DEV     FSID              FSC
v4 c0a80881  801 0:43    34931f044c2a439b  no

It should be as,
NV SERVER PORT DEV FSID FSC
v4 c0a80881 801 0:43 34931f044c2a439b:954c5d830fa4be8c no

The max buffer length for storing "%llx:%llx" format should be
16 + 1 + 16 + 1 = 34 (16 for %llx, 1 for ':', 1 for '\0').

Also, for storing "%u:%u" of MAJOR() and MINOR() should be
8 + 1 + 3 + 1 = 13 (8 for 2^24, 1 for ':', 3 for 2^8, 1 for '\0').

v2, add comments for dev/fsid buffer and use sizeof in snprintf.

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/nfs/client.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

fs/nfs/client.c +7 -5

index 892aeff..1e37491 100644
@@ -1364,27 +1364,29 @@ static int nfs_volume_list_show(struct seq_file *m, void *v)
 {
 	struct nfs_server *server;
 	struct nfs_client *clp;
-	char dev[8], fsid[17];
+	char dev[13];	// 8 for 2^24, 1 for ':', 3 for 2^8, 1 for '\0'
+	char fsid[34];	// 2 * 16 for %llx, 1 for ':', 1 for '\0'
 	struct nfs_net *nn = net_generic(seq_file_net(m), nfs_net_id);
 
 	/* display header on line 1 */
 	if (v == &nn->nfs_volume_list) {
-		seq_puts(m, "NV SERVER   PORT DEV     FSID              FSC\n");
+		seq_puts(m, "NV SERVER   PORT DEV          FSID"
+			    "                              FSC\n");
 		return 0;
 	}
 	/* display one transport per line on subsequent lines */
 	server = list_entry(v, struct nfs_server, master_link);
 	clp = server->nfs_client;
 
-	snprintf(dev, 8, "%u:%u",
+	snprintf(dev, sizeof(dev), "%u:%u",
 		 MAJOR(server->s_dev), MINOR(server->s_dev));
 
-	snprintf(fsid, 17, "%llx:%llx",
+	snprintf(fsid, sizeof(fsid), "%llx:%llx",
 		 (unsigned long long) server->fsid.major,
 		 (unsigned long long) server->fsid.minor);
 
 	rcu_read_lock();
-	seq_printf(m, "v%u %s %s %-7s %-17s %s\n",
+	seq_printf(m, "v%u %s %s %-12s %-33s %s\n",
 		   clp->rpc_ops->version,
 		   rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_HEX_ADDR),
 		   rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_HEX_PORT),

Signature · 1 line

2.4.3