From 5d2aa711ec2478aae4cd2c8400b387097ec69dd9 Mon Sep 17 00:00:00 2001 From: Wedson Almeida Filho Date: Sat, 12 Aug 2023 15:31:47 -0300 Subject: [PATCH 1/7] tarfs: keep ctx->pos up to date because dir_emit uses it If we don't keep it up to date, d_off will be incorrect, so callers will seek to the wrong position if they ever try to use d_off. Signed-off-by: Wedson Almeida Filho --- src/tarfs/tarfs.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/tarfs/tarfs.c b/src/tarfs/tarfs.c index bd13021e45b8..0b9aae7e7de0 100644 --- a/src/tarfs/tarfs.c +++ b/src/tarfs/tarfs.c @@ -98,10 +98,9 @@ static int tarfs_readdir(struct file *file, struct dir_context *ctx) int ret = 0; char *name_buffer = NULL; u64 name_len = 0; - u64 cur = ctx->pos; u64 size = i_size_read(inode) / sizeof(disk_dentry) * sizeof(disk_dentry); - /* cur must be aligned to a directory entry. */ + /* ctx->pos must be aligned to a directory entry. */ if (ctx->pos % sizeof(struct tarfs_direntry)) return -ENOENT; @@ -109,15 +108,15 @@ static int tarfs_readdir(struct file *file, struct dir_context *ctx) if (offset + size < offset) return -ERANGE; - /* Make sure the increment of cur won't overflow by limiting size. */ + /* Make sure the increment of ctx->pos won't overflow by limiting size. */ if (size >= U64_MAX - sizeof(disk_dentry)) return -ERANGE; - for (cur = ctx->pos; cur < size; cur += sizeof(disk_dentry)) { + for (; ctx->pos < size; ctx->pos += sizeof(disk_dentry)) { u64 disk_len; u8 type; - ret = tarfs_dev_read(inode->i_sb, offset + cur, &disk_dentry, sizeof(disk_dentry)); + ret = tarfs_dev_read(inode->i_sb, offset + ctx->pos, &disk_dentry, sizeof(disk_dentry)); if (ret) break; @@ -162,10 +161,6 @@ static int tarfs_readdir(struct file *file, struct dir_context *ctx) } kfree(name_buffer); - - if (!ret) - ctx->pos = cur; - return ret; } From 3f07137bdfa8e2920ae080a8b8e64654d2ea88ad Mon Sep 17 00:00:00 2001 From: Wedson Almeida Filho Date: Sat, 12 Aug 2023 15:38:52 -0300 Subject: [PATCH 2/7] tarfs: don't fail if at least one directory was emitted If we run into an error while listing the contents of a directory, don't fail the whole request if we already emitted at least one entry. Signed-off-by: Wedson Almeida Filho --- src/tarfs/tarfs.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/tarfs/tarfs.c b/src/tarfs/tarfs.c index 0b9aae7e7de0..fea07ed45c9f 100644 --- a/src/tarfs/tarfs.c +++ b/src/tarfs/tarfs.c @@ -99,6 +99,7 @@ static int tarfs_readdir(struct file *file, struct dir_context *ctx) char *name_buffer = NULL; u64 name_len = 0; u64 size = i_size_read(inode) / sizeof(disk_dentry) * sizeof(disk_dentry); + loff_t orig_pos = ctx->pos; /* ctx->pos must be aligned to a directory entry. */ if (ctx->pos % sizeof(struct tarfs_direntry)) @@ -123,13 +124,18 @@ static int tarfs_readdir(struct file *file, struct dir_context *ctx) disk_len = le64_to_cpu(disk_dentry.namelen); if (disk_len > name_len) { kfree(name_buffer); + name_buffer = NULL; - if (disk_len > SIZE_MAX) - return -ENOMEM; + if (disk_len > SIZE_MAX) { + ret = -ENOMEM; + break; + } name_buffer = kmalloc(disk_len, GFP_KERNEL); - if (!name_buffer) - return -ENOMEM; + if (!name_buffer) { + ret = -ENOMEM; + break; + } name_len = disk_len; } @@ -161,7 +167,7 @@ static int tarfs_readdir(struct file *file, struct dir_context *ctx) } kfree(name_buffer); - return ret; + return ctx->pos != orig_pos ? 0 : ret; } static int tarfs_readpage(struct file *file, struct page *page) From 68b255c1f45ec05119575f21a37db5326c833a3d Mon Sep 17 00:00:00 2001 From: Wedson Almeida Filho Date: Mon, 14 Aug 2023 14:39:54 -0300 Subject: [PATCH 3/7] tarfs: unlock the page (and set the error flag) on failure to map it Without this, the lock would stay locked indefinitely. Signed-off-by: Wedson Almeida Filho --- src/tarfs/tarfs.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/tarfs/tarfs.c b/src/tarfs/tarfs.c index fea07ed45c9f..7fac60244037 100644 --- a/src/tarfs/tarfs.c +++ b/src/tarfs/tarfs.c @@ -179,8 +179,11 @@ static int tarfs_readpage(struct file *file, struct page *page) int ret; buf = kmap_local_page(page); - if (!buf) + if (!buf) { + SetPageError(page); + unlock_page(page); return -ENOMEM; + } offset = page_offset(page); size = i_size_read(inode); From 4ebef3f6a82d6f89238d17f03612f4243c1c0394 Mon Sep 17 00:00:00 2001 From: Wedson Almeida Filho Date: Thu, 17 Aug 2023 01:40:40 -0300 Subject: [PATCH 4/7] tarfs: don't issue bdev read requests for bad offsets These checks will never trigger for valid images but they are useful when dealing with corrupted or malicious images: although not strictly necessary as block devices should reject such read attempts, having them is a good security-in-depth approach because it protects the block layer. Signed-off-by: Wedson Almeida Filho --- src/tarfs/tarfs.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/tarfs/tarfs.c b/src/tarfs/tarfs.c index 7fac60244037..6d9f53aa0c57 100644 --- a/src/tarfs/tarfs.c +++ b/src/tarfs/tarfs.c @@ -339,15 +339,20 @@ static struct inode *tarfs_iget(struct super_block *sb, u64 ino) return ERR_PTR(ret); } -static int tarfs_strcmp(struct super_block *sb, unsigned long pos, - const char *str, size_t size) +static int tarfs_strcmp(struct super_block *sb, u64 pos, const char *str, + size_t size) { struct buffer_head *bh; unsigned long offset; size_t segment; bool matched; + const struct tarfs_state *state = sb->s_fs_info; + + /* If the string doesn't fit in the data size, it doesn't match. */ + if (pos + size < pos || pos + size > state->data_size) + return 0; - /* compare string up to a block at a time. */ + /* Compare string up to a block at a time. */ while (size) { offset = pos & (TARFS_BSIZE - 1); segment = min_t(size_t, size, TARFS_BSIZE - offset); From c0492c2af2d70f5d61a25866a5ea49a9758b9e13 Mon Sep 17 00:00:00 2001 From: Wedson Almeida Filho Date: Thu, 17 Aug 2023 01:54:35 -0300 Subject: [PATCH 5/7] tarfs: call iget_failed on failure post iget_locked This the right function to call on failure because it unhashes the inode, which will cause any waiters (e.g., callers of ilookup or iget_locked) to retry after waiting for inode init to complete. Signed-off-by: Wedson Almeida Filho --- src/tarfs/tarfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tarfs/tarfs.c b/src/tarfs/tarfs.c index 6d9f53aa0c57..924df1ceeb1b 100644 --- a/src/tarfs/tarfs.c +++ b/src/tarfs/tarfs.c @@ -335,7 +335,7 @@ static struct inode *tarfs_iget(struct super_block *sb, u64 ino) return inode; discard: - discard_new_inode(inode); + iget_failed(inode); return ERR_PTR(ret); } From 2a17d18f27ca321f5a6b0a014a6148fb533a8163 Mon Sep 17 00:00:00 2001 From: Wedson Almeida Filho Date: Thu, 7 Sep 2023 15:54:05 -0300 Subject: [PATCH 6/7] tarfs: ensure the underlying block device can hold at least one block This check ensures that the expression below to determine the superblock position won't underflow. Signed-off-by: Wedson Almeida Filho --- src/tarfs/tarfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tarfs/tarfs.c b/src/tarfs/tarfs.c index 924df1ceeb1b..147a0106dc35 100644 --- a/src/tarfs/tarfs.c +++ b/src/tarfs/tarfs.c @@ -523,7 +523,7 @@ static int tarfs_fill_super(struct super_block *sb, struct fs_context *fc) sb->s_xattr = xattr_handlers; scount = bdev_nr_sectors(sb->s_bdev); - if (!scount) + if (scount < TARFS_BSIZE / SECTOR_SIZE) return -ENXIO; state = kmalloc(sizeof(*state), GFP_KERNEL); From 14e22b6d577425c2edc1318d77dd44828e9fec62 Mon Sep 17 00:00:00 2001 From: Wedson Almeida Filho Date: Fri, 9 Feb 2024 10:57:25 -0300 Subject: [PATCH 7/7] tarfs: use GPF_NOFS when allocating memory on a mounted fs When a mounted filesystem needs memory that isn't available, this flag instructs the kernel allocator to not request (and wait for) file systems to shrink caches because it may lead to deadlocks. Signed-off-by: Wedson Almeida Filho --- src/tarfs/tarfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tarfs/tarfs.c b/src/tarfs/tarfs.c index 147a0106dc35..92de8bcc779d 100644 --- a/src/tarfs/tarfs.c +++ b/src/tarfs/tarfs.c @@ -131,7 +131,7 @@ static int tarfs_readdir(struct file *file, struct dir_context *ctx) break; } - name_buffer = kmalloc(disk_len, GFP_KERNEL); + name_buffer = kmalloc(disk_len, GFP_NOFS); if (!name_buffer) { ret = -ENOMEM; break; @@ -457,7 +457,7 @@ static struct inode *tarfs_alloc_inode(struct super_block *sb) { struct tarfs_inode_info *info; - info = alloc_inode_sb(sb, tarfs_inode_cachep, GFP_KERNEL); + info = alloc_inode_sb(sb, tarfs_inode_cachep, GFP_NOFS); if (!info) return NULL;