net: microchip: sparx5: Support for copying and modifying rules in the API

Hello Steen Hegelund,

Commit 465a38a269e9 ("net: microchip: sparx5: Support for copying and modifying rules in the API") from Nov 23, 2022 (linux-next), leads to the following Smatch static checker warning:

drivers/net/ethernet/microchip/vcap/vcap_api.c:3516 vcap_filter_rule_keys()
error: double free of 'ckf' (line 3516)

drivers/net/ethernet/microchip/vcap/vcap_api.c

    3506         int idx;
    3507 
    3508         if (length > 0) {
    3509                 err = -EEXIST;
    3510                 list_for_each_entry_safe(ckf, next_ckf,
    3511                                          &ri->data.keyfields, ctrl.list) {
    3512                         key = ckf->ctrl.key;
    3513                         for (idx = 0; idx < length; ++idx)
    3514                                 if (key == keylist[idx]) {
    3515                                         list_del(&ckf->ctrl.list);
--> 3516                                         kfree(ckf);
    3517                                         idx++;

The "idx++" makes it look like we're going to keep iterating through this inner loop. Why not just break instead? If we keep iterating and find another match it leads to a double free.

    3518                                         err = 0;
    3519                                 }
    3520                 }
    3521         }
    3522         if (drop_unsupported) {
    3523                 err = -EEXIST;
    3524                 fields = vcap_keyfields(ri->vctrl, ri->admin->vtype,
    3525                                         rule->keyset);
    3526                 if (!fields)
    3527                         return err;
    3528                 list_for_each_entry_safe(ckf, next_ckf,
    3529                                          &ri->data.keyfields, ctrl.list) {
    3530                         key = ckf->ctrl.key;
    3531                         if (fields[key].width == 0) {
    3532                                 list_del(&ckf->ctrl.list);
    3533                                 kfree(ckf);
    3534                                 err = 0;
    3535                         }
    3536                 }
    3537         }
    3538         return err;
    3539 }

This email is a free service from the Smatch-CI project [smatch.sf.net].

regards,
dan carpenter