HI,
On 10/6/20 10:46 AM, Bharathi, Divya wrote:
Sorry for another mail on same patch. I missed to add one response on
previous comments
No problem.
<snip>
+/**
+ * init_bios_attributes() - Initialize all attributes for a type
+ * @attr_type: The attribute type to initialize
+ * @guid: The WMI GUID associated with this type to initialize
+ *
+ * Initialiaze all 4 types of attributes enumeration, integer, string and
password object.
+ * Populates each attrbute typ's respective properties under sysfs files
+ **/
+static int init_bios_attributes(int attr_type, const char *guid)
+{
+ struct kobject *attr_name_kobj; //individual attribute names
+ union acpi_object *obj = NULL;
+ union acpi_object *elements;
+ struct kset *tmp_set;
+
+ /* instance_id needs to be reset for each type GUID
+ * also, instance IDs are unique within GUID but not across
+ */
+ int instance_id = 0;
+ int retval = 0;
+
+ retval = alloc_attributes_data(attr_type);
+ if (retval)
+ return retval;
+ /* need to use specific instance_id and guid combination to get right
data */
+ obj = get_wmiobj_pointer(instance_id, guid);
+ if (!obj)
+ return -ENODEV;
+ elements = obj->package.elements;
+
+ mutex_lock(&wmi_priv.mutex);
+ while (elements) {
+ /* sanity checking */
+ if (strlen(elements[ATTR_NAME].string.pointer) == 0) {
+ pr_debug("empty attribute found\n");
+ goto nextobj;
+ }
+ if (attr_type == PO)
+ tmp_set = wmi_priv.authentication_dir_kset;
+ else
+ tmp_set = wmi_priv.main_dir_kset;
+
+ if (kset_find_obj(tmp_set,
elements[ATTR_NAME].string.pointer)) {
+ pr_debug("duplicate attribute name found - %s\n",
+ elements[ATTR_NAME].string.pointer);
+ goto nextobj;
+ }
+
+ /* build attribute */
+ attr_name_kobj = kzalloc(sizeof(*attr_name_kobj),
GFP_KERNEL);
+ if (!attr_name_kobj)
+ goto err_attr_init;
+
+ attr_name_kobj->kset = tmp_set;
+
+ retval = kobject_init_and_add(attr_name_kobj,
&attr_name_ktype, NULL, "%s",
+
elements[ATTR_NAME].string.pointer);
+ if (retval) {
+ kobject_put(attr_name_kobj);
+ goto err_attr_init;
+ }
+
+ /* enumerate all of this attribute */
+ switch (attr_type) {
+ case ENUM:
+ retval = populate_enum_data(elements, instance_id,
attr_name_kobj);
+ break;
+ case INT:
+ retval = populate_int_data(elements, instance_id,
attr_name_kobj);
+ break;
+ case STR:
+ retval = populate_str_data(elements, instance_id,
attr_name_kobj);
+ break;
+ case PO:
+ retval = populate_po_data(elements, instance_id,
attr_name_kobj);
+ break;
+ default:
+ break;
+ }
The show functions don't take the mutex and can be called as soon as
kobject_init_and_add() is called, so it would be better to first populate the
data for the current instance_id and only then call kobject_init_and_add()
Populate functions called here for each type of attribute uses
attribute_koject which helps in attribute group cleanup.
Good point, and we do allocate the data before creating the kobjects,
so if a user manages to hit the race (which almost certainly would have
to be done intentionally) then the read would just result in an empty
string (rather then say a null pointer dereference oops).
So lets just keep this as is.
Regards,
Hans
