public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 2/4] mm/damon/sysfs: check contexts->nr before clear_schemes_tried_regions
       [not found] <20260319155742.186627-3-objecting@objecting.org>
@ 2026-03-20  2:13 ` SeongJae Park
  2026-03-20  7:06   ` Josh Law
  0 siblings, 1 reply; 6+ messages in thread
From: SeongJae Park @ 2026-03-20  2:13 UTC (permalink / raw)
  To: Josh Law; +Cc: SeongJae Park, akpm, damon, linux-mm, linux-kernel, stable

On Thu, 19 Mar 2026 15:57:40 +0000 Josh Law <objecting@objecting.org> wrote:

> The CLEAR_SCHEMES_TRIED_REGIONS command accesses contexts_arr[0]
> without verifying nr_contexts >= 1, causing a NULL pointer dereference
> when no context is configured. Add the missing check.

Nice catch, thank you!

Privileged users can trigger this using DAMON sysfs interface.  E.g.,

    # cd /sys/kernel/mm/damon/admin/kdamonds/
    # echo 1 > nr_kdamonds
    # echo clear_schemes_tried_regions > state
    killed
    # dmesg
    [...]
    [63541.377604] BUG: kernel NULL pointer dereference, address: 0000000000000000
    [...]

Privileged users can do anything even worse than this, but they might also do
this by a mistake.

So this deserves Fixes: and Cc stable.

>
> Signed-off-by: Josh Law <objecting@objecting.org>
> ---
>  mm/damon/sysfs.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> index b573b9d60784..36ad2e8956c9 100644
> --- a/mm/damon/sysfs.c
> +++ b/mm/damon/sysfs.c
> @@ -1769,6 +1769,8 @@ static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
>       case DAMON_SYSFS_CMD_UPDATE_SCHEMES_TRIED_REGIONS:
>               return damon_sysfs_update_schemes_tried_regions(kdamond, false);
>       case DAMON_SYSFS_CMD_CLEAR_SCHEMES_TRIED_REGIONS:
> +             if (kdamond->contexts->nr != 1)
> +                     return -EINVAL;
>               return damon_sysfs_schemes_clear_regions(
>                       kdamond->contexts->contexts_arr[0]->schemes);
>       case DAMON_SYSFS_CMD_UPDATE_SCHEMES_EFFECTIVE_QUOTAS:
> --
> 2.34.1

So this patch looks good as an individual fix for the individual bug, but...

Sashiko commented.

# review url: https://sashiko.dev/#/patchset/20260319155742.186627-3-objecting@objecting.org

: Does this missing check also affect other manual commands?
:
: If a user writes UPDATE_SCHEMES_STATS, UPDATE_SCHEMES_EFFECTIVE_QUOTAS,
: or UPDATE_TUNED_INTERVALS to the state file after setting nr_contexts
: to 0, damon_sysfs_handle_cmd() queues the corresponding callback via
: damon_sysfs_damon_call().
:
: When the kdamond thread executes the callback, it appears functions like
: damon_sysfs_upd_schemes_stats() access contexts_arr[0] without verifying
: contexts->nr:
:
: static int damon_sysfs_upd_schemes_stats(void *data)
: {
:         struct damon_sysfs_kdamond *kdamond = data;
:         struct damon_ctx *ctx = kdamond->damon_ctx;
:
:         damon_sysfs_schemes_update_stats(
:                         kdamond->contexts->contexts_arr[0]->schemes, ctx);
:         return 0;
: }
:
: Could this result in a similar NULL pointer dereference if these commands
: are triggered while no context is configured?

Sashiko is correct.  Privileged users can trigger the issues like below.

# damo start
# cd /sys/kernel/mm/damon/admin/kdamonds/0
# echo 0 > contexts/nr_contexts
# echo update_schemes_stats > state
# echo update_schemes_effective_quotas > state
# echo update_tuned_intervals > state

Not necessarily blocker of this patch, but seems all the issues are in a same
category.  The third patch of this series is also fixing one of the category
bugs.  How about fixing all at once by checking kdamond->contexts->nr at the
beginning of damon_sysfs_handle_cmd(), like below?

--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -2404,6 +2404,9 @@ static int damon_sysfs_update_schemes_tried_regions(
 static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
                struct damon_sysfs_kdamond *kdamond)
 {
+       if (cmd != DAMON_SYSFS_CMD_OFF && kdamond->contexts->nr != 1)
+               return -EINVAL;
+
        switch (cmd) {
        case DAMON_SYSFS_CMD_ON:
                return damon_sysfs_turn_damon_on(kdamond);

If we pick this, Fixes: would be deserve to the oldest buggy commit that
introduced the first bug of this category.  It is indeed quite old.

Fixes: 0ac32b8affb5 ("mm/damon/sysfs: support DAMOS stats")
Cc: <stable@vger.kernel.org> # 5.18.x


Thanks,
SJ

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/4] mm/damon/sysfs: check contexts->nr before clear_schemes_tried_regions
  2026-03-20  2:13 ` [PATCH 2/4] mm/damon/sysfs: check contexts->nr before clear_schemes_tried_regions SeongJae Park
@ 2026-03-20  7:06   ` Josh Law
  2026-03-20 14:47     ` SeongJae Park
  0 siblings, 1 reply; 6+ messages in thread
From: Josh Law @ 2026-03-20  7:06 UTC (permalink / raw)
  To: SeongJae Park; +Cc: akpm, damon, linux-mm, linux-kernel, stable



On 20 March 2026 02:13:17 GMT, SeongJae Park <sj@kernel.org> wrote:
>On Thu, 19 Mar 2026 15:57:40 +0000 Josh Law <objecting@objecting.org> wrote:
>
>> The CLEAR_SCHEMES_TRIED_REGIONS command accesses contexts_arr[0]
>> without verifying nr_contexts >= 1, causing a NULL pointer dereference
>> when no context is configured. Add the missing check.
>
>Nice catch, thank you!
>
>Privileged users can trigger this using DAMON sysfs interface.  E.g.,
>
>    # cd /sys/kernel/mm/damon/admin/kdamonds/
>    # echo 1 > nr_kdamonds
>    # echo clear_schemes_tried_regions > state
>    killed
>    # dmesg
>    [...]
>    [63541.377604] BUG: kernel NULL pointer dereference, address: 0000000000000000
>    [...]
>
>Privileged users can do anything even worse than this, but they might also do
>this by a mistake.
>
>So this deserves Fixes: and Cc stable.
>
>>
>> Signed-off-by: Josh Law <objecting@objecting.org>
>> ---
>>  mm/damon/sysfs.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
>> index b573b9d60784..36ad2e8956c9 100644
>> --- a/mm/damon/sysfs.c
>> +++ b/mm/damon/sysfs.c
>> @@ -1769,6 +1769,8 @@ static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
>>       case DAMON_SYSFS_CMD_UPDATE_SCHEMES_TRIED_REGIONS:
>>               return damon_sysfs_update_schemes_tried_regions(kdamond, false);
>>       case DAMON_SYSFS_CMD_CLEAR_SCHEMES_TRIED_REGIONS:
>> +             if (kdamond->contexts->nr != 1)
>> +                     return -EINVAL;
>>               return damon_sysfs_schemes_clear_regions(
>>                       kdamond->contexts->contexts_arr[0]->schemes);
>>       case DAMON_SYSFS_CMD_UPDATE_SCHEMES_EFFECTIVE_QUOTAS:
>> --
>> 2.34.1
>
>So this patch looks good as an individual fix for the individual bug, but...
>
>Sashiko commented.
>
># review url: https://sashiko.dev/#/patchset/20260319155742.186627-3-objecting@objecting.org
>
>: Does this missing check also affect other manual commands?
>:
>: If a user writes UPDATE_SCHEMES_STATS, UPDATE_SCHEMES_EFFECTIVE_QUOTAS,
>: or UPDATE_TUNED_INTERVALS to the state file after setting nr_contexts
>: to 0, damon_sysfs_handle_cmd() queues the corresponding callback via
>: damon_sysfs_damon_call().
>:
>: When the kdamond thread executes the callback, it appears functions like
>: damon_sysfs_upd_schemes_stats() access contexts_arr[0] without verifying
>: contexts->nr:
>:
>: static int damon_sysfs_upd_schemes_stats(void *data)
>: {
>:         struct damon_sysfs_kdamond *kdamond = data;
>:         struct damon_ctx *ctx = kdamond->damon_ctx;
>:
>:         damon_sysfs_schemes_update_stats(
>:                         kdamond->contexts->contexts_arr[0]->schemes, ctx);
>:         return 0;
>: }
>:
>: Could this result in a similar NULL pointer dereference if these commands
>: are triggered while no context is configured?
>
>Sashiko is correct.  Privileged users can trigger the issues like below.
>
># damo start
># cd /sys/kernel/mm/damon/admin/kdamonds/0
># echo 0 > contexts/nr_contexts
># echo update_schemes_stats > state
># echo update_schemes_effective_quotas > state
># echo update_tuned_intervals > state
>
>Not necessarily blocker of this patch, but seems all the issues are in a same
>category.  The third patch of this series is also fixing one of the category
>bugs.  How about fixing all at once by checking kdamond->contexts->nr at the
>beginning of damon_sysfs_handle_cmd(), like below?
>
>--- a/mm/damon/sysfs.c
>+++ b/mm/damon/sysfs.c
>@@ -2404,6 +2404,9 @@ static int damon_sysfs_update_schemes_tried_regions(
> static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
>                struct damon_sysfs_kdamond *kdamond)
> {
>+       if (cmd != DAMON_SYSFS_CMD_OFF && kdamond->contexts->nr != 1)
>+               return -EINVAL;
>+
>        switch (cmd) {
>        case DAMON_SYSFS_CMD_ON:
>                return damon_sysfs_turn_damon_on(kdamond);
>
>If we pick this, Fixes: would be deserve to the oldest buggy commit that
>introduced the first bug of this category.  It is indeed quite old.
>
>Fixes: 0ac32b8affb5 ("mm/damon/sysfs: support DAMOS stats")
>Cc: <stable@vger.kernel.org> # 5.18.x
>
>
>Thanks,
>SJ



Hello, did you give Reviewed by you? Or not..

V/R


Josh Law

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/4] mm/damon/sysfs: check contexts->nr before clear_schemes_tried_regions
  2026-03-20  7:06   ` Josh Law
@ 2026-03-20 14:47     ` SeongJae Park
  2026-03-20 15:14       ` Josh Law
  0 siblings, 1 reply; 6+ messages in thread
From: SeongJae Park @ 2026-03-20 14:47 UTC (permalink / raw)
  To: Josh Law; +Cc: SeongJae Park, akpm, damon, linux-mm, linux-kernel, stable

On Fri, 20 Mar 2026 07:06:48 +0000 Josh Law <objecting@objecting.org> wrote:

> 
> 
> On 20 March 2026 02:13:17 GMT, SeongJae Park <sj@kernel.org> wrote:
> >On Thu, 19 Mar 2026 15:57:40 +0000 Josh Law <objecting@objecting.org> wrote:
> >
> >> The CLEAR_SCHEMES_TRIED_REGIONS command accesses contexts_arr[0]
> >> without verifying nr_contexts >= 1, causing a NULL pointer dereference
> >> when no context is configured. Add the missing check.
> >
> >Nice catch, thank you!
> >
> >Privileged users can trigger this using DAMON sysfs interface.  E.g.,
> >
> >    # cd /sys/kernel/mm/damon/admin/kdamonds/
> >    # echo 1 > nr_kdamonds
> >    # echo clear_schemes_tried_regions > state
> >    killed
> >    # dmesg
> >    [...]
> >    [63541.377604] BUG: kernel NULL pointer dereference, address: 0000000000000000
> >    [...]
> >
> >Privileged users can do anything even worse than this, but they might also do
> >this by a mistake.
> >
> >So this deserves Fixes: and Cc stable.
> >
> >>
> >> Signed-off-by: Josh Law <objecting@objecting.org>
> >> ---
> >>  mm/damon/sysfs.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> >> index b573b9d60784..36ad2e8956c9 100644
> >> --- a/mm/damon/sysfs.c
> >> +++ b/mm/damon/sysfs.c
> >> @@ -1769,6 +1769,8 @@ static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
> >>       case DAMON_SYSFS_CMD_UPDATE_SCHEMES_TRIED_REGIONS:
> >>               return damon_sysfs_update_schemes_tried_regions(kdamond, false);
> >>       case DAMON_SYSFS_CMD_CLEAR_SCHEMES_TRIED_REGIONS:
> >> +             if (kdamond->contexts->nr != 1)
> >> +                     return -EINVAL;
> >>               return damon_sysfs_schemes_clear_regions(
> >>                       kdamond->contexts->contexts_arr[0]->schemes);
> >>       case DAMON_SYSFS_CMD_UPDATE_SCHEMES_EFFECTIVE_QUOTAS:
> >> --
> >> 2.34.1
> >
> >So this patch looks good as an individual fix for the individual bug, but...
> >
> >Sashiko commented.
> >
> ># review url: https://sashiko.dev/#/patchset/20260319155742.186627-3-objecting@objecting.org
> >
> >: Does this missing check also affect other manual commands?
> >:
> >: If a user writes UPDATE_SCHEMES_STATS, UPDATE_SCHEMES_EFFECTIVE_QUOTAS,
> >: or UPDATE_TUNED_INTERVALS to the state file after setting nr_contexts
> >: to 0, damon_sysfs_handle_cmd() queues the corresponding callback via
> >: damon_sysfs_damon_call().
> >:
> >: When the kdamond thread executes the callback, it appears functions like
> >: damon_sysfs_upd_schemes_stats() access contexts_arr[0] without verifying
> >: contexts->nr:
> >:
> >: static int damon_sysfs_upd_schemes_stats(void *data)
> >: {
> >:         struct damon_sysfs_kdamond *kdamond = data;
> >:         struct damon_ctx *ctx = kdamond->damon_ctx;
> >:
> >:         damon_sysfs_schemes_update_stats(
> >:                         kdamond->contexts->contexts_arr[0]->schemes, ctx);
> >:         return 0;
> >: }
> >:
> >: Could this result in a similar NULL pointer dereference if these commands
> >: are triggered while no context is configured?
> >
> >Sashiko is correct.  Privileged users can trigger the issues like below.
> >
> ># damo start
> ># cd /sys/kernel/mm/damon/admin/kdamonds/0
> ># echo 0 > contexts/nr_contexts
> ># echo update_schemes_stats > state
> ># echo update_schemes_effective_quotas > state
> ># echo update_tuned_intervals > state
> >
> >Not necessarily blocker of this patch, but seems all the issues are in a same
> >category.  The third patch of this series is also fixing one of the category
> >bugs.  How about fixing all at once by checking kdamond->contexts->nr at the
> >beginning of damon_sysfs_handle_cmd(), like below?
> >
> >--- a/mm/damon/sysfs.c
> >+++ b/mm/damon/sysfs.c
> >@@ -2404,6 +2404,9 @@ static int damon_sysfs_update_schemes_tried_regions(
> > static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
> >                struct damon_sysfs_kdamond *kdamond)
> > {
> >+       if (cmd != DAMON_SYSFS_CMD_OFF && kdamond->contexts->nr != 1)
> >+               return -EINVAL;
> >+
> >        switch (cmd) {
> >        case DAMON_SYSFS_CMD_ON:
> >                return damon_sysfs_turn_damon_on(kdamond);
> >
> >If we pick this, Fixes: would be deserve to the oldest buggy commit that
> >introduced the first bug of this category.  It is indeed quite old.
> >
> >Fixes: 0ac32b8affb5 ("mm/damon/sysfs: support DAMOS stats")
> >Cc: <stable@vger.kernel.org> # 5.18.x
> >
> >
> >Thanks,
> >SJ
> 
> 
> 
> Hello, did you give Reviewed by you? Or not..

Are you meaning Reviewed-by: tag?  If so, no, not yet.  I want to get your
answer to above question first.  Could you please answer?


Thanks,
SJ

[...]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/4] mm/damon/sysfs: check contexts->nr before clear_schemes_tried_regions
  2026-03-20 14:47     ` SeongJae Park
@ 2026-03-20 15:14       ` Josh Law
  2026-03-20 15:51         ` SeongJae Park
  0 siblings, 1 reply; 6+ messages in thread
From: Josh Law @ 2026-03-20 15:14 UTC (permalink / raw)
  To: SeongJae Park; +Cc: akpm, damon, linux-mm, linux-kernel, stable



On 20 March 2026 14:47:40 GMT, SeongJae Park <sj@kernel.org> wrote:
>On Fri, 20 Mar 2026 07:06:48 +0000 Josh Law <objecting@objecting.org> wrote:
>
>> 
>> 
>> On 20 March 2026 02:13:17 GMT, SeongJae Park <sj@kernel.org> wrote:
>> >On Thu, 19 Mar 2026 15:57:40 +0000 Josh Law <objecting@objecting.org> wrote:
>> >
>> >> The CLEAR_SCHEMES_TRIED_REGIONS command accesses contexts_arr[0]
>> >> without verifying nr_contexts >= 1, causing a NULL pointer dereference
>> >> when no context is configured. Add the missing check.
>> >
>> >Nice catch, thank you!
>> >
>> >Privileged users can trigger this using DAMON sysfs interface.  E.g.,
>> >
>> >    # cd /sys/kernel/mm/damon/admin/kdamonds/
>> >    # echo 1 > nr_kdamonds
>> >    # echo clear_schemes_tried_regions > state
>> >    killed
>> >    # dmesg
>> >    [...]
>> >    [63541.377604] BUG: kernel NULL pointer dereference, address: 0000000000000000
>> >    [...]
>> >
>> >Privileged users can do anything even worse than this, but they might also do
>> >this by a mistake.
>> >
>> >So this deserves Fixes: and Cc stable.
>> >
>> >>
>> >> Signed-off-by: Josh Law <objecting@objecting.org>
>> >> ---
>> >>  mm/damon/sysfs.c | 2 ++
>> >>  1 file changed, 2 insertions(+)
>> >>
>> >> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
>> >> index b573b9d60784..36ad2e8956c9 100644
>> >> --- a/mm/damon/sysfs.c
>> >> +++ b/mm/damon/sysfs.c
>> >> @@ -1769,6 +1769,8 @@ static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
>> >>       case DAMON_SYSFS_CMD_UPDATE_SCHEMES_TRIED_REGIONS:
>> >>               return damon_sysfs_update_schemes_tried_regions(kdamond, false);
>> >>       case DAMON_SYSFS_CMD_CLEAR_SCHEMES_TRIED_REGIONS:
>> >> +             if (kdamond->contexts->nr != 1)
>> >> +                     return -EINVAL;
>> >>               return damon_sysfs_schemes_clear_regions(
>> >>                       kdamond->contexts->contexts_arr[0]->schemes);
>> >>       case DAMON_SYSFS_CMD_UPDATE_SCHEMES_EFFECTIVE_QUOTAS:
>> >> --
>> >> 2.34.1
>> >
>> >So this patch looks good as an individual fix for the individual bug, but...
>> >
>> >Sashiko commented.
>> >
>> ># review url: https://sashiko.dev/#/patchset/20260319155742.186627-3-objecting@objecting.org
>> >
>> >: Does this missing check also affect other manual commands?
>> >:
>> >: If a user writes UPDATE_SCHEMES_STATS, UPDATE_SCHEMES_EFFECTIVE_QUOTAS,
>> >: or UPDATE_TUNED_INTERVALS to the state file after setting nr_contexts
>> >: to 0, damon_sysfs_handle_cmd() queues the corresponding callback via
>> >: damon_sysfs_damon_call().
>> >:
>> >: When the kdamond thread executes the callback, it appears functions like
>> >: damon_sysfs_upd_schemes_stats() access contexts_arr[0] without verifying
>> >: contexts->nr:
>> >:
>> >: static int damon_sysfs_upd_schemes_stats(void *data)
>> >: {
>> >:         struct damon_sysfs_kdamond *kdamond = data;
>> >:         struct damon_ctx *ctx = kdamond->damon_ctx;
>> >:
>> >:         damon_sysfs_schemes_update_stats(
>> >:                         kdamond->contexts->contexts_arr[0]->schemes, ctx);
>> >:         return 0;
>> >: }
>> >:
>> >: Could this result in a similar NULL pointer dereference if these commands
>> >: are triggered while no context is configured?
>> >
>> >Sashiko is correct.  Privileged users can trigger the issues like below.
>> >
>> ># damo start
>> ># cd /sys/kernel/mm/damon/admin/kdamonds/0
>> ># echo 0 > contexts/nr_contexts
>> ># echo update_schemes_stats > state
>> ># echo update_schemes_effective_quotas > state
>> ># echo update_tuned_intervals > state
>> >
>> >Not necessarily blocker of this patch, but seems all the issues are in a same
>> >category.  The third patch of this series is also fixing one of the category
>> >bugs.  How about fixing all at once by checking kdamond->contexts->nr at the
>> >beginning of damon_sysfs_handle_cmd(), like below?
>> >
>> >--- a/mm/damon/sysfs.c
>> >+++ b/mm/damon/sysfs.c
>> >@@ -2404,6 +2404,9 @@ static int damon_sysfs_update_schemes_tried_regions(
>> > static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
>> >                struct damon_sysfs_kdamond *kdamond)
>> > {
>> >+       if (cmd != DAMON_SYSFS_CMD_OFF && kdamond->contexts->nr != 1)
>> >+               return -EINVAL;
>> >+
>> >        switch (cmd) {
>> >        case DAMON_SYSFS_CMD_ON:
>> >                return damon_sysfs_turn_damon_on(kdamond);
>> >
>> >If we pick this, Fixes: would be deserve to the oldest buggy commit that
>> >introduced the first bug of this category.  It is indeed quite old.
>> >
>> >Fixes: 0ac32b8affb5 ("mm/damon/sysfs: support DAMOS stats")
>> >Cc: <stable@vger.kernel.org> # 5.18.x
>> >
>> >
>> >Thanks,
>> >SJ
>> 
>> 
>> 
>> Hello, did you give Reviewed by you? Or not..
>
>Are you meaning Reviewed-by: tag?  If so, no, not yet.  I want to get your
>answer to above question first.  Could you please answer?
>
>
>Thanks,
>SJ
>
>[...]


Well, two is in the same catagory. But seperate fixes may be best.  Because patch 3 dont call that function, so it may be screwy, i mean, if you want me to. Ill guard it. But its a bit on the hacky side


V/R


Josh Law

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/4] mm/damon/sysfs: check contexts->nr before clear_schemes_tried_regions
  2026-03-20 15:14       ` Josh Law
@ 2026-03-20 15:51         ` SeongJae Park
  2026-03-20 15:56           ` Josh Law
  0 siblings, 1 reply; 6+ messages in thread
From: SeongJae Park @ 2026-03-20 15:51 UTC (permalink / raw)
  To: Josh Law; +Cc: SeongJae Park, akpm, damon, linux-mm, linux-kernel, stable

On Fri, 20 Mar 2026 15:14:54 +0000 Josh Law <objecting@objecting.org> wrote:

> 
> 
> On 20 March 2026 14:47:40 GMT, SeongJae Park <sj@kernel.org> wrote:
> >On Fri, 20 Mar 2026 07:06:48 +0000 Josh Law <objecting@objecting.org> wrote:
> >
> >> 
> >> 
> >> On 20 March 2026 02:13:17 GMT, SeongJae Park <sj@kernel.org> wrote:
> >> >On Thu, 19 Mar 2026 15:57:40 +0000 Josh Law <objecting@objecting.org> wrote:
[...]
> >> >Not necessarily blocker of this patch, but seems all the issues are in a same
> >> >category.  The third patch of this series is also fixing one of the category
> >> >bugs.  How about fixing all at once by checking kdamond->contexts->nr at the
> >> >beginning of damon_sysfs_handle_cmd(), like below?
> >> >
> >> >--- a/mm/damon/sysfs.c
> >> >+++ b/mm/damon/sysfs.c
> >> >@@ -2404,6 +2404,9 @@ static int damon_sysfs_update_schemes_tried_regions(
> >> > static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
> >> >                struct damon_sysfs_kdamond *kdamond)
> >> > {
> >> >+       if (cmd != DAMON_SYSFS_CMD_OFF && kdamond->contexts->nr != 1)
> >> >+               return -EINVAL;
> >> >+
> >> >        switch (cmd) {
> >> >        case DAMON_SYSFS_CMD_ON:
> >> >                return damon_sysfs_turn_damon_on(kdamond);
> >> >
> >> >If we pick this, Fixes: would be deserve to the oldest buggy commit that
> >> >introduced the first bug of this category.  It is indeed quite old.
> >> >
> >> >Fixes: 0ac32b8affb5 ("mm/damon/sysfs: support DAMOS stats")
> >> >Cc: <stable@vger.kernel.org> # 5.18.x
> >> >
> >> >
> >> >Thanks,
> >> >SJ
> >> 
> >> 
> >> 
> >> Hello, did you give Reviewed by you? Or not..
> >
> >Are you meaning Reviewed-by: tag?  If so, no, not yet.  I want to get your
> >answer to above question first.  Could you please answer?
> >
> >
> >Thanks,
> >SJ
> >
> >[...]
> 
> 
> Well, two is in the same catagory. But seperate fixes may be best.  Because patch 3 dont call that function, so it may be screwy, i mean, if you want me to. Ill guard it. But its a bit on the hacky side

I agree there could be more cleaner way.  But these fixes need to go to stable,
so I'd prefer a change that also easier to backport.

So, yes, I want to.  Thank you for kindly accepting my suggestion.

Could you please re-post this series for the first and the fourth patches as
they are, after adding my Reviewed-by:, Fixes: and Cc: stable tags, and a patch
checking kdamond->contexts->nr at the beginning of damon_sysfs_handle_cmd() as
I suggested?


Thanks,
SJ

[...]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/4] mm/damon/sysfs: check contexts->nr before clear_schemes_tried_regions
  2026-03-20 15:51         ` SeongJae Park
@ 2026-03-20 15:56           ` Josh Law
  0 siblings, 0 replies; 6+ messages in thread
From: Josh Law @ 2026-03-20 15:56 UTC (permalink / raw)
  To: SeongJae Park; +Cc: akpm, damon, linux-mm, linux-kernel, stable



On 20 March 2026 15:51:14 GMT, SeongJae Park <sj@kernel.org> wrote:
>On Fri, 20 Mar 2026 15:14:54 +0000 Josh Law <objecting@objecting.org> wrote:
>
>> 
>> 
>> On 20 March 2026 14:47:40 GMT, SeongJae Park <sj@kernel.org> wrote:
>> >On Fri, 20 Mar 2026 07:06:48 +0000 Josh Law <objecting@objecting.org> wrote:
>> >
>> >> 
>> >> 
>> >> On 20 March 2026 02:13:17 GMT, SeongJae Park <sj@kernel.org> wrote:
>> >> >On Thu, 19 Mar 2026 15:57:40 +0000 Josh Law <objecting@objecting.org> wrote:
>[...]
>> >> >Not necessarily blocker of this patch, but seems all the issues are in a same
>> >> >category.  The third patch of this series is also fixing one of the category
>> >> >bugs.  How about fixing all at once by checking kdamond->contexts->nr at the
>> >> >beginning of damon_sysfs_handle_cmd(), like below?
>> >> >
>> >> >--- a/mm/damon/sysfs.c
>> >> >+++ b/mm/damon/sysfs.c
>> >> >@@ -2404,6 +2404,9 @@ static int damon_sysfs_update_schemes_tried_regions(
>> >> > static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
>> >> >                struct damon_sysfs_kdamond *kdamond)
>> >> > {
>> >> >+       if (cmd != DAMON_SYSFS_CMD_OFF && kdamond->contexts->nr != 1)
>> >> >+               return -EINVAL;
>> >> >+
>> >> >        switch (cmd) {
>> >> >        case DAMON_SYSFS_CMD_ON:
>> >> >                return damon_sysfs_turn_damon_on(kdamond);
>> >> >
>> >> >If we pick this, Fixes: would be deserve to the oldest buggy commit that
>> >> >introduced the first bug of this category.  It is indeed quite old.
>> >> >
>> >> >Fixes: 0ac32b8affb5 ("mm/damon/sysfs: support DAMOS stats")
>> >> >Cc: <stable@vger.kernel.org> # 5.18.x
>> >> >
>> >> >
>> >> >Thanks,
>> >> >SJ
>> >> 
>> >> 
>> >> 
>> >> Hello, did you give Reviewed by you? Or not..
>> >
>> >Are you meaning Reviewed-by: tag?  If so, no, not yet.  I want to get your
>> >answer to above question first.  Could you please answer?
>> >
>> >
>> >Thanks,
>> >SJ
>> >
>> >[...]
>> 
>> 
>> Well, two is in the same catagory. But seperate fixes may be best.  Because patch 3 dont call that function, so it may be screwy, i mean, if you want me to. Ill guard it. But its a bit on the hacky side
>
>I agree there could be more cleaner way.  But these fixes need to go to stable,
>so I'd prefer a change that also easier to backport.
>
>So, yes, I want to.  Thank you for kindly accepting my suggestion.
>
>Could you please re-post this series for the first and the fourth patches as
>they are, after adding my Reviewed-by:, Fixes: and Cc: stable tags, and a patch
>checking kdamond->contexts->nr at the beginning of damon_sysfs_handle_cmd() as
>I suggested?
>
>
>Thanks,
>SJ
>
>[...]


Absolutely!

Will do

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-03-20 15:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260319155742.186627-3-objecting@objecting.org>
2026-03-20  2:13 ` [PATCH 2/4] mm/damon/sysfs: check contexts->nr before clear_schemes_tried_regions SeongJae Park
2026-03-20  7:06   ` Josh Law
2026-03-20 14:47     ` SeongJae Park
2026-03-20 15:14       ` Josh Law
2026-03-20 15:51         ` SeongJae Park
2026-03-20 15:56           ` Josh Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox