[bug report] w1_therm: adding resolution sysfs entry

[bug report] w1_therm: adding resolution sysfs entry

[Dan Carpenter]

Hello Akira Shimahara,

The patch 308bdb94de0c: "w1_therm: adding resolution sysfs entry"
from May 11, 2020, leads to the following static checker warnings:

drivers/w1/slaves/w1_therm.c:967 w1_therm_add_slave() error: double unlocked '&sl->master->bus_mutex' (orig line 955)
drivers/w1/slaves/w1_therm.c:1867 alarms_store() error: double unlocked '&sl->master->bus_mutex' (orig line 1847)
drivers/w1/slaves/w1_therm.c:617 w1_DS18B20_set_resolution() error: double unlocked '&sl->master->bus_mutex' (orig line 607)
drivers/w1/slaves/w1_therm.c:587 w1_DS18S20_write_data() error: double unlocked '&sl->master->bus_mutex' (orig line 583)

drivers/w1/slaves/w1_therm.c
    915 static int w1_therm_add_slave(struct w1_slave *sl)
    916 {
    917 	struct w1_therm_family_converter *sl_family_conv;
    918 
    919 	/* Allocate memory */
    920 	sl->family_data = kzalloc(sizeof(struct w1_therm_family_data),
    921 		GFP_KERNEL);
    922 	if (!sl->family_data)
    923 		return -ENOMEM;
    924 
    925 	atomic_set(THERM_REFCNT(sl->family_data), 1);
    926 
    927 	/* Get a pointer to the device specific function struct */
    928 	sl_family_conv = device_family(sl);
    929 	if (!sl_family_conv) {
    930 		kfree(sl->family_data);
    931 		return -ENODEV;
    932 	}
    933 	/* save this pointer to the device structure */
    934 	SLAVE_SPECIFIC_FUNC(sl) = sl_family_conv;
    935 
    936 	if (bulk_read_support(sl)) {
    937 		/*
    938 		 * add the sys entry to trigger bulk_read
    939 		 * at master level only the 1st time
    940 		 */
    941 		if (!bulk_read_device_counter) {
    942 			int err = device_create_file(&sl->master->dev,
    943 				&dev_attr_therm_bulk_read);
    944 
    945 			if (err)
    946 				dev_warn(&sl->dev,
    947 				"%s: Device has been added, but bulk read is unavailable. err=%d\n",
    948 				__func__, err);
    949 		}
    950 		/* Increment the counter */
    951 		bulk_read_device_counter++;
    952 	}
    953 
    954 	/* Getting the power mode of the device {external, parasite} */
    955 	SLAVE_POWERMODE(sl) = read_powermode(sl);

Assume the bus_mutex_lock() in read_powermode() fails so we're not
holding the lock.


    956 
    957 	if (SLAVE_POWERMODE(sl) < 0) {
    958 		/* no error returned as device has been added */
    959 		dev_warn(&sl->dev,
    960 			"%s: Device has been added, but power_mode may be corrupted. err=%d\n",
    961 			 __func__, SLAVE_POWERMODE(sl));

Then the comment is correct that we probably end up in a corrupt
situation.

    962 	}
    963 
    964 	/* Getting the resolution of the device */
    965 	if (SLAVE_SPECIFIC_FUNC(sl)->get_resolution) {
    966 		SLAVE_RESOLUTION(sl) =
--> 967 			SLAVE_SPECIFIC_FUNC(sl)->get_resolution(sl);
    968 		if (SLAVE_RESOLUTION(sl) < 0) {
    969 			/* no error returned as device has been added */
    970 			dev_warn(&sl->dev,
    971 				"%s:Device has been added, but resolution may be corrupted. err=%d\n",
    972 				__func__, SLAVE_RESOLUTION(sl));
    973 		}
    974 	}
    975 
    976 	/* Finally initialize convert_triggered flag */
    977 	SLAVE_CONVERT_TRIGGERED(sl) = 0;

regards,
dan carpenter