Hi Kees,
On Mon, Apr 29, 2024 at 10:36:23AM -0700, Kees Cook wrote:
On Fri, Apr 26, 2024 at 07:22:47PM +0200, Erick Archer wrote:
@@ -3957,17 +3947,18 @@ static void batadv_tt_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv,
num_vlan = ntohs(tt_data->num_vlan);
- if (tvlv_value_len < sizeof(*tt_vlan) * num_vlan)
- flex_size = flex_array_size(tt_data, vlan_data, num_vlan);
- if (tvlv_value_len < flex_size) return;
- tt_vlan = (struct batadv_tvlv_tt_vlan_data *)(tt_data + 1);
- tt_change = (struct batadv_tvlv_tt_change *)(tt_vlan + num_vlan);
- tvlv_value_len -= sizeof(*tt_vlan) * num_vlan;
- tt_change = (struct batadv_tvlv_tt_change *)(tt_data->vlan_data +
num_vlan);This is the only part I'm a little worried about. The math all checks out, but the compiler may get bothered about performing a pointer calculation using the vlan_data flexible array memory. As in, this may be calculated as an array offset, since it is the same as:
&tt_data->vlan_data[num_vlan]
Which, for big endian where __counted_by is in effect, the bounds checker may throw a fit since we're going past the end of the array. In other "multiple trailing flexible array" situations, we've done the addressing from the base pointer, since the compiler either knows the full allocation size or it knows nothing about it (this case, since it came from a "void *" function argument). I would suggest:
tt_change = (struct batadv_tvlv_tt_change *)((void *)tt_data + flex_size);
First of all thanks for the review and the great explanation. The change looks reasonable to me. I will send a new version with this suggestion.
Regards, Erick