Hi, On 2/7/23 13:58, Srinivas Pandruvada wrote: > ID alloc and free functions don't have in built protection for parallel > invocation of ida_alloc() and ida_free(). With the current flow in the > vsec driver, there is no such scenario. But add mutex protection for > potential future changes. > > Suggested-by: Hans de Goede <hdegoede@xxxxxxxxxx> > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx> Thank you for your patch, I've applied this patch to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans Note it will show up in my review-hans branch once I've pushed my local branch there, which might take a while. Once I've run some tests on this branch the patches there will be added to the platform-drivers-x86/for-next branch and eventually will be included in the pdx86 pull-request to Linus for the next merge-window. Regards, Hans > --- > drivers/platform/x86/intel/vsec.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c > index b936fc5776d7..f1680b7e64f5 100644 > --- a/drivers/platform/x86/intel/vsec.c > +++ b/drivers/platform/x86/intel/vsec.c > @@ -129,11 +129,16 @@ static void intel_vsec_remove_aux(void *data) > auxiliary_device_uninit(data); > } > > +static DEFINE_MUTEX(vsec_ida_lock); > + > static void intel_vsec_dev_release(struct device *dev) > { > struct intel_vsec_device *intel_vsec_dev = dev_to_ivdev(dev); > > + mutex_lock(&vsec_ida_lock); > ida_free(intel_vsec_dev->ida, intel_vsec_dev->auxdev.id); > + mutex_unlock(&vsec_ida_lock); > + > kfree(intel_vsec_dev->resource); > kfree(intel_vsec_dev); > } > @@ -145,7 +150,9 @@ int intel_vsec_add_aux(struct pci_dev *pdev, struct device *parent, > struct auxiliary_device *auxdev = &intel_vsec_dev->auxdev; > int ret, id; > > + mutex_lock(&vsec_ida_lock); > ret = ida_alloc(intel_vsec_dev->ida, GFP_KERNEL); > + mutex_unlock(&vsec_ida_lock); > if (ret < 0) { > kfree(intel_vsec_dev); > return ret; > @@ -161,7 +168,9 @@ int intel_vsec_add_aux(struct pci_dev *pdev, struct device *parent, > > ret = auxiliary_device_init(auxdev); > if (ret < 0) { > + mutex_lock(&vsec_ida_lock); > ida_free(intel_vsec_dev->ida, auxdev->id); > + mutex_unlock(&vsec_ida_lock); > kfree(intel_vsec_dev->resource); > kfree(intel_vsec_dev); > return ret;
