On 09/04/2018 20:24, Jason Gunthorpe wrote:
On Mon, Apr 09, 2018 at 02:18:53PM +0300, Dan Carpenter wrote:Hello Aviad Yehezkel, The patch c6475a0bca30: "IB/mlx5: Add implementation for create and destroy action_xfrm" from Mar 28, 2018, leads to the following static checker warning: drivers/infiniband/hw/mlx5/main.c:3256 mlx5_ib_create_flow_action_esp() error: uninitialized symbol 'action_flags'. drivers/infiniband/hw/mlx5/main.c 3359 static struct ib_flow_action * 3360 mlx5_ib_create_flow_action_esp(struct ib_device *device, 3361 const struct ib_flow_action_attrs_esp *attr, 3362 struct uverbs_attr_bundle *attrs) 3363 { 3364 struct mlx5_ib_dev *mdev = to_mdev(device); 3365 struct ib_uverbs_flow_action_esp_keymat_aes_gcm *aes_gcm; 3366 struct mlx5_accel_esp_xfrm_attrs accel_attrs = {}; 3367 struct mlx5_ib_flow_action *action; 3368 u64 action_flags; ^^^^^^^^^^^^ 3369 u64 flags; 3370 int err = 0; 3371 3372 if (IS_UVERBS_COPY_ERR(uverbs_copy_from(&action_flags, attrs, ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 3373 MLX5_IB_ATTR_CREATE_FLOW_ACTION_FLAGS))) 3374 return ERR_PTR(-EFAULT); 3375 3376 if (action_flags >= (MLX5_FLOW_ACTION_ESP_CREATE_LAST_SUPPORTED << 1)) ^^^^^^^^^^^^ The problem is that IS_UVERBS_COPY_ERR() treats -ENOENT as A-OK but if we hit that error then uverbs_copy_from() is a no-op...It needs to be uverbs_copy_from_or_zero and wrapping in a macro is so obviously hard to use the macro needs to go too. uverbs_zcopy_opt() or something I assume the intent was action_flags is zero'd if not present.. Matan?
That attribute is currently marked as mandatory, so you actually can't get -ENOENT here (as you'll fail earlier in the parsing stage).
Maybe we can always zero the output on -ENOENT. You won't hit that path with MANDATORY attributes and it seems reasonable for optional attributes.
Jason
Matan -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
