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@xxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxx>
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
