NULL dereference in tty_open()

NULL dereference in tty_open()

[Dan Carpenter]

There is a NULL dereference here.  It was artificially triggered so
not a huge priority.

drivers/tty/tty_io.c
  1893          retval = tty_add_file(tty, filp);
  1894          if (retval) {
  1895                  tty_unlock();
  1896                  tty_release(inode, filp);
  1897                  return retval;
  1898          }

tty_add_file() is supposed to setup filp->private_data but the
allocation fails.  In tty_release() we call file_tty(filp),
__tty_fasync() and tty_del_file() which dereference
filp->private_data and Oops.

I looked at ptmx_open() to see how the error handling was done there.
That function only calls tty_release() if tty_add_file() succeeds,
so maybe we could just call devpts_kill_index() here and remove the
tty_release()?  I don't know the code well enough to say.

regards,
dan carpenter

Re: NULL dereference in tty_open() [and other bugs there]

[Jiri Slaby]

On 10/04/2011 10:05 PM, Dan Carpenter wrote:
> There is a NULL dereference here.  It was artificially triggered so
> not a huge priority.
> 
> drivers/tty/tty_io.c
>   1893          retval = tty_add_file(tty, filp);
>   1894          if (retval) {
>   1895                  tty_unlock();
>   1896                  tty_release(inode, filp);
>   1897                  return retval;
>   1898          }
> 
> tty_add_file() is supposed to setup filp->private_data but the
> allocation fails.  In tty_release() we call file_tty(filp),
> __tty_fasync() and tty_del_file() which dereference
> filp->private_data and Oops.

Thanks, that's very true. It was added by:
commit 0259894c732837c801565d038eaecdcf8fc5bbe7
Author: Jiri Slaby <jslaby@suse.cz>
Date:   Wed Mar 23 10:48:37 2011 +0100

    TTY: fix fail path in tty_open
=====

So instead of leaks, we now crash, hmm.

> I looked at ptmx_open() to see how the error handling was done there.
> That function only calls tty_release() if tty_add_file() succeeds,
> so maybe we could just call devpts_kill_index() here and remove the
> tty_release()?  I don't know the code well enough to say.

No, that won't work. We need to revert all the changes done in
tty_reopen/tty_init_dev. Yes, ptmx_open looks broken to me too because
the tty is not properly freed.

What is worse, the tty_open code seems to be broken more than that.
* when tty_driver_lookup_tty fails in tty_open, we don't drop a
  reference to the tty driver.
* tty lookup looks broken for the current console. Look:

As of:
commit 4a2b5fddd53b80efcb3266ee36e23b8de28e761a
Author: Sukadev Bhattiprolu <sukadev@us.ibm.com>
Date:   Mon Oct 13 10:42:49 2008 +0100

    Move tty lookup/reopen to caller
=====

The code looks like:
struct tty_struct *tty = NULL;
...
if (device == MKDEV(TTYAUX_MAJOR, 0)) {
  tty = get_current_tty(); // ==== tty is not NULL
...
  tty_kref_put(tty); // ==== tty is dropped
  goto got_driver;
}
... // ============== POINT 1 (see below)
if (tty) {
  retval = tty_reopen(tty);
  ...
}
=====

Previously we called tty_driver_lookup_tty unconditionally at POINT 1.
Now we pass a tty structure to tty_reopen for which we dropped a
reference count. I don't think this was intentional, right?


Back to the point of your email. I have a patch which splits
tty_add_file into:
* tty_alloc_file intented to be called earlier in the open functions, so
we don't need to rollback
* tty_add_file which doesn't fail. It sets up the structure and adds it
to the list.

Otherwise we would need to split tty_release into smaller functions
somehow. We would need at least decreasing refcounts, checking tty count
for zero and freeing tty.

I will submit the simpler way above (tty_add_file split) along with
fixes for the other 3 bugs (ptmx_open, tty driver refcount, tty_reopen
of an unreferenced tty) after I test them all a bit.

thanks,
-- 
js
suse labs

[PATCH 1/4] TTY: drop driver reference in tty_open fail path

[Jiri Slaby]

When tty_driver_lookup_tty fails in tty_open, we forget to drop a
reference to the tty driver. This was added by commit 4a2b5fddd5 (Move
tty lookup/reopen to caller).

Fix that by adding tty_driver_kref_put to the fail path.

I will refactor the code later. This is for the ease of backporting to
stable.

Introduced-in: v2.6.28-rc2
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: stable@kernel.org
Cc: Sukadev Bhattiprolu <sukadev@us.ibm.com>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
---
 drivers/tty/tty_io.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 4f1fc81..54b3cc4 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1872,6 +1872,7 @@ got_driver:
 		if (IS_ERR(tty)) {
 			tty_unlock();
 			mutex_unlock(&tty_mutex);
+			tty_driver_kref_put(driver);
 			return PTR_ERR(tty);
 		}
 	}
-- 
1.7.7

[PATCH 2/4] TTY: make tty_add_file non-failing

[Jiri Slaby]

If tty_add_file fails at the point it is now, we have to revert all
the changes we did to the tty. It means either decrease all refcounts
if this was a tty reopen or delete the tty if it was newly allocated.

There was a try to fix this in v3.0-rc2 using tty_release in 0259894c7
(TTY: fix fail path in tty_open). But instead it introduced a NULL
dereference. It's because tty_release dereferences
filp->private_data, but that one is set even in our tty_add_file. And
when tty_add_file fails, it's still NULL/garbage. Hence tty_release
cannot be called there.

To circumvent the original leak (and the current NULL deref) we split
tty_add_file into two functions, making the latter non-failing. In
that case we may do the former early in open, where handling failures
is easy. The latter stays as it is now. So there is no change in
functionality.

The original bug (leak) was introduced by f573bd176 (tty: Remove
__GFP_NOFAIL from tty_add_file()). Thanks Dan for reporting this.

Later, we may split tty_release into more functions and call only some
of them in this fail path instead. (If at all possible.)

Introduced-in: v2.6.37-rc2
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Cc: stable@kernel.org
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Greg KH <gregkh@suse.de>
Cc: Pekka Enberg <penberg@kernel.org>
---
 drivers/tty/pty.c    |   16 +++++++++++-----
 drivers/tty/tty_io.c |   47 +++++++++++++++++++++++++++++++++++------------
 include/linux/tty.h  |    4 +++-
 3 files changed, 49 insertions(+), 18 deletions(-)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index e809e9d..696e851 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -670,12 +670,18 @@ static int ptmx_open(struct inode *inode, struct file *filp)
 
 	nonseekable_open(inode, filp);
 
+	retval = tty_alloc_file(filp);
+	if (retval)
+		return retval;
+
 	/* find a device that is not in use. */
 	tty_lock();
 	index = devpts_new_index(inode);
 	tty_unlock();
-	if (index < 0)
-		return index;
+	if (index < 0) {
+		retval = index;
+		goto err_file;
+	}
 
 	mutex_lock(&tty_mutex);
 	tty_lock();
@@ -689,9 +695,7 @@ static int ptmx_open(struct inode *inode, struct file *filp)
 
 	set_bit(TTY_PTY_LOCK, &tty->flags); /* LOCK THE SLAVE */
 
-	retval = tty_add_file(tty, filp);
-	if (retval)
-		goto out;
+	tty_add_file(tty, filp);
 
 	retval = devpts_pty_new(inode, tty->link);
 	if (retval)
@@ -710,6 +714,8 @@ out2:
 out:
 	devpts_kill_index(inode, index);
 	tty_unlock();
+err_file:
+	tty_free_file(filp);
 	return retval;
 }
 
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 54b3cc4..1a890e2 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -194,8 +194,7 @@ static inline struct tty_struct *file_tty(struct file *file)
 	return ((struct tty_file_private *)file->private_data)->tty;
 }
 
-/* Associate a new file with the tty structure */
-int tty_add_file(struct tty_struct *tty, struct file *file)
+int tty_alloc_file(struct file *file)
 {
 	struct tty_file_private *priv;
 
@@ -203,15 +202,36 @@ int tty_add_file(struct tty_struct *tty, struct file *file)
 	if (!priv)
 		return -ENOMEM;
 
+	file->private_data = priv;
+
+	return 0;
+}
+
+/* Associate a new file with the tty structure */
+void tty_add_file(struct tty_struct *tty, struct file *file)
+{
+	struct tty_file_private *priv = file->private_data;
+
 	priv->tty = tty;
 	priv->file = file;
-	file->private_data = priv;
 
 	spin_lock(&tty_files_lock);
 	list_add(&priv->list, &tty->tty_files);
 	spin_unlock(&tty_files_lock);
+}
 
-	return 0;
+/**
+ * tty_free_file - free file->private_data
+ *
+ * This shall be used only for fail path handling when tty_add_file was not
+ * called yet.
+ */
+void tty_free_file(struct file *file)
+{
+	struct tty_file_private *priv = file->private_data;
+
+	file->private_data = NULL;
+	kfree(priv);
 }
 
 /* Delete file from its tty */
@@ -222,8 +242,7 @@ void tty_del_file(struct file *file)
 	spin_lock(&tty_files_lock);
 	list_del(&priv->list);
 	spin_unlock(&tty_files_lock);
-	file->private_data = NULL;
-	kfree(priv);
+	tty_free_file(file);
 }
 
 
@@ -1811,6 +1830,10 @@ static int tty_open(struct inode *inode, struct file *filp)
 	nonseekable_open(inode, filp);
 
 retry_open:
+	retval = tty_alloc_file(filp);
+	if (retval)
+		return -ENOMEM;
+
 	noctty = filp->f_flags & O_NOCTTY;
 	index  = -1;
 	retval = 0;
@@ -1823,6 +1846,7 @@ retry_open:
 		if (!tty) {
 			tty_unlock();
 			mutex_unlock(&tty_mutex);
+			tty_free_file(filp);
 			return -ENXIO;
 		}
 		driver = tty_driver_kref_get(tty->driver);
@@ -1855,6 +1879,7 @@ retry_open:
 		}
 		tty_unlock();
 		mutex_unlock(&tty_mutex);
+		tty_free_file(filp);
 		return -ENODEV;
 	}
 
@@ -1862,6 +1887,7 @@ retry_open:
 	if (!driver) {
 		tty_unlock();
 		mutex_unlock(&tty_mutex);
+		tty_free_file(filp);
 		return -ENODEV;
 	}
 got_driver:
@@ -1873,6 +1899,7 @@ got_driver:
 			tty_unlock();
 			mutex_unlock(&tty_mutex);
 			tty_driver_kref_put(driver);
+			tty_free_file(filp);
 			return PTR_ERR(tty);
 		}
 	}
@@ -1888,15 +1915,11 @@ got_driver:
 	tty_driver_kref_put(driver);
 	if (IS_ERR(tty)) {
 		tty_unlock();
+		tty_free_file(filp);
 		return PTR_ERR(tty);
 	}
 
-	retval = tty_add_file(tty, filp);
-	if (retval) {
-		tty_unlock();
-		tty_release(inode, filp);
-		return retval;
-	}
+	tty_add_file(tty, filp);
 
 	check_tty_count(tty, "tty_open");
 	if (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 6af44ab..ac275c2 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -479,7 +479,9 @@ extern void proc_clear_tty(struct task_struct *p);
 extern struct tty_struct *get_current_tty(void);
 extern void tty_default_fops(struct file_operations *fops);
 extern struct tty_struct *alloc_tty_struct(void);
-extern int tty_add_file(struct tty_struct *tty, struct file *file);
+extern int tty_alloc_file(struct file *file);
+extern void tty_add_file(struct tty_struct *tty, struct file *file);
+extern void tty_free_file(struct file *file);
 extern void free_tty_struct(struct tty_struct *tty);
 extern void initialize_tty_struct(struct tty_struct *tty,
 		struct tty_driver *driver, int idx);
-- 
1.7.7

[PATCH 3/4] TTY: pty, release tty in all ptmx_open fail paths

[Jiri Slaby]

Mistakenly, commit 64ba3dc3143d (tty: never hold BTM while getting
tty_mutex) switched one fail path in ptmx_open to not free the newly
allocated tty.

Fix that by jumping to the appropriate place. And rename the labels so
that it's clear what is going on there.

Introduced-in: v2.6.36-rc2
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: stable@kernel.org
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
---
 drivers/tty/pty.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 696e851..e18604b 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -699,15 +699,15 @@ static int ptmx_open(struct inode *inode, struct file *filp)
 
 	retval = devpts_pty_new(inode, tty->link);
 	if (retval)
-		goto out1;
+		goto err_release;
 
 	retval = ptm_driver->ops->open(tty, filp);
 	if (retval)
-		goto out2;
-out1:
+		goto err_release;
+
 	tty_unlock();
-	return retval;
-out2:
+	return 0;
+err_release:
 	tty_unlock();
 	tty_release(inode, filp);
 	return retval;
-- 
1.7.7

Re: [PATCH 3/4] TTY: pty, release tty in all ptmx_open fail paths

[Arnd Bergmann]

On Wednesday 12 October 2011, Jiri Slaby wrote:
> Mistakenly, commit 64ba3dc3143d (tty: never hold BTM while getting
> tty_mutex) switched one fail path in ptmx_open to not free the newly
> allocated tty.
> 
> Fix that by jumping to the appropriate place. And rename the labels so
> that it's clear what is going on there.
> 
> Introduced-in: v2.6.36-rc2
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: stable@kernel.org
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
> Cc: Greg Kroah-Hartman <gregkh@suse.de>

Ok, I have checked that this patch returns the code into the state
before my broken patch, that looks good.

I think I made the same mistake trying to understand your patch
that I made originally when I introduced the bug, which was to
confuse the lifetime of the pty master with the lifetime of the
slave device, so I thought that tty_release should not be called
if we never called ptm_driver->ops->open(), but that's actually
a different device.

	Arnd

[PATCH 4/4] TTY: call tty_driver_lookup_tty unconditionally

[Jiri Slaby]

Commit 4a2b5fddd5 (Move tty lookup/reopen to caller) made the call to
tty_driver_lookup_tty conditional in tty_open. It doesn't look like it
was an intention. Or if it was, it was not documented in the changelog
and the code now looks weird. For example there would be no need to
remember the tty driver and tty index. Further the condition depends
on a tty which we drop a reference of already.

If I'm looking correctly, this should not matter thanks to the locking
currently done there. Thus, tty_driver->ttys[idx] cannot change under
our hands. But anyway, it makes sense to change that to the old
behaviour.

Introduced-in: v2.6.28-rc2
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Sukadev Bhattiprolu <sukadev@us.ibm.com>
Cc: Alan Cox <alan@redhat.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
---
 drivers/tty/tty_io.c |   21 +++++++++------------
 1 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 1a890e2..3283d0a 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1820,7 +1820,7 @@ int tty_release(struct inode *inode, struct file *filp)
 
 static int tty_open(struct inode *inode, struct file *filp)
 {
-	struct tty_struct *tty = NULL;
+	struct tty_struct *tty;
 	int noctty, retval;
 	struct tty_driver *driver;
 	int index;
@@ -1891,17 +1891,14 @@ retry_open:
 		return -ENODEV;
 	}
 got_driver:
-	if (!tty) {
-		/* check whether we're reopening an existing tty */
-		tty = tty_driver_lookup_tty(driver, inode, index);
-
-		if (IS_ERR(tty)) {
-			tty_unlock();
-			mutex_unlock(&tty_mutex);
-			tty_driver_kref_put(driver);
-			tty_free_file(filp);
-			return PTR_ERR(tty);
-		}
+	/* check whether we're reopening an existing tty */
+	tty = tty_driver_lookup_tty(driver, inode, index);
+	if (IS_ERR(tty)) {
+		tty_unlock();
+		mutex_unlock(&tty_mutex);
+		tty_driver_kref_put(driver);
+		tty_free_file(filp);
+		return PTR_ERR(tty);
 	}
 
 	if (tty) {
-- 
1.7.7

Re: [PATCH 4/4] TTY: call tty_driver_lookup_tty unconditionally

[Jiri Slaby]

On 10/12/2011 11:32 AM, Jiri Slaby wrote:
> Commit 4a2b5fddd5 (Move tty lookup/reopen to caller) made the call to
> tty_driver_lookup_tty conditional in tty_open. It doesn't look like it
> was an intention. Or if it was, it was not documented in the changelog
> and the code now looks weird. For example there would be no need to
> remember the tty driver and tty index. Further the condition depends
> on a tty which we drop a reference of already.
> 
> If I'm looking correctly, this should not matter thanks to the locking
> currently done there. Thus, tty_driver->ttys[idx] cannot change under
> our hands. But anyway, it makes sense to change that to the old
> behaviour.

Well, this doesn't work for ptys. devpts lookup code expects an inode to
be one of devpts filesystem (/dev/pts/*), not something on tmpfs like
/dev/tty. So it looks like the change was intentional, but very
undocumented and leaving there some unused code.

Now, I don't know what to do with the tty being around with a dropped
reference. Maybe nothing in the presence of the BTM which I *think*
prevents tty to go now.

Also I have just found that having ptm_unix98_lookup is pointless. For
master, we have ptmx_open which doesn't call lookup. And if it did, it
would trigger BUG_ON in devpts_get_tty called from ptm_unix98_lookup.

thanks,
-- 
js

Re: [PATCH 4/4] TTY: call tty_driver_lookup_tty unconditionally

[Sukadev Bhattiprolu]

Ccing H.Peter Anvin.

Jiri Slaby [jirislaby@gmail.com] wrote:
| On 10/12/2011 11:32 AM, Jiri Slaby wrote:
| > Commit 4a2b5fddd5 (Move tty lookup/reopen to caller) made the call to
| > tty_driver_lookup_tty conditional in tty_open. It doesn't look like it
| > was an intention. Or if it was, it was not documented in the changelog
| > and the code now looks weird. For example there would be no need to
| > remember the tty driver and tty index. Further the condition depends
| > on a tty which we drop a reference of already.
| > 
| > If I'm looking correctly, this should not matter thanks to the locking
| > currently done there. Thus, tty_driver->ttys[idx] cannot change under
| > our hands. But anyway, it makes sense to change that to the old
| > behaviour.
| 
| Well, this doesn't work for ptys. devpts lookup code expects an inode to
| be one of devpts filesystem (/dev/pts/*), not something on tmpfs like
| /dev/tty. So it looks like the change was intentional, but very
| undocumented and leaving there some unused code.

Yes this was intentional - even though the tty_driver_lookup() was
unconditional in tty_init_dev() I believe it did not do anything useful
when called from ptmx_open(). ptmx_open() would be setting up a new pty and
the lookup would not find a tty.

Would the following change to tty_open() help ?

got_driver:
+	/* check if we are opening a new pty or reopening an existing tty */
        if (!tty) {
-	/* check whether we're reopening an existing tty */
                tty = tty_driver_lookup_tty(driver, inode, index);

I am not sure about the unused code you refer to above. Can you please
clarify ?

Sukadev

Re: [PATCH 4/4] TTY: call tty_driver_lookup_tty unconditionally

[Jiri Slaby]

Fixing Alan's address. Somehow I put there a RH (defunct) one.

On 10/16/2011 09:20 PM, Sukadev Bhattiprolu wrote:
> Jiri Slaby [jirislaby@gmail.com] wrote:
> | On 10/12/2011 11:32 AM, Jiri Slaby wrote:
> | > Commit 4a2b5fddd5 (Move tty lookup/reopen to caller) made the call to
> | > tty_driver_lookup_tty conditional in tty_open. It doesn't look like it
> | > was an intention. Or if it was, it was not documented in the changelog
> | > and the code now looks weird. For example there would be no need to
> | > remember the tty driver and tty index. Further the condition depends
> | > on a tty which we drop a reference of already.
> | > 
> | > If I'm looking correctly, this should not matter thanks to the locking
> | > currently done there. Thus, tty_driver->ttys[idx] cannot change under
> | > our hands. But anyway, it makes sense to change that to the old
> | > behaviour.
> | 
> | Well, this doesn't work for ptys. devpts lookup code expects an inode to
> | be one of devpts filesystem (/dev/pts/*), not something on tmpfs like
> | /dev/tty. So it looks like the change was intentional, but very
> | undocumented and leaving there some unused code.
> 
> Yes this was intentional - even though the tty_driver_lookup() was
> unconditional in tty_init_dev() I believe it did not do anything useful
> when called from ptmx_open(). ptmx_open() would be setting up a new pty and
> the lookup would not find a tty.

Yes, I'm not arguing against moving the code from tty_init_dev to
tty_open change. That is perfectly OK.

What I mind is that now we do:
=====
  tty = get_current_tty();
  if (!tty)
        return -ENXIO;
  driver = tty_driver_kref_get(tty->driver); /* ZZZ */
  index = tty->index;                        /* ZZZ */
...
  tty_kref_put(tty); /* XXX */
  goto got_driver;
...
got_driver:
  if (!tty) {   /* YYY */
  }
=====

But at the point of YYY, the tty may be invalid due to reference drop at
XXX. I *hope* it is not the case thanks to the current locking there
(namely BTM) but I'm really *not sure*. And if it is the case, there
should definitely be a note. (Or better the reference should be held
while necessary.)

> Would the following change to tty_open() help ?

No, it doesn't matter now. I would prefer a description of the change to
be a part of the commit log. And it missed such a notice.

> I am not sure about the unused code you refer to above. Can you please
> clarify ?

It is index and driver assigned in the branch above (see ZZZ). When we
have a live tty (which I'm not sure we have -- see above), it is
guaranteed that the driver is reff'ed. And we need neither driver nor
index when we have such a tty.

thanks,
-- 
js

Re: [PATCH 1/4] TTY: drop driver reference in tty_open fail path

[Sukadev Bhattiprolu]

Jiri Slaby [jslaby@suse.cz] wrote:
| When tty_driver_lookup_tty fails in tty_open, we forget to drop a
| reference to the tty driver. This was added by commit 4a2b5fddd5 (Move
| tty lookup/reopen to caller).
| 
| Fix that by adding tty_driver_kref_put to the fail path.

Arg. Sorry about that. Thanks for spotting/fixing it.

| 
| I will refactor the code later. This is for the ease of backporting to
| stable.
| 
| Introduced-in: v2.6.28-rc2
| Signed-off-by: Jiri Slaby <jslaby@suse.cz>
| Cc: stable@kernel.org
| Cc: Sukadev Bhattiprolu <sukadev@us.ibm.com>
| Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
| Cc: Greg Kroah-Hartman <gregkh@suse.de>

Acked-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>