netdev - Re: [PATCH rfc v2 08/10] net: dsa: qca8k: Pass error code from reply decoder to requester

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:   Fri, 23 Sep 2022 22:42:02 +0000
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     Andrew Lunn <andrew@...n.ch>
CC:     netdev <netdev@...r.kernel.org>,
        "mattias.forsblad@...il.com" <mattias.forsblad@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Christian Marangi <ansuelsmth@...il.com>
Subject: Re: [PATCH rfc v2 08/10] net: dsa: qca8k: Pass error code from reply
 decoder to requester

On Thu, Sep 22, 2022 at 07:58:19PM +0200, Andrew Lunn wrote:
> The code which decodes the frame and signals the complete can detect
> error within the reply, such as fields have unexpected values. Pass an
> error code between the completer and the function waiting on the
> complete. This simplifies the error handling, since all errors are
> combined into one place.
> 
> Signed-off-by: Andrew Lunn <andrew@...n.ch>
> ---
> v2:
> 
> Remove EPROTO if the sequence numbers don't match, drop the reply
> ---
> @@ -1439,6 +1400,7 @@ static void qca8k_mib_autocast_handler(struct dsa_switch *ds, struct sk_buff *sk
>  	const struct qca8k_mib_desc *mib;
>  	struct mib_ethhdr *mib_ethhdr;
>  	int i, mib_len, offset = 0;
> +	int err = 0;
>  	u64 *data;
>  	u8 port;
>  
> @@ -1449,8 +1411,10 @@ static void qca8k_mib_autocast_handler(struct dsa_switch *ds, struct sk_buff *sk
>  	 * parse only the requested one.
>  	 */
>  	port = FIELD_GET(QCA_HDR_RECV_SOURCE_PORT, ntohs(mib_ethhdr->hdr));
> -	if (port != mib_eth_data->req_port)
> +	if (port != mib_eth_data->req_port) {
> +		err = -EPROTO;
>  		goto exit;
> +	}
>  
>  	data = mib_eth_data->data;
>  
> @@ -1479,7 +1443,7 @@ static void qca8k_mib_autocast_handler(struct dsa_switch *ds, struct sk_buff *sk
>  exit:
>  	/* Complete on receiving all the mib packet */
>  	if (refcount_dec_and_test(&mib_eth_data->port_parsed))
> -		dsa_inband_complete(&mib_eth_data->inband);
> +		dsa_inband_complete(&mib_eth_data->inband, err);
>  }

My understanding of the autocast function (I could be wrong) is that
it's essentially one request with 10 (or how many ports there are)
responses. At least this is what the code appears to handle.

I don't think you can say "if port isn't the requested port => set err
to -EPROTO". Because this will make dsa_inband_wait_for_completion() see
as a return code -EPROTO whenever the requested port was anything except
for the last port for which the switch had something to autocast. Which
is probably the last port.