linux-kernel - Re: [PATCH] lib/vsprintf: add %pT[C012] format specifier

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20131227150211.61328c02d61722cb212649e9@linux-foundation.org>
Date:	Fri, 27 Dec 2013 15:02:11 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Cc:	jkosina@...e.cz, joe@...ches.com, viro@...iv.linux.org.uk,
	davem@...emloft.net, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] lib/vsprintf: add %pT[C012] format specifier

On Wed, 25 Dec 2013 21:37:33 +0900 Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp> wrote:

> Some examples for converting direct ->comm users are shown below.
> 
>   pr_info("comm=%s\n", p->comm);                => pr_info("comm=%pTC\n", p);
>   pr_info("%s[%u]\n", p->comm, p->pid);         => pr_info("%pT0\n", p);
>   pr_info("%s[%u]\n", p->comm, task_pid_nr(p)); => pr_info("%pT0\n", p);
>   pr_info("%s/%u\n", p->comm, p->pid);          => pr_info("%pT1\n", p);
>   pr_info("%s,%u\n", p->comm, p->pid);          => pr_info("%pT2\n", p);

Places where one task accesses a different tasks's ->comm are rare, and
those places damn well better have a lot of locking in place -
otherwise they are racy against much more serious things than prctl().

The vast majority of ->comm accesses are accessing current->comm, for
debug reasons.  I'm counting 350-odd sites.  At all these sites it's
pointless passing `current' to the printk function at all!

I wonder if there's some way in which we can invent a vsprintf token
which means "insert corrent->comm here" and which doesn't require that
the caller pass in the additional argument?

That being said.....

current->comm isn't a terribly good way of identifying a task - it's
unaware of namespaces, is non-unique, userspace can overwrite ->comm[]
to anything it wants and evil users can probably write silly stuff into
->comm[] to confuse sysadmin tools.

So perhaps the world would be a better place if we were to invent a
standard kernel-wide way of identifying a process within a debug
printk.  Presumably it would include ->comm and the pid, but other
things can be added later if needed.

So a usage site would look like:

	pr_warn("%s: hair on fire\n", this_task_id());

but we need storage so it's really

	char b[THIS_TASK_ID_SIZE];

	pr_warn("%s: hair on fire\n", this_task_id(b));

which is painful, so we also provide the new vsprintf token as a
convenience:

	pr_warn("%|: hair on fire\n");

but I don't know what we can use in place of %|.

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