Received-SPF: pass (google.com: domain of linux-kernel+bounces-128520-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20240402184543.898923-1-namhyung@kernel.org>
In-Reply-To: <20240402184543.898923-1-namhyung@kernel.org>
From: Ian Rogers <irogers@google.com>
Date: Tue, 2 Apr 2024 11:49:28 -0700
Message-ID: <CAP-5=fWh+nrZukmvANf0rmG_Fw11hMgm=dPkHCG79F+tdzsTvQ@mail.gmail.com>
Subject: Re: [PATCH] perf lock contention: Add a missing NULL check
To: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>, Kan Liang <kan.liang@linux.intel.com>, 
	Jiri Olsa <jolsa@kernel.org>, Adrian Hunter <adrian.hunter@intel.com>, 
	Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@kernel.org>, 
	LKML <linux-kernel@vger.kernel.org>, linux-perf-users@vger.kernel.org, 
	Song Liu <song@kernel.org>, bpf@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Apr 2, 2024 at 11:45=E2=80=AFAM Namhyung Kim <namhyung@kernel.org> =
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 =3D=3D (void *)lock)
>   265: (79) r1 =3D *(u64 *)(r0 +2624)     ; frame1: R0_w=3Dtrusted_ptr_ta=
sk_struct(off=3D0,imm=3D0) R1_w=3Drcu_ptr_or_null_sighand_struct(off=3D0,im=
m=3D0)
>   266: (b7) r2 =3D 0                      ; frame1: R2_w=3D0
>   267: (0f) r1 +=3D r2
>   R1 pointer arithmetic on rcu_ptr_or_null_ prohibited, null-check it fir=
st
>   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@kernel.org>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  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 =3D bpf_get_current_task_btf();
> -               if (&curr->sighand->siglock =3D=3D (void *)lock)
> +               if (curr->sighand && &curr->sighand->siglock =3D=3D (void=
 *)lock)
>                         return LCD_F_SIGHAND_LOCK;
>                 break;
>         default:
> --
> 2.44.0.478.gd926399ef9-goog
>