netdev - Re: [PATCH net-next 1/5] sfc: check recirc

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, 25 Oct 2022 19:40:35 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     <edward.cree@....com>
Cc:     <netdev@...r.kernel.org>, <linux-net-drivers@....com>,
        <davem@...emloft.net>, <pabeni@...hat.com>, <edumazet@...gle.com>,
        <habetsm.xilinx@...il.com>, Edward Cree <ecree.xilinx@...il.com>
Subject: Re: [PATCH net-next 1/5] sfc: check recirc_id match caps before MAE
 offload

On Mon, 24 Oct 2022 10:29:21 +0100 edward.cree@....com wrote:
> From: Edward Cree <ecree.xilinx@...il.com>
> 
> Offloaded TC rules always match on recirc_id in the MAE, so we should
>  check that the MAE reported support for this match before attempting
>  to insert the rule.
> 
> Fixes: d902e1a737d4 ("sfc: bare bones TC offload on EF100")

This commit made it to net, needs to go separately there.

> +/* Validate field mask against hardware capabilities.  May return from caller */
> +#define CHECK(_mcdi, _field)	do {					       \
> +	enum mask_type typ = classify_mask((const u8 *)&mask->_field,	       \
> +					   sizeof(mask->_field));	       \
> +									       \
> +	rc = efx_mae_match_check_cap_typ(supported_fields[MAE_FIELD_ ## _mcdi],\
> +					 typ);				       \
> +	if (rc) {							       \
> +		NL_SET_ERR_MSG_FMT_MOD(extack,				       \
> +				       "No support for %s mask in field %s",   \
> +				       mask_type_name(typ), #_field);	       \
> +		return rc;						       \

We still don't allow flow control to hide inside macros.

You add the checks next to each other (looking at the next patch) 
so you can return rc from the macro and easily combine the checks
into one large if statement. Result - close to ~1 line per check.

> +	}								       \
> +} while (0)
> +
>  int efx_mae_match_check_caps(struct efx_nic *efx,
>  			     const struct efx_tc_match_fields *mask,
>  			     struct netlink_ext_ack *extack)
> @@ -269,6 +284,7 @@ int efx_mae_match_check_caps(struct efx_nic *efx,
>  				       mask_type_name(ingress_port_mask_type));
>  		return rc;
>  	}
> +	CHECK(RECIRC_ID, recirc_id);
>  	return 0;

I think the #undef leaked into the next patch.