On Wed, 14 Sep 2016, Tariq Toukan wrote: > On 14/09/2016 4:53 PM, Sebastian Ott wrote: > > On Wed, 14 Sep 2016, Tariq Toukan wrote: > > > On 14/09/2016 2:09 PM, Sebastian Ott wrote: > > > > If an error occurs in mlx4_init_eq_table the index used in the > > > > err_out_unmap label is one too big which results in a panic in > > > > mlx4_free_eq. This patch fixes the index in the error path. > > > You are right, but your change below does not cover all cases. > > > The full solution looks like this: > > > > > > @@ -1260,7 +1260,7 @@ int mlx4_init_eq_table(struct mlx4_dev *dev) > > > eq); > > > } > > > if (err) > > > - goto err_out_unmap; > > > + goto err_out_unmap_excluded; > > In this case a call to mlx4_create_eq failed. Do you really have to call > > mlx4_free_eq for this index again? > > We agree on this part, that's why here we should goto the _excluded_ label. > For all other parts, we should not exclude the eq in the highest index, and > thus we goto the _non_excluded_ label.
But that's exactly what the original patch does. If the failure is within the for loop at index i, we do the cleanup starting at index i-1. If the failure is after the for loop then i == dev->caps.num_comp_vectors + 1 and we do the cleanup starting at index i == dev->caps.num_comp_vectors. In the latter case your patch would have an out of bounds array access. Regards, Sebastian
