Skip to contentSkip to conversation

pnfsd: Add IP address validation to nfsd4_set_pnfs_dlm_device()

PATCH In review J. Bruce Fields 2 participants 4 messages Jun 14, 2010 – Jun 15, 2010

Subsystems (2)
NFS client NFS server
lore
Share & tools
lore

J. Bruce Fields

author
#0 lore raw

PATCH 1/1 pnfsd: Add IP address validation to nfsd4_set_pnfs_dlm_device()

From: Eric Anderle <[email protected]>

We should catch errors in the format at the time the list is given to the kernel, rather than just returning garbage to the client and letting the client fail.

Signed-off-by J. Bruce Fields <[email protected]>
---
 fs/nfsd/nfs4pnfsdlm.c |   29 +++++++++++++++++++++++------
 1 files changed, 23 insertions(+), 6 deletions(-)
fs/nfsd/nfs4pnfsdlm.c +23 -6
index 1cf1b69..b6d9f23 100644
@@ -26,6 +26,7 @@
 #include <linux/nfsd/debug.h>
 #include <linux/nfsd/nfs4pnfsdlm.h>
 #include <linux/nfsd/nfs4layoutxdr.h>
+#include <linux/sunrpc/clnt.h>
 
 #include "nfsfh.h"
 #include "nfsd.h"

@@ -87,6 +88,24 @@ out:
 	return ret;
 }
 
+bool nfsd4_validate_pnfs_dlm_device(char *ds_list, int len, int *num_ds)
+{
+	char *start = ds_list;
+
+	*num_ds = 0;
+
+	while (*start) {
+		struct sockaddr_storage tempAddr;
+		int ipLen = strcspn(start, ",");
+
+		if (!rpc_pton(start, ipLen, (struct sockaddr *)&tempAddr, sizeof(tempAddr)))
+			return false;
+		(*num_ds)++;
+		start += ipLen + 1;
+	}
+	return true;
+}
+
 /*
  * pnfs_dlm_device string format:
  *     block-device-path:<ds1 ipv4 address>,<ds2 ipv4 address>

@@ -140,12 +159,10 @@ nfsd4_set_pnfs_dlm_device(char *pnfs_dlm_device, int len)
 		goto out_free;
 	memcpy(new->ds_list, bufp, len);
 
-	/* count the number of comma-delimited DS IPs */
-	new->num_ds = 1;
-	while ((bufp = strchr(bufp, ',')) != NULL) {
-		new->num_ds++;
-		bufp++;
-	}
+
+	/*  validate the ips */
+	if (!nfsd4_validate_pnfs_dlm_device(new->ds_list, len, &(new->num_ds)))
+		goto out_free;
 
 	dprintk("%s disk_name %s num_ds %d ds_list %s\n", __func__,
 		new->disk_name, new->num_ds, new->ds_list);
Signature · 1 line
1.7.0.4
#1 lore raw
Quoted 24 lines · J. Bruce Fields

From: Eric Anderle <[email protected]>

We should catch errors in the format at the time the list is given to the kernel, rather than just returning garbage to the client and letting the client fail.

Signed-off-by J. Bruce Fields <[email protected]>
---
 fs/nfsd/nfs4pnfsdlm.c |   29 +++++++++++++++++++++++------
 1 files changed, 23 insertions(+), 6 deletions(-)
fs/nfsd/nfs4pnfsdlm.c +1 -0
index 1cf1b69..b6d9f23 100644
@@ -26,6 +26,7 @@
 #include <linux/nfsd/debug.h>
 #include <linux/nfsd/nfs4pnfsdlm.h>
 #include <linux/nfsd/nfs4layoutxdr.h>
+#include <linux/sunrpc/clnt.h>
 
 #include "nfsfh.h"
 #include "nfsd.h"

fs/nfsd/nfs4pnfsdlm.c out: · line 91

Quoted 4 lines
fs/nfsd/nfs4pnfsdlm.c +1 -0
@@ -87,6 +88,24 @@ out:
 	return ret;
 }
 
+bool nfsd4_validate_pnfs_dlm_device(char *ds_list, int len, int *num_ds)

Is "len" used?

Benny

Quoted 37 lines
fs/nfsd/nfs4pnfsdlm.c +21 -6
+{
+	char *start = ds_list;
+
+	*num_ds = 0;
+
+	while (*start) {
+		struct sockaddr_storage tempAddr;
+		int ipLen = strcspn(start, ",");
+
+		if (!rpc_pton(start, ipLen, (struct sockaddr *)&tempAddr, sizeof(tempAddr)))
+			return false;
+		(*num_ds)++;
+		start += ipLen + 1;
+	}
+	return true;
+}
+
 /*
  * pnfs_dlm_device string format:
  *     block-device-path:<ds1 ipv4 address>,<ds2 ipv4 address>

@@ -140,12 +159,10 @@ nfsd4_set_pnfs_dlm_device(char *pnfs_dlm_device, int len)
 		goto out_free;
 	memcpy(new->ds_list, bufp, len);
 
-	/* count the number of comma-delimited DS IPs */
-	new->num_ds = 1;
-	while ((bufp = strchr(bufp, ',')) != NULL) {
-		new->num_ds++;
-		bufp++;
-	}
+
+	/*  validate the ips */
+	if (!nfsd4_validate_pnfs_dlm_device(new->ds_list, len, &(new->num_ds)))
+		goto out_free;
 
 	dprintk("%s disk_name %s num_ds %d ds_list %s\n", __func__,
 		new->disk_name, new->num_ds, new->ds_list);
#2 lore raw
Quoted 28 lines · Benny Halevy
Quoted 27 lines · J. Bruce Fields

From: Eric Anderle <[email protected]>

We should catch errors in the format at the time the list is given to the kernel, rather than just returning garbage to the client and letting the client fail.

Signed-off-by J. Bruce Fields <[email protected]>
---
 fs/nfsd/nfs4pnfsdlm.c |   29 +++++++++++++++++++++++------
 1 files changed, 23 insertions(+), 6 deletions(-)
fs/nfsd/nfs4pnfsdlm.c +1 -0
index 1cf1b69..b6d9f23 100644
@@ -26,6 +26,7 @@
 #include <linux/nfsd/debug.h>
 #include <linux/nfsd/nfs4pnfsdlm.h>
 #include <linux/nfsd/nfs4layoutxdr.h>
+#include <linux/sunrpc/clnt.h>
 
 #include "nfsfh.h"
 #include "nfsd.h"

@@ -87,6 +88,24 @@ out:
 	return ret;
 }
Quoted 3 lines

fs/nfsd/nfs4pnfsdlm.c out: · line 91

Quoted 1 line
fs/nfsd/nfs4pnfsdlm.c +1 -0
+bool nfsd4_validate_pnfs_dlm_device(char *ds_list, int len, int *num_ds)

Is "len" used?

Merged at pnfs-all-2.6.35-rc3-2010-06-14
len parameter removed.

Thanks!

Benny

Quoted 44 lines

Benny

Quoted 27 lines
fs/nfsd/nfs4pnfsdlm.c +17 -3
+{
+	char *start = ds_list;
+
+	*num_ds = 0;
+
+	while (*start) {
+		struct sockaddr_storage tempAddr;
+		int ipLen = strcspn(start, ",");
+
+		if (!rpc_pton(start, ipLen, (struct sockaddr *)&tempAddr, sizeof(tempAddr)))
+			return false;
+		(*num_ds)++;
+		start += ipLen + 1;
+	}
+	return true;
+}
+
 /*
  * pnfs_dlm_device string format:
  *     block-device-path:<ds1 ipv4 address>,<ds2 ipv4 address>

@@ -140,12 +159,10 @@ nfsd4_set_pnfs_dlm_device(char *pnfs_dlm_device, int len)
 		goto out_free;
 	memcpy(new->ds_list, bufp, len);
 
-	/* count the number of comma-delimited DS IPs */
-	new->num_ds = 1;
-	while ((bufp = strchr(bufp, ',')) != NULL) {

fs/nfsd/nfs4pnfsdlm.c nfsd4_set_pnfs_dlm_device(char *pnfs_dlm_device, int len) · line 168

Quoted 10 lines
fs/nfsd/nfs4pnfsdlm.c +4 -3
-		new->num_ds++;
-		bufp++;
-	}
+
+	/*  validate the ips */
+	if (!nfsd4_validate_pnfs_dlm_device(new->ds_list, len, &(new->num_ds)))
+		goto out_free;
 
 	dprintk("%s disk_name %s num_ds %d ds_list %s\n", __func__,
 		new->disk_name, new->num_ds, new->ds_list);

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#3 lore raw
Quoted 29 lines · Benny Halevy
Quoted 28 lines · Benny Halevy
Quoted 27 lines · J. Bruce Fields

From: Eric Anderle <[email protected]>

We should catch errors in the format at the time the list is given to the kernel, rather than just returning garbage to the client and letting the client fail.

Signed-off-by J. Bruce Fields <[email protected]>
---
 fs/nfsd/nfs4pnfsdlm.c |   29 +++++++++++++++++++++++------
 1 files changed, 23 insertions(+), 6 deletions(-)
fs/nfsd/nfs4pnfsdlm.c +1 -0
index 1cf1b69..b6d9f23 100644
@@ -26,6 +26,7 @@
 #include <linux/nfsd/debug.h>
 #include <linux/nfsd/nfs4pnfsdlm.h>
 #include <linux/nfsd/nfs4layoutxdr.h>
+#include <linux/sunrpc/clnt.h>
 
 #include "nfsfh.h"
 #include "nfsd.h"

@@ -87,6 +88,24 @@ out:
 	return ret;
 }
Quoted 6 lines
Quoted 3 lines

fs/nfsd/nfs4pnfsdlm.c out: · line 91

Quoted 1 line
fs/nfsd/nfs4pnfsdlm.c +1 -0
+bool nfsd4_validate_pnfs_dlm_device(char *ds_list, int len, int *num_ds)

Is "len" used?

Merged at pnfs-all-2.6.35-rc3-2010-06-14
len parameter removed.

Thanks.

--b.