On Monday, August 22, 2022 2:22 PM, Christoph Hellwig <hch@xxxxxx> wrote:
> /*
> * Used in mdev_type_attribute sysfs functions to return the parent struct
> * device
> @@ -85,6 +65,8 @@ static int mdev_device_remove_cb(struct device *dev,
> void *data)
> * @parent: parent structure registered
> * @dev: device structure representing parent device.
> * @mdev_driver: Device driver to bind to the newly created mdev
> + * @types: Array of supported mdev types
> + * @nr_types: Number of entries in @types
> *
> * Registers the @parent stucture as a parent for mdev types and thus mdev
> * devices. The caller needs to hold a reference on @dev that must not be
> @@ -93,20 +75,19 @@ static int mdev_device_remove_cb(struct device
> *dev, void *data)
> * Returns a negative value on error, otherwise 0.
> */
> int mdev_register_parent(struct mdev_parent *parent, struct device *dev,
> - struct mdev_driver *mdev_driver)
> + struct mdev_driver *mdev_driver, struct mdev_type
> **types,
> + unsigned int nr_types)
> {
> char *env_string = "MDEV_STATE=registered";
> char *envp[] = { env_string, NULL };
> int ret;
>
> - /* check for mandatory ops */
> - if (!mdev_driver->supported_type_groups)
> - return -EINVAL;
> -
> memset(parent, 0, sizeof(*parent));
> init_rwsem(&parent->unreg_sem);
> parent->dev = dev;
> parent->mdev_driver = mdev_driver;
> + parent->types = types;
This would potentially introduce a bug. Types is passed from the parent and memory reserved for it is
managed by the parent driver, while if you are doing so, it will be freed when types->kobj is released in
mdev module, i.e. in mdev_type_release, types will be freed as a chunk of memory in heap.
This will lead to unpredictable behavior and require a fix, either in here or in mdev_type_release.
Thanks,
Xin
> + parent->nr_types = nr_types;
>
> if (!mdev_bus_compat_class) {
> mdev_bus_compat_class =
> class_compat_register("mdev_bus");
> +static int mdev_type_add(struct mdev_parent *parent, struct mdev_type
> *type)
> {
> - struct mdev_type *type;
> - struct attribute_group *group =
> - parent->mdev_driver-
> >supported_type_groups[type_group_id];
> int ret;
>
> - if (!group->name) {
> - pr_err("%s: Type name empty!\n", __func__);
> - return ERR_PTR(-EINVAL);
> - }
> -
> - type = kzalloc(sizeof(*type), GFP_KERNEL);
> - if (!type)
> - return ERR_PTR(-ENOMEM);
> -
> type->kobj.kset = parent->mdev_types_kset;
> type->parent = parent;
> /* Pairs with the put in mdev_type_release() */
> get_device(parent->dev);
> - type->type_group_id = type_group_id;
>
> ret = kobject_init_and_add(&type->kobj, &mdev_type_ktype, NULL,
> "%s-%s", dev_driver_string(parent->dev),
> - group->name);
> + type->sysfs_name);
> if (ret) {
> kobject_put(&type->kobj);
> - return ERR_PTR(ret);
> + return ret;
> }
>
> ret = sysfs_create_file(&type->kobj, &mdev_type_attr_create.attr);
