[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALOAHbCrZp2XV_zp0-mH2frW2Fk15Tz-A9J0K6gcJTbSXvTsPg@mail.gmail.com>
Date: Fri, 14 Jun 2024 10:41:34 +0800
From: Yafang Shao <laoar.shao@...il.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>, ebiederm@...ssion.com,
alexei.starovoitov@...il.com, rostedt@...dmis.org, linux-mm@...ck.org,
linux-fsdevel@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
audit@...r.kernel.org, linux-security-module@...r.kernel.org,
selinux@...r.kernel.org, bpf@...r.kernel.org, netdev@...r.kernel.org,
dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v2 05/10] mm/util: Fix possible race condition in kstrdup()
On Fri, Jun 14, 2024 at 6:18 AM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> On Thu, 13 Jun 2024 at 14:14, Andrew Morton <akpm@...ux-foundation.org> 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
Powered by blists - more mailing lists