netdev - Re: [PATCH] net: sxgbe: fix return value of _

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: <20220224214302.4262c26f@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Thu, 24 Feb 2022 21:43:02 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Randy Dunlap <rdunlap@...radead.org>
Cc:     netdev@...r.kernel.org, patches@...ts.linux.dev,
        Igor Zhbanov <i.zhbanov@...russia.ru>,
        Siva Reddy <siva.kallam@...sung.com>,
        Girish K S <ks.giri@...sung.com>,
        Byungho An <bh74.an@...sung.com>,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH] net: sxgbe: fix return value of __setup handler

On Wed, 23 Feb 2022 19:35:28 -0800 Randy Dunlap wrote:
> __setup() handlers should return 1 on success, i.e., the parameter
> has been handled. A return of 0 causes the "option=value" string to be
> added to init's environment strings, polluting it.

Meaning early_param_on_off() also returns the wrong thing?
Or that's different?

> Fixes: acc18c147b22 ("net: sxgbe: add EEE(Energy Efficient Ethernet) for Samsung sxgbe")
> Fixes: 1edb9ca69e8a ("net: sxgbe: add basic framework for Samsung 10Gb ethernet driver")
> Signed-off-by: Randy Dunlap <rdunlap@...radead.org>
> Reported-by: Igor Zhbanov <i.zhbanov@...russia.ru>
> Link: lore.kernel.org/r/64644a2f-4a20-bab3-1e15-3b2cdd0defe3@...russia.ru
>
> --- linux-next-20220223.orig/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c
> +++ linux-next-20220223/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c
> @@ -2285,18 +2285,18 @@ static int __init sxgbe_cmdline_opt(char
>  	char *opt;
>  
>  	if (!str || !*str)
> -		return -EINVAL;
> +		return 1;
>  	while ((opt = strsep(&str, ",")) != NULL) {
>  		if (!strncmp(opt, "eee_timer:", 10)) {
>  			if (kstrtoint(opt + 10, 0, &eee_timer))
>  				goto err;
>  		}
>  	}
> -	return 0;
> +	return 1;
>  
>  err:
>  	pr_err("%s: ERROR broken module parameter conversion\n", __func__);
> -	return -EINVAL;
> +	return 1;
>  }
>  
>  __setup("sxgbeeth=", sxgbe_cmdline_opt);

Was the option of making __setup() return void considered?
Sounds like we always want to return 1 so what's the point?