linux-kernel - Re: [PATCH] drivers/perf: Change WARN_ON() to dev_err() on irq_set

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: <YvokT+ZQQMlEAga8@kroah.com>
Date:   Mon, 15 Aug 2022 12:47:43 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Yicong Yang <yangyicong@...wei.com>
Cc:     will@...nel.org, mark.rutland@....com, Frank.li@....com,
        shawnguo@...nel.org, s.hauer@...gutronix.de, kernel@...gutronix.de,
        festevam@...il.com, linux-imx@....com, zhangshaokun@...ilicon.com,
        agross@...nel.org, bjorn.andersson@...aro.org,
        konrad.dybcio@...ainline.org, khuong@...amperecomputing.com,
        john.garry@...wei.com, jonathan.cameron@...wei.com,
        yangyicong@...ilicon.com, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH] drivers/perf: Change WARN_ON() to dev_err() on
 irq_set_affinity() failure

On Mon, Aug 15, 2022 at 05:28:15PM +0800, Yicong Yang wrote:
> From: Yicong Yang <yangyicong@...ilicon.com>
> 
> The WARN_ON() on irq_set_affinity() failure is misused according to the [1]
> and may crash people's box unintentionally. This may also be redundant since
> in the failure case we may also trigger the WARN and dump the stack in the
> perf core[2] for a second time.
> 
> So change the WARN_ON() to dev_err() to just print the failure message.
> 
> [1] https://github.com/torvalds/linux/blob/master/include/asm-generic/bug.h#L74
> [2] https://github.com/torvalds/linux/blob/master/kernel/events/core.c#L313

Please point to git.kernel.org links, we do not control github.com and
it's random mirrors.

> 
> Suggested-by: Greg KH <gregkh@...uxfoundation.org>
> [https://lore.kernel.org/lkml/YuOi3i0XHV++z1YI@kroah.com/]
> Signed-off-by: Yicong Yang <yangyicong@...ilicon.com>
> ---
>  drivers/perf/arm-ccn.c                   | 5 +++--
>  drivers/perf/arm_dmc620_pmu.c            | 3 ++-
>  drivers/perf/arm_smmuv3_pmu.c            | 6 ++++--
>  drivers/perf/fsl_imx8_ddr_perf.c         | 3 ++-
>  drivers/perf/hisilicon/hisi_pcie_pmu.c   | 6 ++++--
>  drivers/perf/hisilicon/hisi_uncore_pmu.c | 6 ++++--
>  drivers/perf/qcom_l2_pmu.c               | 8 ++++++--
>  drivers/perf/xgene_pmu.c                 | 6 ++++--
>  8 files changed, 29 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/perf/arm-ccn.c b/drivers/perf/arm-ccn.c
> index 728d13d8e98a..83abd909ba49 100644
> --- a/drivers/perf/arm-ccn.c
> +++ b/drivers/perf/arm-ccn.c
> @@ -1210,8 +1210,9 @@ static int arm_ccn_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
>  		return 0;
>  	perf_pmu_migrate_context(&dt->pmu, cpu, target);
>  	dt->cpu = target;
> -	if (ccn->irq)
> -		WARN_ON(irq_set_affinity(ccn->irq, cpumask_of(dt->cpu)));
> +	if (ccn->irq && irq_set_affinity(ccn->irq, cpumask_of(dt->cpu)))
> +		dev_err(ccn->dev, "Failed to set interrupt affinity\n");
> +
>  	return 0;

Why are you returning with no error, if an error happened?

Same everywhere else, you need to explain this in your changelog text.

thanks,

greg k-h