Al Viro pointed me to the fact that '->readdir()' and '->llseek()' have no mutual exclusion, which means the 'ubifs_dir_llseek()' can be run while we are in the middle of 'ubifs_readdir()'. First of all, this means that 'file->private_data' can be freed while 'ubifs_readdir()' uses it. But this particular patch does not fix the problem. This patch is only a preparation, and the fix will follow next. In this patch we make 'ubifs_readdir()' stop using 'file->f_pos' directly, because 'file->f_pos' can be changed by '->llseek()' at any point. This may lead 'ubifs_readdir()' to returning inconsistent data: directory entry names may correspond to incorrect file positions. So here we introduce a local variable 'pos', read 'file->f_pose' once at very the beginning, and then stick to 'pos'. The result of this is that when 'ubifs_dir_llseek()' changes 'file->f_pos' while we are in the middle of 'ubifs_readdir()', the latter "wins".
Unified · Split
| @@ -349,15 +349,16 @@ static unsigned int vfs_dent_type(uint8_t type) · 348 unmodified lines | |||
| 349 | static int ubifs_readdir(struct file *file, void *dirent, filldir_t filldir) | 349 | static int ubifs_readdir(struct file *file, void *dirent, filldir_t filldir) |
| 350 | { | 350 | { |
| 351 | int err, over = 0; | 351 | int err, over = 0; |
| 352 | loff_t pos = file->f_pos; | ||
| 352 | struct qstr nm; | 353 | struct qstr nm; |
| 353 | union ubifs_key key; | 354 | union ubifs_key key; |
| 354 | struct ubifs_dent_node *dent; | 355 | struct ubifs_dent_node *dent; |
| 355 | struct inode *dir = file_inode(file); | 356 | struct inode *dir = file_inode(file); |
| 356 | struct ubifs_info *c = dir->i_sb->s_fs_info; | 357 | struct ubifs_info *c = dir->i_sb->s_fs_info; |
| 357 | 358 | ||
| 358 | dbg_gen("dir ino %lu, f_pos %#llx", dir->i_ino, file->f_pos); | 359 | dbg_gen("dir ino %lu, f_pos %#llx", dir->i_ino, pos); |
| 359 | 360 | ||
| 360 | if (file->f_pos > UBIFS_S_KEY_HASH_MASK || file->f_pos == 2) | 361 | if (pos > UBIFS_S_KEY_HASH_MASK || pos == 2) |
| 361 | /* | 362 | /* |
| 362 | * The directory was seek'ed to a senseless position or there | 363 | * The directory was seek'ed to a senseless position or there |
| 363 | * are no more entries. | 364 | * are no more entries. |
| @@ -365,15 +366,15 @@ static int ubifs_readdir(struct file *file, void *dirent, filldir_t filldir) · 1 unmodified line | |||
| 365 | return 0; | 366 | return 0; |
| 366 | 367 | ||
| 367 | /* File positions 0 and 1 correspond to "." and ".." */ | 368 | /* File positions 0 and 1 correspond to "." and ".." */ |
| 368 | if (file->f_pos == 0) { | 369 | if (pos == 0) { |
| 369 | ubifs_assert(!file->private_data); | 370 | ubifs_assert(!file->private_data); |
| 370 | over = filldir(dirent, ".", 1, 0, dir->i_ino, DT_DIR); | 371 | over = filldir(dirent, ".", 1, 0, dir->i_ino, DT_DIR); |
| 371 | if (over) | 372 | if (over) |
| 372 | return 0; | 373 | return 0; |
| 373 | file->f_pos = 1; | 374 | file->f_pos = pos = 1; |
| 374 | } | 375 | } |
| 375 | 376 | ||
| 376 | if (file->f_pos == 1) { | 377 | if (pos == 1) { |
| 377 | ubifs_assert(!file->private_data); | 378 | ubifs_assert(!file->private_data); |
| 378 | over = filldir(dirent, "..", 2, 1, | 379 | over = filldir(dirent, "..", 2, 1, |
| 379 | parent_ino(file->f_path.dentry), DT_DIR); | 380 | parent_ino(file->f_path.dentry), DT_DIR); |
| @@ -389,7 +390,7 @@ static int ubifs_readdir(struct file *file, void *dirent, filldir_t filldir) · 9 unmodified lines | |||
| 389 | goto out; | 390 | goto out; |
| 390 | } | 391 | } |
| 391 | 392 | ||
| 392 | file->f_pos = key_hash_flash(c, &dent->key); | 393 | file->f_pos = pos = key_hash_flash(c, &dent->key); |
| 393 | file->private_data = dent; | 394 | file->private_data = dent; |
| 394 | } | 395 | } |
| 395 | 396 | ||
| @@ -397,17 +398,16 @@ static int ubifs_readdir(struct file *file, void *dirent, filldir_t filldir) · 1 unmodified line | |||
| 397 | if (!dent) { | 398 | if (!dent) { |
| 398 | /* | 399 | /* |
| 399 | * The directory was seek'ed to and is now readdir'ed. | 400 | * The directory was seek'ed to and is now readdir'ed. |
| 400 | * Find the entry corresponding to @file->f_pos or the | 401 | * Find the entry corresponding to @pos or the closest one. |
| 401 | * closest one. | ||
| 402 | */ | 402 | */ |
| 403 | dent_key_init_hash(c, &key, dir->i_ino, file->f_pos); | 403 | dent_key_init_hash(c, &key, dir->i_ino, pos); |
| 404 | nm.name = NULL; | 404 | nm.name = NULL; |
| 405 | dent = ubifs_tnc_next_ent(c, &key, &nm); | 405 | dent = ubifs_tnc_next_ent(c, &key, &nm); |
| 406 | if (IS_ERR(dent)) { | 406 | if (IS_ERR(dent)) { |
| 407 | err = PTR_ERR(dent); | 407 | err = PTR_ERR(dent); |
| 408 | goto out; | 408 | goto out; |
| 409 | } | 409 | } |
| 410 | file->f_pos = key_hash_flash(c, &dent->key); | 410 | file->f_pos = pos = key_hash_flash(c, &dent->key); |
| 411 | file->private_data = dent; | 411 | file->private_data = dent; |
| 412 | } | 412 | } |
| 413 | 413 | ||
| @@ -419,7 +419,7 @@ static int ubifs_readdir(struct file *file, void *dirent, filldir_t filldir) · 5 unmodified lines | |||
| 419 | ubifs_inode(dir)->creat_sqnum); | 419 | ubifs_inode(dir)->creat_sqnum); |
| 420 | 420 | ||
| 421 | nm.len = le16_to_cpu(dent->nlen); | 421 | nm.len = le16_to_cpu(dent->nlen); |
| 422 | over = filldir(dirent, dent->name, nm.len, file->f_pos, | 422 | over = filldir(dirent, dent->name, nm.len, pos, |
| 423 | le64_to_cpu(dent->inum), | 423 | le64_to_cpu(dent->inum), |
| 424 | vfs_dent_type(dent->type)); | 424 | vfs_dent_type(dent->type)); |
| 425 | if (over) | 425 | if (over) |
| @@ -435,7 +435,7 @@ static int ubifs_readdir(struct file *file, void *dirent, filldir_t filldir) · 9 unmodified lines | |||
| 435 | } | 435 | } |
| 436 | 436 | ||
| 437 | kfree(file->private_data); | 437 | kfree(file->private_data); |
| 438 | file->f_pos = key_hash_flash(c, &dent->key); | 438 | file->f_pos = pos = key_hash_flash(c, &dent->key); |
| 439 | file->private_data = dent; | 439 | file->private_data = dent; |
| 440 | cond_resched(); | 440 | cond_resched(); |
| 441 | } | 441 | } |