[PATCH v2 2/4] uacce: fix isolate sysfs check condition

Chenghai Huang posted 4 patches 2 weeks, 1 day ago
[PATCH v2 2/4] uacce: fix isolate sysfs check condition
Posted by Chenghai Huang 2 weeks, 1 day ago
The uacce supports device isolation feature. If the driver
implements the isolate_err_threshold_read and
isolate_err_threshold_write callbacks, the uacce will create sysfs
files. Users can read and configure isolation policies through
sysfs. Currently, if either isolate_err_threshold_read or
isolate_err_threshold_write callback exists, sysfs files are
created.

However, accessing a non-existent callback may cause a system panic.
Therefore, sysfs files are only created when both
isolate_err_threshold_read and isolate_err_threshold_write are
present.

Fixes: e3e289fbc0b5 ("uacce: supports device isolation feature")
Cc: stable@vger.kernel.org
Signed-off-by: Chenghai Huang <huangchenghai2@huawei.com>
---
 drivers/misc/uacce/uacce.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c
index 12370469f646..770a931ef68d 100644
--- a/drivers/misc/uacce/uacce.c
+++ b/drivers/misc/uacce/uacce.c
@@ -441,7 +441,7 @@ static umode_t uacce_dev_is_visible(struct kobject *kobj,
 		return 0;
 
 	if (attr == &dev_attr_isolate_strategy.attr &&
-	    (!uacce->ops->isolate_err_threshold_read &&
+	    (!uacce->ops->isolate_err_threshold_read ||
 	     !uacce->ops->isolate_err_threshold_write))
 		return 0;
 
-- 
2.33.0
Re: [PATCH v2 2/4] uacce: fix isolate sysfs check condition
Posted by Greg KH 2 weeks, 1 day ago
On Tue, Sep 16, 2025 at 10:48:09PM +0800, Chenghai Huang wrote:
> The uacce supports device isolation feature. If the driver
> implements the isolate_err_threshold_read and
> isolate_err_threshold_write callbacks, the uacce will create sysfs
> files. Users can read and configure isolation policies through
> sysfs. Currently, if either isolate_err_threshold_read or
> isolate_err_threshold_write callback exists, sysfs files are
> created.
> 
> However, accessing a non-existent callback may cause a system panic.

Where is the callback happening that fails?  Shouldn't that be checked
instead of doing this change?

> Therefore, sysfs files are only created when both
> isolate_err_threshold_read and isolate_err_threshold_write are
> present.

What if a device only has 1?  That should still work properly?

And why not just create the file if it is going to be used, that is the
real solution here.

thanks,

greg k-h
Re: [PATCH v2 2/4] uacce: fix isolate sysfs check condition
Posted by huangchenghai 2 weeks, 1 day ago
On Mon, Sep 16, 2025 at 11:15 PM +0800, Greg KH wrote:
> On Tue, Sep 16, 2025 at 10:48:09PM +0800, Chenghai Huang wrote:
>> The uacce supports device isolation feature. If the driver
>> implements the isolate_err_threshold_read and
>> isolate_err_threshold_write callbacks, the uacce will create sysfs
>> files. Users can read and configure isolation policies through
>> sysfs. Currently, if either isolate_err_threshold_read or
>> isolate_err_threshold_write callback exists, sysfs files are
>> created.
>>
>> However, accessing a non-existent callback may cause a system panic.
> Where is the callback happening that fails?  Shouldn't that be checked
> instead of doing this change?
>
>> Therefore, sysfs files are only created when both
>> isolate_err_threshold_read and isolate_err_threshold_write are
>> present.
> What if a device only has 1?  That should still work properly?
>
> And why not just create the file if it is going to be used, that is the
> real solution here.
>
> thanks,
>
> greg k-h
Thank you for your feedback.I agree that the check should be done in the 
corresponding `isolate_strategy_show()` and `isolate_strategy_store()` 
functions.

How about the updated:

@@ -402,6 +402,9 @@ static ssize_t isolate_strategy_show(struct device 
*dev, struct device_attribute
         struct uacce_device *uacce = to_uacce_device(dev);
         u32 val;

+       if (!uacce->ops->isolate_err_threshold_read)
+               return -ENOENT;
+
         val = uacce->ops->isolate_err_threshold_read(uacce);

         return sysfs_emit(buf, "%u\n", val);
@@ -414,6 +417,9 @@ static ssize_t isolate_strategy_store(struct device 
*dev, struct device_attribut
         unsigned long val;
         int ret;

+       if (!uacce->ops->isolate_err_threshold_write)
+               return -ENOENT;
+
         if (kstrtoul(buf, 0, &val) < 0)
                 return -EINVAL;

@@ -460,9 +466,7 @@ static umode_t uacce_dev_is_visible(struct kobject 
*kobj,
             (!uacce->qf_pg_num[UACCE_QFRT_DUS])))
                 return 0;

-       if (attr == &dev_attr_isolate_strategy.attr &&
-           (!uacce->ops->isolate_err_threshold_read ||
-            !uacce->ops->isolate_err_threshold_write))
+       if (attr == &dev_attr_isolate_strategy.attr)
                 return 0;

This way, the sysfs files will only be created if they are going to be 
used, and the checks are done at the appropriate places.

Thanks,
Chenghai