Re: Subject: Re: NULL pointer in proc_pid_stat -- oops.

[Ricky Beam]

On Mon, 29 Mar 2004, OGAWA Hirofumi wrote:
>> >  if (task->tty) {
>> >   tty_pgrp = task->tty->pgrp;
>> >   tty_nr = new_encode_dev(tty_devnum(task->tty));
>> >  }
>> >
>> > Some place doesn't take the any lock for ->tty.
>> > I think we need to take the lock for ->tty.
>>
>> Probably this isn't the thing for 2.6.xx,
>
>Ah, sorry for confusing. This is 2.6.x (maybe also 2.4.x).
>
>e.g. the above take the task_lock(). But disassociate_ctty() just take
>read_lock(&tasklist_lock), etc. etc. So looks like racy.

Yes, there is a race condition.  I never chased it back to the specifics.
I just did this:
===== tty.h 1.23 vs 1.24 =====
--- 1.23/include/linux/tty.h    Wed Sep 24 02:15:15 2003
+++ 1.24/include/linux/tty.h    Wed Feb 11 18:29:20 2004
@@ -404,7 +404,16 @@

 static inline dev_t tty_devnum(struct tty_struct *tty)
 {
-       return MKDEV(tty->driver->major, tty->driver->minor_start) + tty->index;
+       int ret = 0;
+
+       if(!tty) {
+               printk(KERN_CRIT "tty_devnum(): NULL tty (%p)\n", tty);
+       } else if(!tty->driver) {
+               printk(KERN_CRIT "tty_devnum(): NULL tty->driver (%p)\n", tty->d
river);
+       } else
+       ret = MKDEV(tty->driver->major, tty->driver->minor_start) + tty->index;
+
+       return ret;
 }

 #endif /* __KERNEL__ */

We were seeing this with some application(s) that called the java keytool
a bit too often.  (We moved the calls to directly calling the keytool
classes.)

"Works for me" :-)

--Ricky


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/