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]
Date:   Tue, 30 Aug 2022 14:44:50 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Arun Ramadoss <arun.ramadoss@...rochip.com>
Cc:     linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
        Woojung Huh <woojung.huh@...rochip.com>,
        UNGLinuxDriver@...rochip.com,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Russell King <linux@...linux.org.uk>,
        Tristram Ha <Tristram.Ha@...rochip.com>
Subject: Re: [RFC Patch net-next v3 1/3] net: dsa: microchip: use
 dev_ops->reset instead of exit in ksz_switch_register

On Tue, Aug 30, 2022 at 04:23:01PM +0530, Arun Ramadoss wrote:
> ksz8_switch_exit, ksz9477_switch_exit and lan937x_switch_exit functions
> all call the reset function which is assigned to dev_ops->reset hooks.
> So instead of calling the dev_ops->exit in ksz_switch_register during
> the error condition of dsa_register_switch, dev_ops->reset is used now.
> The dev_ops->exit can be extended in lan937x for freeing up the irq
> during the ksz_spi_remove. If we add the irq remove in the exit function
> and it is called during the dsa_switch_register error condition, kernel
> panic happens since irq is setup only in setup operation. To avoid the
> kernel panic, dev_ops->reset is used instead of exit.
> 
> Signed-off-by: Arun Ramadoss <arun.ramadoss@...rochip.com>
> ---
>  drivers/net/dsa/microchip/ksz_common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 6bd69a7e6809..da9bdf753f7a 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -1963,7 +1963,7 @@ int ksz_switch_register(struct ksz_device *dev)
>  
>  	ret = dsa_register_switch(dev->ds);
>  	if (ret) {
> -		dev->dev_ops->exit(dev);
> +		dev->dev_ops->reset(dev);
>  		return ret;

I don't like this change. You are supposed to be undoing whatever
ksz_switch_register() has done so far. It appears to of called
ops->init() so far, so calling ops->exit() should be the opposite.

If you are making exit() not the opposite of init() in later patches,
it suggests your code structure is wrong. If exit() is not already the
opposite of init() then fix that.

   Andrew

Powered by blists - more mailing lists