netdev - Re: [PATCH net-next v1] drivers/net/ethernet/intel/e100: check the return value of e100_exec

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 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:   Mon, 12 Sep 2022 15:08:27 -0700
From:   Tony Nguyen <anthony.l.nguyen@...el.com>
To:     Li Zhong <floridsleeves@...il.com>, <linux-kernel@...r.kernel.org>,
        <netdev@...r.kernel.org>, <intel-wired-lan@...ts.osuosl.org>
CC:     <pabeni@...hat.com>, <kuba@...nel.org>, <edumazet@...gle.com>,
        <davem@...emloft.net>, <jesse.brandeburg@...el.com>
Subject: Re: [PATCH net-next v1] drivers/net/ethernet/intel/e100: check the
 return value of e100_exec_cmd()

On 9/8/2022 9:16 PM, Li Zhong wrote:
> Check the return value of e100_exec_cmd() which could return error code
> when execution fails.

Are you coming across this as a real bug or as something reported by 
static analysis? If the latter, I suggest checking the return value and 
reporting it as debug, however, not changing existing behavior. We don't 
have validation on this driver so there is limited ability to check for 
regressions and the code has been like this for a long time without 
reported issues.

Thanks,
Tony

> Signed-off-by: Li Zhong <floridsleeves@...il.com>
> ---
>   drivers/net/ethernet/intel/e100.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/e100.c b/drivers/net/ethernet/intel/e100.c
> index 11a884aa5082..3b84745376fe 100644
> --- a/drivers/net/ethernet/intel/e100.c
> +++ b/drivers/net/ethernet/intel/e100.c
> @@ -1911,7 +1911,8 @@ static inline void e100_start_receiver(struct nic *nic, struct rx *rx)
>   
>   	/* (Re)start RU if suspended or idle and RFA is non-NULL */
>   	if (rx->skb) {
> -		e100_exec_cmd(nic, ruc_start, rx->dma_addr);
> +		if (!e100_exec_cmd(nic, ruc_start, rx->dma_addr))
> +			return;
>   		nic->ru_running = RU_RUNNING;
>   	}
>   }