Skip to content

Commit bc4ef75

Browse files
kdavemasoncl
authored andcommitted
btrfs: properly set the termination value of ctx->pos in readdir
The value of ctx->pos in the last readdir call is supposed to be set to INT_MAX due to 32bit compatibility, unless 'pos' is intentially set to a larger value, then it's LLONG_MAX. There's a report from PaX SIZE_OVERFLOW plugin that "ctx->pos++" overflows (https://forums.grsecurity.net/viewtopic.php?f=1&t=4284), on a 64bit arch, where the value is 0x7fffffffffffffff ie. LLONG_MAX before the increment. We can get to that situation like that: * emit all regular readdir entries * still in the same call to readdir, bump the last pos to INT_MAX * next call to readdir will not emit any entries, but will reach the bump code again, finds pos to be INT_MAX and sets it to LLONG_MAX Normally this is not a problem, but if we call readdir again, we'll find 'pos' set to LLONG_MAX and the unconditional increment will overflow. The report from Victor at (http://thread.gmane.org/gmane.comp.file-systems.btrfs/49500) with debugging print shows that pattern: Overflow: e Overflow: 7fffffff Overflow: 7fffffffffffffff PAX: size overflow detected in function btrfs_real_readdir fs/btrfs/inode.c:5760 cicus.935_282 max, count: 9, decl: pos; num: 0; context: dir_context; CPU: 0 PID: 2630 Comm: polkitd Not tainted 4.2.3-grsec #1 Hardware name: Gigabyte Technology Co., Ltd. H81ND2H/H81ND2H, BIOS F3 08/11/2015 ffffffff81901608 0000000000000000 ffffffff819015e6 ffffc90004973d48 ffffffff81742f0f 0000000000000007 ffffffff81901608 ffffc90004973d78 ffffffff811cb706 0000000000000000 ffff8800d47359e0 ffffc90004973ed8 Call Trace: [<ffffffff81742f0f>] dump_stack+0x4c/0x7f [<ffffffff811cb706>] report_size_overflow+0x36/0x40 [<ffffffff812ef0bc>] btrfs_real_readdir+0x69c/0x6d0 [<ffffffff811dafc8>] iterate_dir+0xa8/0x150 [<ffffffff811e6d8d>] ? __fget_light+0x2d/0x70 [<ffffffff811dba3a>] SyS_getdents+0xba/0x1c0 Overflow: 1a [<ffffffff811db070>] ? iterate_dir+0x150/0x150 [<ffffffff81749b69>] entry_SYSCALL_64_fastpath+0x12/0x83 The jump from 7fffffff to 7fffffffffffffff happens when new dir entries are not yet synced and are processed from the delayed list. Then the code could go to the bump section again even though it might not emit any new dir entries from the delayed list. The fix avoids entering the "bump" section again once we've finished emitting the entries, both for synced and delayed entries. References: https://forums.grsecurity.net/viewtopic.php?f=1&t=4284 Reported-by: Victor <[email protected]> CC: [email protected] Signed-off-by: David Sterba <[email protected]> Tested-by: Holger Hoffstätte <[email protected]> Signed-off-by: Chris Mason <[email protected]>
1 parent 43d871f commit bc4ef75

File tree

3 files changed

+16
-3
lines changed

3 files changed

+16
-3
lines changed

fs/btrfs/delayed-inode.c

+2-1
Original file line numberDiff line numberDiff line change
@@ -1689,7 +1689,7 @@ int btrfs_should_delete_dir_index(struct list_head *del_list,
16891689
*
16901690
*/
16911691
int btrfs_readdir_delayed_dir_index(struct dir_context *ctx,
1692-
struct list_head *ins_list)
1692+
struct list_head *ins_list, bool *emitted)
16931693
{
16941694
struct btrfs_dir_item *di;
16951695
struct btrfs_delayed_item *curr, *next;
@@ -1733,6 +1733,7 @@ int btrfs_readdir_delayed_dir_index(struct dir_context *ctx,
17331733

17341734
if (over)
17351735
return 1;
1736+
*emitted = true;
17361737
}
17371738
return 0;
17381739
}

fs/btrfs/delayed-inode.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ void btrfs_put_delayed_items(struct list_head *ins_list,
144144
int btrfs_should_delete_dir_index(struct list_head *del_list,
145145
u64 index);
146146
int btrfs_readdir_delayed_dir_index(struct dir_context *ctx,
147-
struct list_head *ins_list);
147+
struct list_head *ins_list, bool *emitted);
148148

149149
/* for init */
150150
int __init btrfs_delayed_inode_init(void);

fs/btrfs/inode.c

+13-1
Original file line numberDiff line numberDiff line change
@@ -5716,6 +5716,7 @@ static int btrfs_real_readdir(struct file *file, struct dir_context *ctx)
57165716
char *name_ptr;
57175717
int name_len;
57185718
int is_curr = 0; /* ctx->pos points to the current index? */
5719+
bool emitted;
57195720

57205721
/* FIXME, use a real flag for deciding about the key type */
57215722
if (root->fs_info->tree_root == root)
@@ -5744,6 +5745,7 @@ static int btrfs_real_readdir(struct file *file, struct dir_context *ctx)
57445745
if (ret < 0)
57455746
goto err;
57465747

5748+
emitted = false;
57475749
while (1) {
57485750
leaf = path->nodes[0];
57495751
slot = path->slots[0];
@@ -5823,6 +5825,7 @@ static int btrfs_real_readdir(struct file *file, struct dir_context *ctx)
58235825

58245826
if (over)
58255827
goto nopos;
5828+
emitted = true;
58265829
di_len = btrfs_dir_name_len(leaf, di) +
58275830
btrfs_dir_data_len(leaf, di) + sizeof(*di);
58285831
di_cur += di_len;
@@ -5835,11 +5838,20 @@ static int btrfs_real_readdir(struct file *file, struct dir_context *ctx)
58355838
if (key_type == BTRFS_DIR_INDEX_KEY) {
58365839
if (is_curr)
58375840
ctx->pos++;
5838-
ret = btrfs_readdir_delayed_dir_index(ctx, &ins_list);
5841+
ret = btrfs_readdir_delayed_dir_index(ctx, &ins_list, &emitted);
58395842
if (ret)
58405843
goto nopos;
58415844
}
58425845

5846+
/*
5847+
* If we haven't emitted any dir entry, we must not touch ctx->pos as
5848+
* it was was set to the termination value in previous call. We assume
5849+
* that "." and ".." were emitted if we reach this point and set the
5850+
* termination value as well for an empty directory.
5851+
*/
5852+
if (ctx->pos > 2 && !emitted)
5853+
goto nopos;
5854+
58435855
/* Reached end of directory/root. Bump pos past the last item. */
58445856
ctx->pos++;
58455857

0 commit comments

Comments
 (0)