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>] [day] [month] [year] [list]
Message-ID: <0f61f5ab-2358-4d1c-ab3c-78c345335bda@intel.com>
Date: Wed, 4 Sep 2024 14:37:23 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
CC: <intel-wired-lan@...ts.osuosl.org>, Larysa Zaremba
	<larysa.zaremba@...el.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, Tony Nguyen <anthony.l.nguyen@...el.com>
Subject: Re: [PATCH iwl-next] ice: make ice_cfg_tx_topo() almost const-correct

On 9/4/24 11:31, Przemek Kitszel wrote:
> Constify ice_cfg_tx_topo() @buf parameter.
> This cascades further down to few more functions.
> 
> I set the boundary on ice_get_set_tx_topo(), which could not be easily
> constified as it is get-or-set, but it's set variant does not change
> provided data, the same holds for ice_aqc_opc_set_tx_topo() in particular.
> That's why there is (u8 *) cast to loose const prior to entering into AQ
> world of const-obliviousness.
> 
> Reviewed-by: Larysa Zaremba <larysa.zaremba@...el.com>
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>

I've forgot to CC netdev, sorry :F
I think there is no need to repost though.
Original URL (content is quoted entriely here) if it is more helpful
https://lore.kernel.org/intel-wired-lan/20240904093135.8795-2-przemyslaw.kitszel@intel.com/

I will make an alias for my git-sent-email invocations ;)

> ---
>   drivers/net/ethernet/intel/ice/ice_ddp.h |  4 +-
>   drivers/net/ethernet/intel/ice/ice_ddp.c | 48 ++++++++++++------------
>   2 files changed, 26 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.h b/drivers/net/ethernet/intel/ice/ice_ddp.h
> index 97f272317475..79551da2a4b0 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ddp.h
> +++ b/drivers/net/ethernet/intel/ice/ice_ddp.h
> @@ -438,7 +438,7 @@ struct ice_pkg_enum {
>   	u32 buf_idx;
>   
>   	u32 type;
> -	struct ice_buf_hdr *buf;
> +	const struct ice_buf_hdr *buf;
>   	u32 sect_idx;
>   	void *sect;
>   	u32 sect_type;
> @@ -467,6 +467,6 @@ ice_pkg_enum_entry(struct ice_seg *ice_seg, struct ice_pkg_enum *state,
>   void *ice_pkg_enum_section(struct ice_seg *ice_seg, struct ice_pkg_enum *state,
>   			   u32 sect_type);
>   
> -int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len);
> +int ice_cfg_tx_topo(struct ice_hw *hw, const void *buf, u32 len);
>   
>   #endif
> diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.c b/drivers/net/ethernet/intel/ice/ice_ddp.c
> index 953262b88a58..f2ac1d265510 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ddp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ddp.c
> @@ -31,7 +31,7 @@ static const struct ice_tunnel_type_scan tnls[] = {
>    * Verifies various attributes of the package file, including length, format
>    * version, and the requirement of at least one segment.
>    */
> -static enum ice_ddp_state ice_verify_pkg(struct ice_pkg_hdr *pkg, u32 len)
> +static enum ice_ddp_state ice_verify_pkg(const struct ice_pkg_hdr *pkg, u32 len)
>   {
>   	u32 seg_count;
>   	u32 i;
> @@ -57,13 +57,13 @@ static enum ice_ddp_state ice_verify_pkg(struct ice_pkg_hdr *pkg, u32 len)
>   	/* all segments must fit within length */
>   	for (i = 0; i < seg_count; i++) {
>   		u32 off = le32_to_cpu(pkg->seg_offset[i]);
> -		struct ice_generic_seg_hdr *seg;
> +		const struct ice_generic_seg_hdr *seg;
>   
>   		/* segment header must fit */
>   		if (len < off + sizeof(*seg))
>   			return ICE_DDP_PKG_INVALID_FILE;
>   
> -		seg = (struct ice_generic_seg_hdr *)((u8 *)pkg + off);
> +		seg = (void *)pkg + off;
>   
>   		/* segment body must fit */
>   		if (len < off + le32_to_cpu(seg->seg_size))
> @@ -119,13 +119,13 @@ static enum ice_ddp_state ice_chk_pkg_version(struct ice_pkg_ver *pkg_ver)
>    *
>    * This helper function validates a buffer's header.
>    */
> -static struct ice_buf_hdr *ice_pkg_val_buf(struct ice_buf *buf)
> +static const struct ice_buf_hdr *ice_pkg_val_buf(const struct ice_buf *buf)
>   {
> -	struct ice_buf_hdr *hdr;
> +	const struct ice_buf_hdr *hdr;
>   	u16 section_count;
>   	u16 data_end;
>   
> -	hdr = (struct ice_buf_hdr *)buf->buf;
> +	hdr = (const struct ice_buf_hdr *)buf->buf;
>   	/* verify data */
>   	section_count = le16_to_cpu(hdr->section_count);
>   	if (section_count < ICE_MIN_S_COUNT || section_count > ICE_MAX_S_COUNT)
> @@ -165,8 +165,8 @@ static struct ice_buf_table *ice_find_buf_table(struct ice_seg *ice_seg)
>    * unexpected value has been detected (for example an invalid section count or
>    * an invalid buffer end value).
>    */
> -static struct ice_buf_hdr *ice_pkg_enum_buf(struct ice_seg *ice_seg,
> -					    struct ice_pkg_enum *state)
> +static const struct ice_buf_hdr *ice_pkg_enum_buf(struct ice_seg *ice_seg,
> +						  struct ice_pkg_enum *state)
>   {
>   	if (ice_seg) {
>   		state->buf_table = ice_find_buf_table(ice_seg);
> @@ -1800,9 +1800,9 @@ int ice_update_pkg(struct ice_hw *hw, struct ice_buf *bufs, u32 count)
>    * success it returns a pointer to the segment header, otherwise it will
>    * return NULL.
>    */
> -static struct ice_generic_seg_hdr *
> +static const struct ice_generic_seg_hdr *
>   ice_find_seg_in_pkg(struct ice_hw *hw, u32 seg_type,
> -		    struct ice_pkg_hdr *pkg_hdr)
> +		    const struct ice_pkg_hdr *pkg_hdr)
>   {
>   	u32 i;
>   
> @@ -1813,11 +1813,9 @@ ice_find_seg_in_pkg(struct ice_hw *hw, u32 seg_type,
>   
>   	/* Search all package segments for the requested segment type */
>   	for (i = 0; i < le32_to_cpu(pkg_hdr->seg_count); i++) {
> -		struct ice_generic_seg_hdr *seg;
> +		const struct ice_generic_seg_hdr *seg;
>   
> -		seg = (struct ice_generic_seg_hdr
> -			       *)((u8 *)pkg_hdr +
> -				  le32_to_cpu(pkg_hdr->seg_offset[i]));
> +		seg = (void *)pkg_hdr + le32_to_cpu(pkg_hdr->seg_offset[i]);
>   
>   		if (le32_to_cpu(seg->seg_type) == seg_type)
>   			return seg;
> @@ -2354,14 +2352,15 @@ ice_get_set_tx_topo(struct ice_hw *hw, u8 *buf, u16 buf_size,
>    *
>    * Return: zero when update was successful, negative values otherwise.
>    */
> -int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len)
> +int ice_cfg_tx_topo(struct ice_hw *hw, const void *buf, u32 len)
>   {
> -	u8 *current_topo, *new_topo = NULL;
> -	struct ice_run_time_cfg_seg *seg;
> -	struct ice_buf_hdr *section;
> -	struct ice_pkg_hdr *pkg_hdr;
> +	const struct ice_run_time_cfg_seg *seg;
> +	const struct ice_buf_hdr *section;
> +	const struct ice_pkg_hdr *pkg_hdr;
> +	const u8 *new_topo = NULL;
>   	enum ice_ddp_state state;
>   	u16 offset, size = 0;
> +	u8 *current_topo;
>   	u32 reg = 0;
>   	int status;
>   	u8 flags;
> @@ -2379,7 +2378,7 @@ int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len)
>   	if (!current_topo)
>   		return -ENOMEM;
>   
> -	/* Get the current Tx topology */
> +	/* Get the current Tx topology flags */
>   	status = ice_get_set_tx_topo(hw, current_topo, ICE_AQ_MAX_BUF_LEN, NULL,
>   				     &flags, false);
>   
> @@ -2419,16 +2418,16 @@ int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len)
>   		goto update_topo;
>   	}
>   
> -	pkg_hdr = (struct ice_pkg_hdr *)buf;
> +	pkg_hdr = (const struct ice_pkg_hdr *)buf;
>   	state = ice_verify_pkg(pkg_hdr, len);
>   	if (state) {
>   		ice_debug(hw, ICE_DBG_INIT, "Failed to verify pkg (err: %d)\n",
>   			  state);
>   		return -EIO;
>   	}
>   
>   	/* Find runtime configuration segment */
> -	seg = (struct ice_run_time_cfg_seg *)
> +	seg = (const struct ice_run_time_cfg_seg *)
>   	      ice_find_seg_in_pkg(hw, SEGMENT_TYPE_ICE_RUN_TIME_CFG, pkg_hdr);
>   	if (!seg) {
>   		ice_debug(hw, ICE_DBG_INIT, "5 layer topology segment is missing\n");
> @@ -2462,7 +2461,7 @@ int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len)
>   	}
>   
>   	/* Get the new topology buffer */
> -	new_topo = ((u8 *)section) + offset;
> +	new_topo = (const u8 *)section + offset;
>   
>   update_topo:
>   	/* Acquire global lock to make sure that set topology issued
> @@ -2485,7 +2484,8 @@ int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len)
>   	}
>   
>   	/* Set new topology */
> -	status = ice_get_set_tx_topo(hw, new_topo, size, NULL, NULL, true);
> +	status = ice_get_set_tx_topo(hw, (u8 *)new_topo, size, NULL, NULL,
> +				     true);
>   	if (status) {
>   		ice_debug(hw, ICE_DBG_INIT, "Failed setting Tx topology\n");
>   		return status;
> 
> base-commit: d6f75d86aa786740ef7a7607685e9e1039e30aab


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ