[PATCH v7 5/8] mm/util: Fix possible race condition in kstrdup()
Yafang Shao
laoar.shao at gmail.com
Fri Sep 27 08:57:26 UTC 2024
On Fri, Sep 27, 2024 at 1:35 AM Andy Shevchenko
<andy.shevchenko at gmail.com> wrote:
>
> On Thu, Sep 26, 2024 at 7:44 PM Yafang Shao <laoar.shao at gmail.com> wrote:
> >
> > In kstrdup(), it is critical to ensure that the dest string is always
> > NUL-terminated. However, potential race condidtion can occur between a
>
> condition
>
> > writer and a reader.
> >
> > Consider the following scenario involving task->comm:
> >
> > reader writer
> >
> > len = strlen(s) + 1;
> > strlcpy(tsk->comm, buf, sizeof(tsk->comm));
> > memcpy(buf, s, len);
> >
> > In this case, there is a race condition between the reader and the
> > writer. The reader calculate the length of the string `s` based on the
>
> calculates
>
> > old value of task->comm. However, during the memcpy(), the string `s`
> > might be updated by the writer to a new value of task->comm.
> >
> > If the new task->comm is larger than the old one, the `buf` might not be
> > NUL-terminated. This can lead to undefined behavior and potential
> > security vulnerabilities.
> >
> > Let's fix it by explicitly adding a NUL-terminator.
>
> memcpy() is not atomic AFAIK, meaning that the new string can be also
> shorter and when memcpy() already copied past the new NUL. I would
> amend the explanation to include this as well.
>
> ...
>
> > + /* During memcpy(), the string might be updated to a new value,
> > + * which could be longer than the string when strlen() is
> > + * called. Therefore, we need to add a null termimator.
>
> /*
> * The wrong comment style. Besides that a typo
> * in the word 'terminator'. Please, run codespell on your changes.
> * Also use the same form: NUL-terminator when you are talking
> * about '\0' and not NULL.
> */
Thank you for pointing out these errors and for recommending the use
of codespell.
Will fix them in the next version.
--
Regards
Yafang
More information about the dri-devel
mailing list