Skip to content

Commit a48fc69

Browse files
committed
udf: Fix crash after seekdir
udf_readdir() didn't validate the directory position it should start reading from. Thus when user uses lseek(2) on directory file descriptor it can trick udf_readdir() into reading from a position in the middle of directory entry which then upsets directory parsing code resulting in errors or even possible kernel crashes. Similarly when the directory is modified between two readdir calls, the directory position need not be valid anymore. Add code to validate current offset in the directory. This is actually rather expensive for UDF as we need to read from the beginning of the directory and parse all directory entries. This is because in UDF a directory is just a stream of data containing directory entries and since file names are fully under user's control we cannot depend on detecting magic numbers and checksums in the header of directory entry as a malicious attacker could fake them. We skip this step if we detect that nothing changed since the last readdir call. Reported-by: Nathan Wilson <[email protected]> CC: [email protected] Signed-off-by: Jan Kara <[email protected]>
1 parent 6b75d88 commit a48fc69

File tree

3 files changed

+35
-2
lines changed

3 files changed

+35
-2
lines changed

fs/udf/dir.c

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include <linux/mm.h>
3232
#include <linux/slab.h>
3333
#include <linux/bio.h>
34+
#include <linux/iversion.h>
3435

3536
#include "udf_i.h"
3637
#include "udf_sb.h"
@@ -43,7 +44,7 @@ static int udf_readdir(struct file *file, struct dir_context *ctx)
4344
struct fileIdentDesc *fi = NULL;
4445
struct fileIdentDesc cfi;
4546
udf_pblk_t block, iblock;
46-
loff_t nf_pos;
47+
loff_t nf_pos, emit_pos = 0;
4748
int flen;
4849
unsigned char *fname = NULL, *copy_name = NULL;
4950
unsigned char *nameptr;
@@ -57,6 +58,7 @@ static int udf_readdir(struct file *file, struct dir_context *ctx)
5758
int i, num, ret = 0;
5859
struct extent_position epos = { NULL, 0, {0, 0} };
5960
struct super_block *sb = dir->i_sb;
61+
bool pos_valid = false;
6062

6163
if (ctx->pos == 0) {
6264
if (!dir_emit_dot(file, ctx))
@@ -67,6 +69,21 @@ static int udf_readdir(struct file *file, struct dir_context *ctx)
6769
if (nf_pos >= size)
6870
goto out;
6971

72+
/*
73+
* Something changed since last readdir (either lseek was called or dir
74+
* changed)? We need to verify the position correctly points at the
75+
* beginning of some dir entry so that the directory parsing code does
76+
* not get confused. Since UDF does not have any reliable way of
77+
* identifying beginning of dir entry (names are under user control),
78+
* we need to scan the directory from the beginning.
79+
*/
80+
if (!inode_eq_iversion(dir, file->f_version)) {
81+
emit_pos = nf_pos;
82+
nf_pos = 0;
83+
} else {
84+
pos_valid = true;
85+
}
86+
7087
fname = kmalloc(UDF_NAME_LEN, GFP_NOFS);
7188
if (!fname) {
7289
ret = -ENOMEM;
@@ -122,13 +139,21 @@ static int udf_readdir(struct file *file, struct dir_context *ctx)
122139

123140
while (nf_pos < size) {
124141
struct kernel_lb_addr tloc;
142+
loff_t cur_pos = nf_pos;
125143

126-
ctx->pos = (nf_pos >> 2) + 1;
144+
/* Update file position only if we got past the current one */
145+
if (nf_pos >= emit_pos) {
146+
ctx->pos = (nf_pos >> 2) + 1;
147+
pos_valid = true;
148+
}
127149

128150
fi = udf_fileident_read(dir, &nf_pos, &fibh, &cfi, &epos, &eloc,
129151
&elen, &offset);
130152
if (!fi)
131153
goto out;
154+
/* Still not at offset where user asked us to read from? */
155+
if (cur_pos < emit_pos)
156+
continue;
132157

133158
liu = le16_to_cpu(cfi.lengthOfImpUse);
134159
lfi = cfi.lengthFileIdent;
@@ -186,8 +211,11 @@ static int udf_readdir(struct file *file, struct dir_context *ctx)
186211
} /* end while */
187212

188213
ctx->pos = (nf_pos >> 2) + 1;
214+
pos_valid = true;
189215

190216
out:
217+
if (pos_valid)
218+
file->f_version = inode_query_iversion(dir);
191219
if (fibh.sbh != fibh.ebh)
192220
brelse(fibh.ebh);
193221
brelse(fibh.sbh);

fs/udf/namei.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include <linux/sched.h>
3131
#include <linux/crc-itu-t.h>
3232
#include <linux/exportfs.h>
33+
#include <linux/iversion.h>
3334

3435
static inline int udf_match(int len1, const unsigned char *name1, int len2,
3536
const unsigned char *name2)
@@ -134,6 +135,8 @@ int udf_write_fi(struct inode *inode, struct fileIdentDesc *cfi,
134135
mark_buffer_dirty_inode(fibh->ebh, inode);
135136
mark_buffer_dirty_inode(fibh->sbh, inode);
136137
}
138+
inode_inc_iversion(inode);
139+
137140
return 0;
138141
}
139142

fs/udf/super.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
#include <linux/crc-itu-t.h>
5858
#include <linux/log2.h>
5959
#include <asm/byteorder.h>
60+
#include <linux/iversion.h>
6061

6162
#include "udf_sb.h"
6263
#include "udf_i.h"
@@ -149,6 +150,7 @@ static struct inode *udf_alloc_inode(struct super_block *sb)
149150
init_rwsem(&ei->i_data_sem);
150151
ei->cached_extent.lstart = -1;
151152
spin_lock_init(&ei->i_extent_cache_lock);
153+
inode_set_iversion(&ei->vfs_inode, 1);
152154

153155
return &ei->vfs_inode;
154156
}

0 commit comments

Comments
 (0)