[Konsole-devel] [Bug 209544] Konsole crashes after resume from suspend to RAM [LinuxProcessInfo::readProcInfo , UnixProcessInfo::readProcessInfo, Session::updateForegroundProcessInfo]

[Török Edwin]

https://bugs.kde.org/show_bug.cgi?id=209544





--- Comment #2 from Török Edwin <edwintorok gmail com>  2009-10-06 09:00:38 ---
(In reply to comment #1)
> The backtrace looks related to bug 201119 / bug 195265.
> Thanks

I can't find a way to reproduce this, but I looked at the source code.
There is at least one race condition I spotted:


void UnixProcessInfo::readUserName()
{
    bool ok = false;
    int uid = userId(&ok);
    if (!ok) return;

    struct passwd *pwuser = getpwuid(uid);
    if (pwuser)
        setUserName(QString(pwuser->pw_name));
    else
        setUserName(QString());
}

getpwuid is not thread-safe, according to the manpage "The return value may
point to a static area, and may be overwritten by subsequent calls to 
getpwent(3),  getpwnam(),  or  getp‐wuid()."

So if there are multiple tabs in konsole open, each of them looking up the
username at the same time, they'll access the same buffer of getpwuid(),
and one of the threads may read the pw_name field while being updated, so it
could not contain proper NULL terminators, etc.

Another possible problem is that the process might die while reading it status
info:
        if ( statusInfo.open(QIODevice::ReadOnly) )
        {
            QTextStream stream(&statusInfo);
            QString data = stream.readAll();
            int uidIndex = data.indexOf("Uid:");
            QString uidLine = data.mid(uidIndex + 4, 16); // grab some data
            QString uidString = uidLine.split('\t',
QString::SkipEmptyParts)[0];

There is no check for uidIndex there, if the process happens to die, after you
opened the status file, and after a process dies there's no guarantee that
status contains correct info, I think the file is just truncated:
fs/proc/base.c:proc_single_show:
    task = get_pid_task(pid, PIDTYPE_PID);
    if (!task)
        return -ESRCH;

and in fs/seq_file.c:seq_read:
if (err) {
            /* With prejudice... */
            m->read_pos = 0;
            m->version = 0;
            m->index = 0;
            m->count = 0;
            goto Done;
}
....
while(1) {
     ....
     err = m->op->show(m, p);
     if (err < 0)
    break;
     ....
}
m->op->stop(m, p);
m->count = 0;
goto Done;

This suggests that the file is truncated during read() if the process dies
during that (or between the time open is called and read is called).

To sum up: use thread-safe functions instead of non-threadsafe ones (getpwuid_r
instead of getpwuid), and don't assume any file you read from /proc/<pid>/ is
exactly the format you expect it to be, since it can get truncated.

-- 
Configure bugmail: https://bugs.kde.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.