Re: freevxfs: hp-ux support. ( 1cce17017970c07) patchset 3/4

Re: freevxfs: hp-ux support. ( 1cce17017970c07) patchset 3/4

[Krzysztof Błaszkowski]

refactoring and bug fixing of readdir() and find_entry().
I reckon that my choice of not calling d_add(dp, NULL) was not bad
because if accessing memory out of mapped page range does not happen
then nothing random will be added.

>From 353745b666f0e9e59dc5b759cfb73270d3b4901e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Krzysztof=20B=C5=82aszkowski?= <kb@sysmikro.com.pl>
Date: Fri, 10 Jun 2016 13:30:52 +0200
Subject: [PATCH 3/4] refactored readdir() and find_entry
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

this change fixes also a buffer overflow which was caused by
accessing address space beyond mapped page

Signed-off-by: Krzysztof Błaszkowski <kb@sysmikro.com.pl>
---
 fs/freevxfs/vxfs_lookup.c | 242 ++++++++++++++++++++++------------------------
 1 file changed, 114 insertions(+), 128 deletions(-)

diff --git a/fs/freevxfs/vxfs_lookup.c b/fs/freevxfs/vxfs_lookup.c
index f042caf..75ca587 100644
--- a/fs/freevxfs/vxfs_lookup.c
+++ b/fs/freevxfs/vxfs_lookup.c
@@ -62,35 +62,6 @@ const struct file_operations vxfs_dir_operations = {
 	.iterate_shared =	vxfs_readdir,
 };
 
-static inline u_long
-dir_blocks(struct inode *ip)
-{
-	u_long			bsize = ip->i_sb->s_blocksize;
-	return (ip->i_size + bsize - 1) & ~(bsize - 1);
-}
-
-/*
- * NOTE! unlike strncmp, vxfs_match returns 1 for success, 0 for failure.
- *
- * len <= VXFS_NAMELEN and de != NULL are guaranteed by caller.
- */
-static inline int
-vxfs_match(struct vxfs_sb_info *sbi, int len, const char *const name,
-		struct vxfs_direct *de)
-{
-	if (len != fs16_to_cpu(sbi, de->d_namelen))
-		return 0;
-	if (!de->d_ino)
-		return 0;
-	return !memcmp(name, de->d_name, len);
-}
-
-static inline struct vxfs_direct *
-vxfs_next_entry(struct vxfs_sb_info *sbi, struct vxfs_direct *de)
-{
-	return ((struct vxfs_direct *)
-		((char *)de + fs16_to_cpu(sbi, de->d_reclen)));
-}
 
 /**
  * vxfs_find_entry - find a mathing directory entry for a dentry
@@ -109,53 +80,65 @@ vxfs_next_entry(struct vxfs_sb_info *sbi, struct vxfs_direct *de)
 static struct vxfs_direct *
 vxfs_find_entry(struct inode *ip, struct dentry *dp, struct page **ppp)
 {
-	struct vxfs_sb_info		*sbi = VXFS_SBI(ip->i_sb);
-	u_long				npages, page, nblocks, pblocks, block;
-	u_long				bsize = ip->i_sb->s_blocksize;
-	const char			*name = dp->d_name.name;
-	int				namelen = dp->d_name.len;
-
-	npages = dir_pages(ip);
-	nblocks = dir_blocks(ip);
-	pblocks = VXFS_BLOCK_PER_PAGE(ip->i_sb);
-	
-	for (page = 0; page < npages; page++) {
-		caddr_t			kaddr;
-		struct page		*pp;
-
-		pp = vxfs_get_page(ip->i_mapping, page);
-		if (IS_ERR(pp))
-			continue;
-		kaddr = (caddr_t)page_address(pp);
-
-		for (block = 0; block <= nblocks && block <= pblocks; block++) {
-			caddr_t			baddr, limit;
-			struct vxfs_dirblk	*dbp;
-			struct vxfs_direct	*de;
-
-			baddr = kaddr + (block * bsize);
-			limit = baddr + bsize - VXFS_DIRLEN(1);
-			
-			dbp = (struct vxfs_dirblk *)baddr;
-			de = (struct vxfs_direct *)
-				(baddr + VXFS_DIRBLKOV(sbi, dbp));
-
-			for (; (caddr_t)de <= limit;
-					de = vxfs_next_entry(sbi, de)) {
-				if (!de->d_reclen)
-					break;
-				if (!de->d_ino)
-					continue;
-				if (vxfs_match(sbi, namelen, name, de)) {
-					*ppp = pp;
-					return (de);
-				}
+	u_long bsize = ip->i_sb->s_blocksize;
+	const char *name = dp->d_name.name;
+	int namelen = dp->d_name.len;
+	loff_t limit = VXFS_DIRROUND(ip->i_size);
+	struct vxfs_direct *de_exit = NULL;
+	loff_t pos = 0;
+	struct vxfs_sb_info *sbi = VXFS_SBI(ip->i_sb);
+
+	while (pos < limit) {
+		struct page *pp;
+		char *kaddr;
+		int pg_ofs = pos & ~PAGE_MASK;
+
+		pp = vxfs_get_page(ip->i_mapping, pos >> PAGE_SHIFT);
+		if (IS_ERR(pp)) {
+			return NULL;
+		}
+		kaddr = (char *)page_address(pp);
+
+		while (pg_ofs < PAGE_SIZE && pos < limit) {
+			struct vxfs_direct *de;
+
+			if ((pos & (bsize - 1)) < 4) {
+				struct vxfs_dirblk *dbp =
+				    (struct vxfs_dirblk *)(kaddr + (pos & ~PAGE_MASK));
+				int overhead = (sizeof(short) * fs16_to_cpu(sbi, dbp->d_nhash)) + 4;
+
+				pos += overhead;
+				pg_ofs += overhead;
+			}
+			de = (struct vxfs_direct *)(kaddr + pg_ofs);
+
+			if (!de->d_reclen) {
+				pos += bsize - 1;
+				pos &= ~(bsize - 1);
+				break;
+			}
+
+			pg_ofs += fs16_to_cpu(sbi, de->d_reclen);
+			pos += fs16_to_cpu(sbi, de->d_reclen);
+			if (!de->d_ino) {
+				continue;
+			}
+
+			if (namelen != fs16_to_cpu(sbi, de->d_namelen))
+				continue;
+			if (!memcmp(name, de->d_name, namelen)) {
+				*ppp = pp;
+				de_exit = de;
+				break;
 			}
 		}
-		vxfs_put_page(pp);
+		if (!de_exit)
+			vxfs_put_page(pp);
+		else
+			break;
 	}
 
-	return NULL;
+	return de_exit;
 }
 
 /**
@@ -238,80 +221,83 @@ vxfs_readdir(struct file *fp, struct dir_context *ctx)
 {
 	struct inode		*ip = file_inode(fp);
 	struct super_block	*sbp = ip->i_sb;
-	struct vxfs_sb_info	*sbi = VXFS_SBI(sbp);
 	u_long			bsize = sbp->s_blocksize;
-	u_long			page, npages, block, pblocks, nblocks, offset;
-	loff_t			pos;
-
+	loff_t			pos, limit;
+	struct vxfs_sb_info	*sbi = VXFS_SBI(sbp);
 
 	if (ctx->pos == 0) {
 		if (!dir_emit_dot(fp, ctx))
-			return 0;
-		ctx->pos = 1;
+			goto out;
+		ctx->pos++;
 	}
 	if (ctx->pos == 1) {
 		if (!dir_emit(ctx, "..", 2, VXFS_INO(ip)->vii_dotdot, DT_DIR))
-			return 0;
-		ctx->pos = 2;
+			goto out;
+		ctx->pos++;
 	}
-	pos = ctx->pos - 2;
-	
-	if (pos > VXFS_DIRROUND(ip->i_size))
-		return 0;
 
-	npages = dir_pages(ip);
-	nblocks = dir_blocks(ip);
-	pblocks = VXFS_BLOCK_PER_PAGE(sbp);
+	limit = VXFS_DIRROUND(ip->i_size);
+	if (ctx->pos > limit) {
+#if 0
+		ctx->pos = 0;
+#endif
+		goto out;
+	}
 
-	page = pos >> PAGE_SHIFT;
-	offset = pos & ~PAGE_MASK;
-	block = (u_long)(pos >> sbp->s_blocksize_bits) % pblocks;
+	pos = ctx->pos & ~3L;
 
-	for (; page < npages; page++, block = 0) {
-		char			*kaddr;
-		struct page		*pp;
+	while (pos < limit) {
+		struct page *pp;
+		char *kaddr;
+		int pg_ofs = pos & ~PAGE_MASK;
+		int rc = 0;
 
-		pp = vxfs_get_page(ip->i_mapping, page);
-		if (IS_ERR(pp))
-			continue;
+		pp = vxfs_get_page(ip->i_mapping, pos >> PAGE_SHIFT);
+		if (IS_ERR(pp)) {
+			return -ENOMEM;
+		}
 		kaddr = (char *)page_address(pp);
 
-		for (; block <= nblocks && block <= pblocks; block++) {
-			char			*baddr, *limit;
-			struct vxfs_dirblk	*dbp;
-			struct vxfs_direct	*de;
+		while (pg_ofs < PAGE_SIZE && pos < limit) {
+			struct vxfs_direct *de;
 
-			baddr = kaddr + (block * bsize);
-			limit = baddr + bsize - VXFS_DIRLEN(1);
-	
-			dbp = (struct vxfs_dirblk *)baddr;
-			de = (struct vxfs_direct *)
-				(offset ?
-				 (kaddr + offset) :
-				 (baddr + VXFS_DIRBLKOV(sbi, dbp)));
-
-			for (; (char *)de <= limit;
-					de = vxfs_next_entry(sbi, de)) {
-				if (!de->d_reclen)
-					break;
-				if (!de->d_ino)
-					continue;
-
-				offset = (char *)de - kaddr;
-				ctx->pos = ((page << PAGE_SHIFT) | offset) + 2;
-				if (!dir_emit(ctx, de->d_name,
-						fs16_to_cpu(sbi, de->d_namelen),
-						fs32_to_cpu(sbi, de->d_ino),
-						DT_UNKNOWN)) {
-					vxfs_put_page(pp);
-					return 0;
-				}
+			if ((pos & (bsize - 1)) < 4) {
+				struct vxfs_dirblk *dbp =
+				    (struct vxfs_dirblk *)(kaddr + (pos & ~PAGE_MASK));
+				int overhead = (sizeof(short) * fs16_to_cpu(sbi, dbp->d_nhash)) + 4;
+
+				pos += overhead;
+				pg_ofs += overhead;
+			}
+			de = (struct vxfs_direct *)(kaddr + pg_ofs);
+
+			if (!de->d_reclen) {
+				pos += bsize - 1;
+				pos &= ~(bsize - 1);
+				break;
+			}
+
+			pg_ofs += fs16_to_cpu(sbi, de->d_reclen);
+			pos += fs16_to_cpu(sbi, de->d_reclen);
+			if (!de->d_ino) {
+				continue;
+			}
+
+			rc = dir_emit(ctx, de->d_name, fs16_to_cpu(sbi, de->d_namelen),
+				fs32_to_cpu(sbi, de->d_ino), DT_UNKNOWN);
+			if (!rc) {
+				/* the dir entry was not submitted, so fix pos. */
+				pos -= fs16_to_cpu(sbi, de->d_reclen);
+				break;
 			}
-			offset = 0;
 		}
 		vxfs_put_page(pp);
-		offset = 0;
+		if (!rc)
+			break;
 	}
-	ctx->pos = ((page << PAGE_SHIFT) | offset) + 2;
+
+	ctx->pos = pos | 2;
+
+out:
 	return 0;
 }
-- 
2.8.3





-- 
Krzysztof Blaszkowski