On Mon, Jun 3, 2024 at 10:32 AM Axel Rasmussen <axelrasmussen@xxxxxxxxxx> wrote: > > On Mon, Jun 3, 2024 at 5:33 AM Geunsik Lim <geunsik.lim@xxxxxxxxx> wrote: > > > > This commit addresses a potential memory leak in the > > `get_mm_memcg_path()` function > > by explicitly checking if the allocated buffer (`buf`) is NULL before > > calling the > > `css_put()` function. The prefix 'css' means abbreviation of cgroup_subsys_state > > > > Previously, the code would directly call `css_put()` without checking > > the value of > > `buf`, which could lead to a memory leak if the buffer allocation failed. > > This commit introduces a conditional check to ensure that `css_put()` > > is only called > > if `buf` is not NULL. > > > > This change enhances the code's robustness and prevents memory leaks, improving > > overall system stability. > > > > **Specific Changes:** > > > > * In the `out_put` label, an `if` statement is added to check > > if `buf` is not NULL before calling `css_put()`. > > > > **Benefits:** > > > > * Prevents potential memory leaks > > * Enhances code robustness > > * Improves system stability > > > > Signed-off-by: Geunsik Lim <leemgs@xxxxxxxxx> > > Signed-off-by: Geunsik Lim <geunsik.lim@xxxxxxxxxxx> > > --- > > mm/mmap_lock.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/mm/mmap_lock.c b/mm/mmap_lock.c > > index 1854850b4b89..7314045b0e3b 100644 > > --- a/mm/mmap_lock.c > > +++ b/mm/mmap_lock.c > > @@ -213,7 +213,8 @@ static const char *get_mm_memcg_path(struct mm_struct *mm) > > cgroup_path(memcg->css.cgroup, buf, MEMCG_PATH_BUF_SIZE); > > > > out_put: > > - css_put(&memcg->css); > > + if (buf != NULL) > > + css_put(&memcg->css); > > out: > > return buf; > > } > > I think the existing code is correct, and this change actually > introduces a memory leak where there was none before. > > In the case where get_memcg_path_buf() returns NULL, we *still* need > to css_put() what we got from get_mem_cgroup_from_mm() before. > > NAK, unless I'm missing something. +1 We already skip css_put() if get_mem_cgroup_from_mm() returns NULL. Whether or not get_memcg_path_buf() succeeds is irrelevant here.
