On Fri, Sep 27, 2024 at 1:35 AM Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote: > > On Thu, Sep 26, 2024 at 7:44 PM Yafang Shao <laoar.shao@xxxxxxxxx> 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
