Re: [PATCH net 06/10] iavf: prevent accidental free of filter structure
From: Stefan Assmann <hidden>
Date: 2021-11-16 07:24:48
On 2021-11-15 15:59, Tony Nguyen wrote:
From: Jacob Keller <jacob.e.keller@intel.com> In iavf_config_clsflower, the filter structure could be accidentally released at the end, if iavf_parse_cls_flower or iavf_handle_tclass ever return a non-zero but positive value. In this case, the function continues through to the end, and will call kfree() on the filter structure even though it has been added to the linked list. This can actually happen because iavf_parse_cls_flower will return a positive IAVF_ERR_CONFIG value instead of the traditional negative error codes.
Hi Jacob,
where exactly does this happen?
Looking at iavf_parse_cls_flower() I see all returns of IAVF_ERR_CONFIG
as "return IAVF_ERR_CONFIG;" while IAVF_ERR_CONFIG is defined as
IAVF_ERR_CONFIG = -4,
I'm not opposed to this change, just wondering what's going on.
Stefan
quoted hunk ↗ jump to hunk
Fix this by ensuring that the kfree() check and error checks are similar. Use the more idiomatic "if (err)" to catch all non-zero error codes. Fixes: 0075fa0fadd0 ("i40evf: Add support to apply cloud filters") Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Tested-by: Tony Brelinski <redacted> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com> --- drivers/net/ethernet/intel/iavf/iavf_main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c index 76c4ca0f055e..9c68c8628512 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_main.c +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c@@ -3108,11 +3108,11 @@ static int iavf_configure_clsflower(struct iavf_adapter *adapter, /* start out with flow type and eth type IPv4 to begin with */ filter->f.flow_type = VIRTCHNL_TCP_V4_FLOW; err = iavf_parse_cls_flower(adapter, cls_flower, filter); - if (err < 0) + if (err) goto err; err = iavf_handle_tclass(adapter, tc, filter); - if (err < 0) + if (err) goto err; /* add filter to the list */-- 2.31.1