On Thu, 13 Jun 2024 at 14:14, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> The concept sounds a little strange. If some code takes a copy of a
> string while some other code is altering it, yes, the result will be a
> mess. This is why get_task_comm() exists, and why it uses locking.
The thing is, get_task_comm() is terminally broken.
Nobody sane uses it, and sometimes it's literally _because_ it uses locking.
Let's look at the numbers:
- 39 uses of get_task_comm()
- 2 uses of __get_task_comm() because the locking doesn't work
- 447 uses of raw "current->comm"
- 112 uses of raw 'ta*sk->comm' (and possibly
IOW, we need to just accept the fact that nobody actually wants to use
"get_task_comm()". It's a broken interface. It's inconvenient, and the
locking makes it worse.
Now, I'm not convinced that kstrdup() is what anybody should use
should, but of the 600 "raw" uses of ->comm, four of them do seem to
be kstrdup.
Not great, I think they could be removed, but they are examples of
people doing this. And I think it *would* be good to have the
guarantee that yes, the kstrdup() result is always a proper string,
even if it's used for unstable sources. Who knows what other unstable
sources exist?
I do suspect that most of the raw uses of 'xyz->comm' is for
printouts. And I think we would be better with a '%pTSK' vsnprintf()
format thing for that.
Sadly, I don't think coccinelle can do the kinds of transforms that
involve printf format strings.
And no, a printk() string still couldn't use the locking version.
Linus
