On Fri, Jun 14, 2024 at 6:18 AM Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > > 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. I will implement this change in the next step if no one else handles it. > > Sadly, I don't think coccinelle can do the kinds of transforms that > involve printf format strings. Yes, we need to carefully check them one by one. > > And no, a printk() string still couldn't use the locking version. > > Linus -- Regards Yafang
