linux-kernel - Re: [PATCH] perf lock contention: Add a missing NULL check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZgxgRJdFlwfESwKF@x1>
Date: Tue, 2 Apr 2024 16:45:08 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Ian Rogers <irogers@...gle.com>, Kan Liang <kan.liang@...ux.intel.com>,
	Jiri Olsa <jolsa@...nel.org>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
	linux-perf-users@...r.kernel.org, Song Liu <song@...nel.org>,
	bpf@...r.kernel.org
Subject: Re: [PATCH] perf lock contention: Add a missing NULL check

On Tue, Apr 02, 2024 at 11:45:43AM -0700, Namhyung Kim wrote:
> I got a report for a failure in BPF verifier on a recent kernel with
> perf lock contention command.  It checks task->sighand->siglock without
> checking if sighand is NULL or not.  Let's add one.
> 
>   ; if (&curr->sighand->siglock == (void *)lock)
>   265: (79) r1 = *(u64 *)(r0 +2624)     ; frame1: R0_w=trusted_ptr_task_struct(off=0,imm=0) R1_w=rcu_ptr_or_null_sighand_struct(off=0,imm=0)
>   266: (b7) r2 = 0                      ; frame1: R2_w=0
>   267: (0f) r1 += r2
>   R1 pointer arithmetic on rcu_ptr_or_null_ prohibited, null-check it first
>   processed 164 insns (limit 1000000) max_states_per_insn 1 total_states 15 peak_states 15 mark_read 5
>   -- END PROG LOAD LOG --
>   libbpf: prog 'contention_end': failed to load: -13
>   libbpf: failed to load object 'lock_contention_bpf'
>   libbpf: failed to load BPF skeleton 'lock_contention_bpf': -13
>   Failed to load lock-contention BPF skeleton
>   lock contention BPF setup failed
>   lock contention did not detect any lock contention
> 
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>

Acked-by: Arnaldo Carvalho de Melo <acme@...hat.com>

Are you going to have this merged into perf-tools?

A Fixes: tag isn't perhaps needed as it worked in the past?

- Arnaldo

> ---
>  tools/perf/util/bpf_skel/lock_contention.bpf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/bpf_skel/lock_contention.bpf.c b/tools/perf/util/bpf_skel/lock_contention.bpf.c
> index fb54bd38e7d0..4e5914d7eeaa 100644
> --- a/tools/perf/util/bpf_skel/lock_contention.bpf.c
> +++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c
> @@ -305,7 +305,7 @@ static inline __u32 check_lock_type(__u64 lock, __u32 flags)
>  		break;
>  	case LCB_F_SPIN:  /* spinlock */
>  		curr = bpf_get_current_task_btf();
> -		if (&curr->sighand->siglock == (void *)lock)
> +		if (curr->sighand && &curr->sighand->siglock == (void *)lock)
>  			return LCD_F_SIGHAND_LOCK;
>  		break;
>  	default:
> -- 
> 2.44.0.478.gd926399ef9-goog