All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Al Viro <viro@zeniv.linux.org.uk>,
	Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Subject: [ 20/26] UBIFS: prepare to fix a horrid bug
Date: Mon,  1 Jul 2013 13:10:15 -0700	[thread overview]
Message-ID: <20130701200732.168244578@linuxfoundation.org> (raw)
In-Reply-To: <20130701200729.872850414@linuxfoundation.org>

3.9-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

commit 33f1a63ae84dfd9ad298cf275b8f1887043ced36 upstream.

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".

Reported-by: Al Viro <viro@zeniv.linux.org.uk>
Tested-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 fs/ubifs/dir.c |   24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -349,15 +349,16 @@ static unsigned int vfs_dent_type(uint8_
 static int ubifs_readdir(struct file *file, void *dirent, filldir_t filldir)
 {
 	int err, over = 0;
+	loff_t pos = file->f_pos;
 	struct qstr nm;
 	union ubifs_key key;
 	struct ubifs_dent_node *dent;
 	struct inode *dir = file_inode(file);
 	struct ubifs_info *c = dir->i_sb->s_fs_info;
 
-	dbg_gen("dir ino %lu, f_pos %#llx", dir->i_ino, file->f_pos);
+	dbg_gen("dir ino %lu, f_pos %#llx", dir->i_ino, pos);
 
-	if (file->f_pos > UBIFS_S_KEY_HASH_MASK || file->f_pos == 2)
+	if (pos > UBIFS_S_KEY_HASH_MASK || pos == 2)
 		/*
 		 * The directory was seek'ed to a senseless position or there
 		 * are no more entries.
@@ -365,15 +366,15 @@ static int ubifs_readdir(struct file *fi
 		return 0;
 
 	/* File positions 0 and 1 correspond to "." and ".." */
-	if (file->f_pos == 0) {
+	if (pos == 0) {
 		ubifs_assert(!file->private_data);
 		over = filldir(dirent, ".", 1, 0, dir->i_ino, DT_DIR);
 		if (over)
 			return 0;
-		file->f_pos = 1;
+		file->f_pos = pos = 1;
 	}
 
-	if (file->f_pos == 1) {
+	if (pos == 1) {
 		ubifs_assert(!file->private_data);
 		over = filldir(dirent, "..", 2, 1,
 			       parent_ino(file->f_path.dentry), DT_DIR);
@@ -389,7 +390,7 @@ static int ubifs_readdir(struct file *fi
 			goto out;
 		}
 
-		file->f_pos = key_hash_flash(c, &dent->key);
+		file->f_pos = pos = key_hash_flash(c, &dent->key);
 		file->private_data = dent;
 	}
 
@@ -397,17 +398,16 @@ static int ubifs_readdir(struct file *fi
 	if (!dent) {
 		/*
 		 * The directory was seek'ed to and is now readdir'ed.
-		 * Find the entry corresponding to @file->f_pos or the
-		 * closest one.
+		 * Find the entry corresponding to @pos or the closest one.
 		 */
-		dent_key_init_hash(c, &key, dir->i_ino, file->f_pos);
+		dent_key_init_hash(c, &key, dir->i_ino, pos);
 		nm.name = NULL;
 		dent = ubifs_tnc_next_ent(c, &key, &nm);
 		if (IS_ERR(dent)) {
 			err = PTR_ERR(dent);
 			goto out;
 		}
-		file->f_pos = key_hash_flash(c, &dent->key);
+		file->f_pos = pos = key_hash_flash(c, &dent->key);
 		file->private_data = dent;
 	}
 
@@ -419,7 +419,7 @@ static int ubifs_readdir(struct file *fi
 			     ubifs_inode(dir)->creat_sqnum);
 
 		nm.len = le16_to_cpu(dent->nlen);
-		over = filldir(dirent, dent->name, nm.len, file->f_pos,
+		over = filldir(dirent, dent->name, nm.len, pos,
 			       le64_to_cpu(dent->inum),
 			       vfs_dent_type(dent->type));
 		if (over)
@@ -435,7 +435,7 @@ static int ubifs_readdir(struct file *fi
 		}
 
 		kfree(file->private_data);
-		file->f_pos = key_hash_flash(c, &dent->key);
+		file->f_pos = pos = key_hash_flash(c, &dent->key);
 		file->private_data = dent;
 		cond_resched();
 	}



  parent reply	other threads:[~2013-07-01 20:19 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-01 20:09 [ 00/26] 3.9.9-stable review Greg Kroah-Hartman
2013-07-01 20:09 ` [ 01/26] s390/ipl: Fix FCP WWPN and LUN format strings for read Greg Kroah-Hartman
2013-07-01 20:09 ` [ 02/26] ARM: 7755/1: handle user space mapped pages in flush_kernel_dcache_page Greg Kroah-Hartman
2013-07-01 20:09 ` [ 03/26] ARM: 7772/1: Fix missing flush_kernel_dcache_page() for noMMU Greg Kroah-Hartman
2013-07-01 20:09 ` [ 04/26] Bluetooth: Fix crash in l2cap_build_cmd() with small MTU Greg Kroah-Hartman
2013-07-01 20:10 ` [ 05/26] Bluetooth: Fix invalid length check in l2cap_information_rsp() Greg Kroah-Hartman
2013-07-01 20:10 ` [ 06/26] hw_breakpoint: Fix cpu check in task_bp_pinned(cpu) Greg Kroah-Hartman
2013-07-01 20:10 ` [ 07/26] hw_breakpoint: Use cpu_possible_mask in {reserve,release}_bp_slot() Greg Kroah-Hartman
2013-07-01 20:10 ` [ 08/26] ath9k_htc: Handle IDLE state transition properly Greg Kroah-Hartman
2013-07-01 20:10 ` [ 09/26] iwlwifi: dvm: fix chain noise calibration Greg Kroah-Hartman
2013-07-01 20:10 ` [ 10/26] s390/pci: Implement IRQ functions if !PCI Greg Kroah-Hartman
2013-07-01 20:10 ` [ 11/26] s390/irq: Only define synchronize_irq() on SMP Greg Kroah-Hartman
2013-07-01 20:10 ` [ 12/26] dlci: acquire rtnl_lock before calling __dev_get_by_name() Greg Kroah-Hartman
2013-07-01 20:10 ` [ 13/26] dlci: validate the net device in dlci_del() Greg Kroah-Hartman
2013-07-01 20:10 ` [ 14/26] net/tg3: Avoid delay during MMIO access Greg Kroah-Hartman
2013-07-01 20:10 ` [ 15/26] rt2800: fix RT5390 & RT3290 TX power settings regression Greg Kroah-Hartman
2013-07-01 20:10 ` [ 16/26] iommu/vt-d: add quirk for broken interrupt remapping on 55XX chipsets Greg Kroah-Hartman
2013-07-01 20:10 ` [ 17/26] perf: Disable monitoring on setuid processes for regular users Greg Kroah-Hartman
2013-07-01 20:10 ` [ 18/26] crypto: algboss - Hold ref count on larval Greg Kroah-Hartman
2013-07-01 20:10   ` Greg Kroah-Hartman
2013-07-01 20:10 ` [ 19/26] powerpc/eeh: Fix fetching bus for single-dev-PE Greg Kroah-Hartman
2013-07-01 20:10 ` Greg Kroah-Hartman [this message]
2013-07-01 20:10 ` [ 21/26] UBIFS: fix a horrid bug Greg Kroah-Hartman
2013-07-01 20:10 ` [ 22/26] libata-acpi: add back ACPI based hotplug functionality Greg Kroah-Hartman
2013-07-01 20:10 ` [ 23/26] of/base: release the node correctly in of_parse_phandle_with_args() Greg Kroah-Hartman
2013-07-01 20:10 ` [ 24/26] can: usb_8dev: unregister netdev before free()ing Greg Kroah-Hartman
2013-07-01 20:10 ` [ 25/26] mac80211: work around broken APs not including HT info Greg Kroah-Hartman
2013-07-01 20:10 ` [ 26/26] netfilter: nf_conntrack_ipv6: Plug sk_buff leak in fragment handling Greg Kroah-Hartman
2013-07-02 18:31 ` [ 00/26] 3.9.9-stable review Guenter Roeck
2013-07-02 18:57   ` Greg Kroah-Hartman
2013-07-02 18:47 ` Shuah Khan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130701200732.168244578@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=artem.bityutskiy@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.