linux-kernel - Re: [PATCH v3 1/3] EDAC: mv64x60: check driver registration success

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: <7b51ad125caf4d4c97157187510626cc@svr-chch-ex1.atlnz.lc>
Date:   Wed, 7 Jun 2017 05:29:55 +0000
From:   Chris Packham <Chris.Packham@...iedtelesis.co.nz>
To:     "bp@...en8.de" <bp@...en8.de>,
        "mchehab@...nel.org" <mchehab@...nel.org>,
        "linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
        "mpe@...erman.id.au" <mpe@...erman.id.au>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>
Subject: Re: [PATCH v3 1/3] EDAC: mv64x60: check driver registration success

Hi Borislav,

On 30/05/17 09:21, Chris Packham wrote:
> Check the return status of platform_driver_register() in
> mv64x60_edac_init(). Only output messages and initialise the
> edac_op_state if the registration is successful.
> 
> Signed-off-by: Chris Packham <chris.packham@...iedtelesis.co.nz>
> ---
> Changes in v3:
> - catch the retval of platform_register_drivers and return early on failure
>    (thanks Borislav).
> 
>   drivers/edac/mv64x60_edac.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/edac/mv64x60_edac.c b/drivers/edac/mv64x60_edac.c
> index 14b7e7b71eaa..172081551a70 100644
> --- a/drivers/edac/mv64x60_edac.c
> +++ b/drivers/edac/mv64x60_edac.c
> @@ -853,7 +853,11 @@ static struct platform_driver * const drivers[] = {
>   
>   static int __init mv64x60_edac_init(void)
>   {
> -	int ret = 0;
> +	int ret;
> +
> +	ret = platform_register_drivers(drivers, ARRAY_SIZE(drivers));
> +	if (ret)
> +		return ret;

I actually think this was wrong. The edac_op_state is set as a module 
param and affects the behaviour of the driver probe function which on 
success could be called before we sanity check it below.

I'm back to thinking that my original v1 of just removing the unused ret 
was appropriate. If we still consider the driver too noisy we could just 
remove the printks.

What do you think?

>   
>   	printk(KERN_INFO "Marvell MV64x60 EDAC driver " MV64x60_REVISION "\n");
>   	printk(KERN_INFO "\t(C) 2006-2007 MontaVista Software\n");
> @@ -867,7 +871,7 @@ static int __init mv64x60_edac_init(void)
>   		break;
>   	}
>   
> -	return platform_register_drivers(drivers, ARRAY_SIZE(drivers));
> +	return 0;
>   }
>   module_init(mv64x60_edac_init);
>   
>