[pnfs] [PATCH 16/21] pnfsblock: ask for layout_blksize on mount

Benny Halevy bhalevy at panasas.com
Mon Apr 14 04:59:04 EDT 2008


On Apr. 10, 2008, 17:07 +0300, Fred Isaman <iisaman at citi.umich.edu> wrote:
> Note this requires fattr bitmap to be length 3, instead of 2.
> This patch just hacks in enough to get by.  Really need to
> separate out a patch that correctly handles more generic bitmap
> lengths.
> 
> Signed-off-by: Fred Isaman <iisaman at citi.umich.edu>
> Signed-off-by: Benny Halevy <bhalevy at panasas.com>
> ---
>  fs/nfs/client.c           |    1 +
>  fs/nfs/nfs4_fs.h          |    2 +-
>  fs/nfs/nfs4proc.c         |    5 ++-
>  fs/nfs/nfs4xdr.c          |   74 ++++++++++++++++++++++++++++++++++++++++-----
>  include/linux/nfs4.h      |    1 +
>  include/linux/nfs_fs_sb.h |    1 +
>  include/linux/nfs_xdr.h   |    3 +-
>  7 files changed, 75 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index c3451cf..894265c 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -736,6 +736,7 @@ static void nfs_server_set_fsinfo(struct nfs_server *server, struct nfs_fsinfo *
>  		server->wsize = NFS_MAX_FILE_IO_SIZE;
>  	server->wpages = (server->wsize + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT;
>  #ifdef CONFIG_PNFS
> +	server->pnfs_blksize = fsinfo->blksize; /* XXX Need default if 0? */
>  	/* Save the layout type for use during init of layout driver */
>  	server->pnfs_fs_ltype = fsinfo->layoutclass;
>  #endif /* CONFIG_PNFS */
> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
> index 66eeedb..02567cf 100644
> --- a/fs/nfs/nfs4_fs.h
> +++ b/fs/nfs/nfs4_fs.h
> @@ -218,7 +218,7 @@ extern struct nfs4_state_maintenance_ops *nfs4_state_renewal_ops[];
>  extern const u32 nfs4_fattr_bitmap[2];
>  extern const u32 nfs4_statfs_bitmap[2];
>  extern const u32 nfs4_pathconf_bitmap[2];
> -extern const u32 nfs4_fsinfo_bitmap[2];
> +extern const u32 nfs4_fsinfo_bitmap[3];
>  extern const u32 nfs4_fs_locations_bitmap[2];
>  
>  /* nfs4renewd.c */
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 3a54c7e..2375a52 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -122,12 +122,13 @@ const u32 nfs4_pathconf_bitmap[2] = {
>  	0
>  };
>  
> -const u32 nfs4_fsinfo_bitmap[2] = { FATTR4_WORD0_MAXFILESIZE
> +const u32 nfs4_fsinfo_bitmap[3] = { FATTR4_WORD0_MAXFILESIZE
>  			| FATTR4_WORD0_MAXREAD
>  			| FATTR4_WORD0_MAXWRITE
>  			| FATTR4_WORD0_LEASE_TIME,
>  #ifdef CONFIG_PNFS
> -			FATTR4_WORD1_FS_LAYOUT_TYPES
> +			FATTR4_WORD1_FS_LAYOUT_TYPES,
> +			FATTR4_WORD2_LAYOUT_BLKSIZE
>  #else /* CONFIG_PNFS */
>  			0
>  #endif /* CONFIG_PNFS */
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index e45a2f7..5eab8bd 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -91,7 +91,7 @@ static int nfs4_stat_to_errno(int);
>  #define encode_getfh_maxsz      (op_encode_hdr_maxsz)
>  #define decode_getfh_maxsz      (op_decode_hdr_maxsz + 1 + \
>  				((3+NFS4_FHSIZE) >> 2))
> -#define nfs4_fattr_bitmap_maxsz 3
> +#define nfs4_fattr_bitmap_maxsz 4
>  #define encode_getattr_maxsz    (op_encode_hdr_maxsz + nfs4_fattr_bitmap_maxsz)
>  #define nfs4_name_maxsz		(1 + ((3 + NFS4_MAXNAMLEN) >> 2))
>  #define nfs4_path_maxsz		(1 + ((3 + NFS4_MAXPATHLEN) >> 2))
> @@ -113,7 +113,12 @@ static int nfs4_stat_to_errno(int);
>  #define encode_restorefh_maxsz  (op_encode_hdr_maxsz)
>  #define decode_restorefh_maxsz  (op_decode_hdr_maxsz)
>  #define encode_fsinfo_maxsz	(encode_getattr_maxsz)
> +#if !defined(CONFIG_PNFS)
>  #define decode_fsinfo_maxsz	(op_decode_hdr_maxsz + 11)
> +#else /* CONFIG_PNFS */
> +/* The size 15 assumes only a single layoutdriver is returned. */
> +#define decode_fsinfo_maxsz	(op_decode_hdr_maxsz + 15)
> +#endif /* CONFIG_PNFS */
>  #define encode_renew_maxsz	(op_encode_hdr_maxsz + 3)
>  #define decode_renew_maxsz	(op_decode_hdr_maxsz)
>  #define encode_setclientid_maxsz \
> @@ -1079,6 +1084,32 @@ static int encode_getattr_two(struct xdr_stream *xdr, uint32_t bm0, uint32_t bm1
>          return 0;
>  }
>  
> +static int encode_getattr_three(struct xdr_stream *xdr,
> +				uint32_t bm0, uint32_t bm1, uint32_t bm2)
> +{
> +	__be32 *p;
> +
> +	RESERVE_SPACE(4);
> +	WRITE32(OP_GETATTR);
> +	if (bm2) {
> +		RESERVE_SPACE(16);
> +		WRITE32(3);
> +		WRITE32(bm0);
> +		WRITE32(bm1);
> +		WRITE32(bm2);
> +	} else if (bm1) {
> +		RESERVE_SPACE(12);
> +		WRITE32(2);
> +		WRITE32(bm0);
> +		WRITE32(bm1);
> +	} else {
> +		RESERVE_SPACE(8);
> +		WRITE32(1);
> +		WRITE32(bm0);
> +	}
> +	return 0;
> +}
> +
>  static int encode_getfattr(struct xdr_stream *xdr, const u32* bitmask)
>  {
>  	return encode_getattr_two(xdr,
> @@ -1088,8 +1119,10 @@ static int encode_getfattr(struct xdr_stream *xdr, const u32* bitmask)
>  
>  static int encode_fsinfo(struct xdr_stream *xdr, const u32* bitmask)
>  {
> -	return encode_getattr_two(xdr, bitmask[0] & nfs4_fsinfo_bitmap[0],
> -			bitmask[1] & nfs4_fsinfo_bitmap[1]);
> +	return encode_getattr_three(xdr,
> +			bitmask[0] & nfs4_fsinfo_bitmap[0],
> +			bitmask[1] & nfs4_fsinfo_bitmap[1],
> +			bitmask[2] & nfs4_fsinfo_bitmap[2]);
>  }
>  
>  #ifdef CONFIG_PNFS
> @@ -3737,12 +3770,15 @@ static int decode_attr_bitmap(struct xdr_stream *xdr, uint32_t *bitmap)
>  	READ_BUF(4);
>  	READ32(bmlen);
>  
> -	bitmap[0] = bitmap[1] = 0;
> +	bitmap[0] = bitmap[1] = bitmap[2] = 0;

This is unsafe.  There are still several paths that allocate only a bitmap
of size 2 (decode_server_caps for example).

Either we scrub and move all call sites to 3 words long bitmaps
or we can pass the bitmap length to decode_attr_bitmap, which is safer
and will allow verifying it against bmlen.

Trond, mind looking at this too?
If this is the way to go, I'd like to send a preliminary
patch upstream that adds the bitmap size argument.

Benny

diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index e45a2f7..49ca008 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -3729,21 +3729,25 @@ static int decode_ace(struct xdr_stream *xdr, void *ace, struct nfs_client *clp)
 	return decode_opaque_inline(xdr, &strlen, &str);
 }
 
-static int decode_attr_bitmap(struct xdr_stream *xdr, uint32_t *bitmap)
+static int decode_attr_bitmap(struct xdr_stream *xdr, uint32_t *bitmap,
+			      int bmsize)
 {
-	uint32_t bmlen;
+	uint32_t bmlen, i;
 	__be32 *p;
 
 	READ_BUF(4);
 	READ32(bmlen);
 
-	bitmap[0] = bitmap[1] = 0;
+	if (bmlen > bmsize)
+		return -ETOOSMALL;
+
 	READ_BUF((bmlen << 2));
-	if (bmlen > 0) {
-		READ32(bitmap[0]);
-		if (bmlen > 1)
-			READ32(bitmap[1]);
-	}
+	for (i = 0; i < bmlen; i++)
+		READ32(bitmap[i]);
+
+	if (unlikely(bmlen < bmsize))
+		memset(bitmap + bmlen, 0, (bmsize - bmlen) * sizeof(*bitmap));
+
 	return 0;
 }
 
@@ -3757,13 +3761,14 @@ static inline int decode_attr_length(struct xdr_stream *xdr, uint32_t *attrlen,
 	return 0;
 }
 
-static int decode_attr_supported(struct xdr_stream *xdr, uint32_t *bitmap, uint32_t *bitmask)
+static int decode_attr_supported(struct xdr_stream *xdr, uint32_t *bitmap,
+				 uint32_t *bitmask, int bmsize)
 {
 	if (likely(bitmap[0] & FATTR4_WORD0_SUPPORTED_ATTRS)) {
-		decode_attr_bitmap(xdr, bitmask);
+		decode_attr_bitmap(xdr, bitmask, bmsize);
 		bitmap[0] &= ~FATTR4_WORD0_SUPPORTED_ATTRS;
 	} else
-		bitmask[0] = bitmask[1] = 0;
+		memset(bitmask, 0, bmsize * sizeof(*bitmask));
 	dprintk("%s: bitmask=%08x:%08x\n", __func__, bitmask[0], bitmask[1]);
 	return 0;
 }
@@ -4592,17 +4597,17 @@ static int decode_create(struct xdr_stream *xdr, struct nfs4_change_info *cinfo)
 static int decode_server_caps(struct xdr_stream *xdr, struct nfs4_server_caps_res *res)
 {
 	__be32 *savep;
-	uint32_t attrlen, 
-		 bitmap[2] = {0};
+	uint32_t attrlen, bitmap[2];
 	int status;
 
 	if ((status = decode_op_hdr(xdr, OP_GETATTR)) != 0)
 		goto xdr_error;
-	if ((status = decode_attr_bitmap(xdr, bitmap)) != 0)
+	if ((status = decode_attr_bitmap(xdr, bitmap, ARRAY_SIZE(bitmap))) != 0)
 		goto xdr_error;
 	if ((status = decode_attr_length(xdr, &attrlen, &savep)) != 0)
 		goto xdr_error;
-	if ((status = decode_attr_supported(xdr, bitmap, res->attr_bitmask)) != 0)
+	if ((status = decode_attr_supported(xdr, bitmap, res->attr_bitmask,
+					    ARRAY_SIZE(res->attr_bitmask))) != 0)
 		goto xdr_error;
 	if ((status = decode_attr_link_support(xdr, bitmap, &res->has_links)) != 0)
 		goto xdr_error;
@@ -4619,13 +4624,12 @@ xdr_error:
 static int decode_statfs(struct xdr_stream *xdr, struct nfs_fsstat *fsstat)
 {
 	__be32 *savep;
-	uint32_t attrlen, 
-		 bitmap[2] = {0};
+	uint32_t attrlen, bitmap[2];
 	int status;
 	
 	if ((status = decode_op_hdr(xdr, OP_GETATTR)) != 0)
 		goto xdr_error;
-	if ((status = decode_attr_bitmap(xdr, bitmap)) != 0)
+	if ((status = decode_attr_bitmap(xdr, bitmap, ARRAY_SIZE(bitmap))) != 0)
 		goto xdr_error;
 	if ((status = decode_attr_length(xdr, &attrlen, &savep)) != 0)
 		goto xdr_error;
@@ -4652,13 +4656,12 @@ xdr_error:
 static int decode_pathconf(struct xdr_stream *xdr, struct nfs_pathconf *pathconf)
 {
 	__be32 *savep;
-	uint32_t attrlen, 
-		 bitmap[2] = {0};
+	uint32_t attrlen, bitmap[2];
 	int status;
 	
 	if ((status = decode_op_hdr(xdr, OP_GETATTR)) != 0)
 		goto xdr_error;
-	if ((status = decode_attr_bitmap(xdr, bitmap)) != 0)
+	if ((status = decode_attr_bitmap(xdr, bitmap, ARRAY_SIZE(bitmap))) != 0)
 		goto xdr_error;
 	if ((status = decode_attr_length(xdr, &attrlen, &savep)) != 0)
 		goto xdr_error;
@@ -4677,15 +4680,13 @@ xdr_error:
 static int decode_getfattr(struct xdr_stream *xdr, struct nfs_fattr *fattr, const struct nfs_server *server)
 {
 	__be32 *savep;
-	uint32_t attrlen,
-		 bitmap[2] = {0},
-		 type;
+	uint32_t attrlen, bitmap[2], type;
 	int status, fmode = 0;
 	uint64_t fileid;
 
 	if ((status = decode_op_hdr(xdr, OP_GETATTR)) != 0)
 		goto xdr_error;
-	if ((status = decode_attr_bitmap(xdr, bitmap)) != 0)
+	if ((status = decode_attr_bitmap(xdr, bitmap, ARRAY_SIZE(bitmap))) != 0)
 		goto xdr_error;
 
 	fattr->bitmap[0] = bitmap[0];
@@ -4751,7 +4752,7 @@ static int decode_fsinfo(struct xdr_stream *xdr, struct nfs_fsinfo *fsinfo)
 
 	if ((status = decode_op_hdr(xdr, OP_GETATTR)) != 0)
 		goto xdr_error;
-	if ((status = decode_attr_bitmap(xdr, bitmap)) != 0)
+	if ((status = decode_attr_bitmap(xdr, bitmap, ARRAY_SIZE(bitmap))) != 0)
 		goto xdr_error;
 	if ((status = decode_attr_length(xdr, &attrlen, &savep)) != 0)
 		goto xdr_error;
@@ -5187,15 +5188,14 @@ static int decode_getacl(struct xdr_stream *xdr, struct rpc_rqst *req,
 		size_t *acl_len)
 {
 	__be32 *savep;
-	uint32_t attrlen,
-		 bitmap[2] = {0};
+	uint32_t attrlen, bitmap[2];
 	struct kvec *iov = req->rq_rcv_buf.head;
 	int status;
 
 	*acl_len = 0;
 	if ((status = decode_op_hdr(xdr, OP_GETATTR)) != 0)
 		goto out;
-	if ((status = decode_attr_bitmap(xdr, bitmap)) != 0)
+	if ((status = decode_attr_bitmap(xdr, bitmap, ARRAY_SIZE(bitmap))) != 0)
 		goto out;
 	if ((status = decode_attr_length(xdr, &attrlen, &savep)) != 0)
 		goto out;


More information about the pNFS mailing list