mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] _x86: intel: pt: fix potential null dereferences
@ 2025-10-22 18:42 Shi Hao
  2026-09-15 22:00 ` Ian Rogers
  0 siblings, 1 reply; 3+ messages in thread
From: Shi Hao @ 2025-10-22 18:42 UTC (permalink / raw)
  To: peterz
  Cc: mingo, acme, namhyung, x86, linux-perf-users, linux-kernel, hpa,
	Shi Hao, Smatch static checker

Add checks to prevent potential null dereferences of buf->stop_te
and buf->intr_te in pt_buffer_reset_markers function.

Smatch reported possible null dereferences of buf->stop_te and
buf->intr_te in the pt_buffer_reset_markers() and when i checked
both pointers were checked for null dereferences in earlier lines
however,after calling pt_topa_entry_for_page() where its return
value is NULL in certain conditions there were no checks for further
buf->stop_te and buf->intr_te uses which could potentially be null
dereferenced.

To avoid null dereference add checks after each pt_topa_entry_for_page()
call to safely handle null returns and also add checks where there was
direct dereference of the pointers.

Reported-by: Smatch static checker <smatch@kernel.org>
Signed-off-by: Shi Hao <i.shihao.999@gmail.com>
---
 arch/x86/events/intel/pt.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
index e8cf29d2b10c..2b7d5d118b48 100644
--- a/arch/x86/events/intel/pt.c
+++ b/arch/x86/events/intel/pt.c
@@ -1148,7 +1148,8 @@ static int pt_buffer_reset_markers(struct pt_buffer *buf,
 	if (idx != buf->stop_pos) {
 		buf->stop_pos = idx;
 		buf->stop_te = pt_topa_entry_for_page(buf, idx);
-		buf->stop_te = pt_topa_prev_entry(buf, buf->stop_te);
+		if (buf->stop_te)
+			buf->stop_te = pt_topa_prev_entry(buf, buf->stop_te);
 	}

 	wakeup = handle->wakeup >> PAGE_SHIFT;
@@ -1162,12 +1163,16 @@ static int pt_buffer_reset_markers(struct pt_buffer *buf,
 	if (idx != buf->intr_pos) {
 		buf->intr_pos = idx;
 		buf->intr_te = pt_topa_entry_for_page(buf, idx);
-		buf->intr_te = pt_topa_prev_entry(buf, buf->intr_te);
+		if (buf->intr_te)
+			buf->intr_te = pt_topa_prev_entry(buf, buf->intr_te);
 	}

-	buf->stop_te->stop = 1;
-	buf->stop_te->intr = 1;
-	buf->intr_te->intr = 1;
+	if (buf->stop_te) {
+		buf->stop_te->stop = 1;
+		buf->stop_te->intr = 1;
+	}
+	if (buf->intr_te)
+		buf->intr_te->intr = 1;

 	return 0;
 }
--
2.51.0


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

* Re: [PATCH] _x86: intel: pt: fix potential null dereferences
  2025-10-22 18:42 [PATCH] _x86: intel: pt: fix potential null dereferences Shi Hao
@ 2026-09-15 22:00 ` Ian Rogers
  2026-09-16  0:35   ` Mi, Dapeng
  0 siblings, 1 reply; 3+ messages in thread
From: Ian Rogers @ 2026-09-15 22:00 UTC (permalink / raw)
  To: Shi Hao, Dapeng Mi
  Cc: peterz, mingo, acme, namhyung, x86, linux-perf-users,
	linux-kernel, hpa, Smatch static checker

On Wed, Oct 22, 2025 at 11:45 AM Shi Hao <i.shihao.999@gmail.com> wrote:
>
> Add checks to prevent potential null dereferences of buf->stop_te
> and buf->intr_te in pt_buffer_reset_markers function.
>
> Smatch reported possible null dereferences of buf->stop_te and
> buf->intr_te in the pt_buffer_reset_markers() and when i checked
> both pointers were checked for null dereferences in earlier lines
> however,after calling pt_topa_entry_for_page() where its return
> value is NULL in certain conditions there were no checks for further
> buf->stop_te and buf->intr_te uses which could potentially be null
> dereferenced.
>
> To avoid null dereference add checks after each pt_topa_entry_for_page()
> call to safely handle null returns and also add checks where there was
> direct dereference of the pointers.
>
> Reported-by: Smatch static checker <smatch@kernel.org>
> Signed-off-by: Shi Hao <i.shihao.999@gmail.com>

The addition of null checks to make smatch happy lgtm. Dapeng, could
you take a look?

Thanks,
Ian

> ---
>  arch/x86/events/intel/pt.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
> index e8cf29d2b10c..2b7d5d118b48 100644
> --- a/arch/x86/events/intel/pt.c
> +++ b/arch/x86/events/intel/pt.c
> @@ -1148,7 +1148,8 @@ static int pt_buffer_reset_markers(struct pt_buffer *buf,
>         if (idx != buf->stop_pos) {
>                 buf->stop_pos = idx;
>                 buf->stop_te = pt_topa_entry_for_page(buf, idx);
> -               buf->stop_te = pt_topa_prev_entry(buf, buf->stop_te);
> +               if (buf->stop_te)
> +                       buf->stop_te = pt_topa_prev_entry(buf, buf->stop_te);
>         }
>
>         wakeup = handle->wakeup >> PAGE_SHIFT;
> @@ -1162,12 +1163,16 @@ static int pt_buffer_reset_markers(struct pt_buffer *buf,
>         if (idx != buf->intr_pos) {
>                 buf->intr_pos = idx;
>                 buf->intr_te = pt_topa_entry_for_page(buf, idx);
> -               buf->intr_te = pt_topa_prev_entry(buf, buf->intr_te);
> +               if (buf->intr_te)
> +                       buf->intr_te = pt_topa_prev_entry(buf, buf->intr_te);
>         }
>
> -       buf->stop_te->stop = 1;
> -       buf->stop_te->intr = 1;
> -       buf->intr_te->intr = 1;
> +       if (buf->stop_te) {
> +               buf->stop_te->stop = 1;
> +               buf->stop_te->intr = 1;
> +       }
> +       if (buf->intr_te)
> +               buf->intr_te->intr = 1;
>
>         return 0;
>  }
> --
> 2.51.0
>
>

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

* Re: [PATCH] _x86: intel: pt: fix potential null dereferences
  2026-09-15 22:00 ` Ian Rogers
@ 2026-09-16  0:35   ` Mi, Dapeng
  0 siblings, 0 replies; 3+ messages in thread
From: Mi, Dapeng @ 2026-09-16  0:35 UTC (permalink / raw)
  To: Ian Rogers, Shi Hao
  Cc: peterz, mingo, acme, namhyung, x86, linux-perf-users,
	linux-kernel, hpa, Smatch static checker


On 9/16/2026 6:00 AM, Ian Rogers wrote:
> On Wed, Oct 22, 2025 at 11:45 AM Shi Hao <i.shihao.999@gmail.com> wrote:
>> Add checks to prevent potential null dereferences of buf->stop_te
>> and buf->intr_te in pt_buffer_reset_markers function.
>>
>> Smatch reported possible null dereferences of buf->stop_te and
>> buf->intr_te in the pt_buffer_reset_markers() and when i checked
>> both pointers were checked for null dereferences in earlier lines
>> however,after calling pt_topa_entry_for_page() where its return
>> value is NULL in certain conditions there were no checks for further
>> buf->stop_te and buf->intr_te uses which could potentially be null
>> dereferenced.
>>
>> To avoid null dereference add checks after each pt_topa_entry_for_page()
>> call to safely handle null returns and also add checks where there was
>> direct dereference of the pointers.
>>
>> Reported-by: Smatch static checker <smatch@kernel.org>
>> Signed-off-by: Shi Hao <i.shihao.999@gmail.com>
> The addition of null checks to make smatch happy lgtm. Dapeng, could
> you take a look?

Base on the code logic, yeah, it's necessary to add these NULL checks,
although I doubt if pt_topa_entry_for_page() could return NULL in reality.
It indicates something is wrong if pt_topa_entry_for_page() returns NULL.

BTW, the prefix of short log is not correct, it should be
"perf/x86/intel/pt" which follows current naming convention. Thanks. 


>
> Thanks,
> Ian
>
>> ---
>>  arch/x86/events/intel/pt.c | 15 ++++++++++-----
>>  1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
>> index e8cf29d2b10c..2b7d5d118b48 100644
>> --- a/arch/x86/events/intel/pt.c
>> +++ b/arch/x86/events/intel/pt.c
>> @@ -1148,7 +1148,8 @@ static int pt_buffer_reset_markers(struct pt_buffer *buf,
>>         if (idx != buf->stop_pos) {
>>                 buf->stop_pos = idx;
>>                 buf->stop_te = pt_topa_entry_for_page(buf, idx);
>> -               buf->stop_te = pt_topa_prev_entry(buf, buf->stop_te);
>> +               if (buf->stop_te)
>> +                       buf->stop_te = pt_topa_prev_entry(buf, buf->stop_te);
>>         }
>>
>>         wakeup = handle->wakeup >> PAGE_SHIFT;
>> @@ -1162,12 +1163,16 @@ static int pt_buffer_reset_markers(struct pt_buffer *buf,
>>         if (idx != buf->intr_pos) {
>>                 buf->intr_pos = idx;
>>                 buf->intr_te = pt_topa_entry_for_page(buf, idx);
>> -               buf->intr_te = pt_topa_prev_entry(buf, buf->intr_te);
>> +               if (buf->intr_te)
>> +                       buf->intr_te = pt_topa_prev_entry(buf, buf->intr_te);
>>         }
>>
>> -       buf->stop_te->stop = 1;
>> -       buf->stop_te->intr = 1;
>> -       buf->intr_te->intr = 1;
>> +       if (buf->stop_te) {
>> +               buf->stop_te->stop = 1;
>> +               buf->stop_te->intr = 1;
>> +       }
>> +       if (buf->intr_te)
>> +               buf->intr_te->intr = 1;
>>
>>         return 0;
>>  }
>> --
>> 2.51.0
>>
>>

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

end of thread, other threads:[~2026-09-16  0:35 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-10-22 18:42 [PATCH] _x86: intel: pt: fix potential null dereferences Shi Hao
2026-09-15 22:00 ` Ian Rogers
2026-09-16  0:35   ` Mi, Dapeng

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

all inboxes | Powered by JetHome®