linux-kernel - Re: [PATCH v2 2/4] uacce: fix isolate sysfs check condition

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ace00212-b02b-4407-98ab-5aff3e0fad77@huawei.com>
Date: Wed, 17 Sep 2025 17:54:13 +0800
From: huangchenghai <huangchenghai2@...wei.com>
To: Greg KH <gregkh@...uxfoundation.org>
CC: <zhangfei.gao@...aro.org>, <wangzhou1@...ilicon.com>,
	<linux-kernel@...r.kernel.org>, <linux-crypto@...r.kernel.org>,
	<linuxarm@...neuler.org>, <fanghao11@...wei.com>, <shenyang39@...wei.com>,
	<liulongfang@...wei.com>, <qianweili@...wei.com>
Subject: Re: [PATCH v2 2/4] uacce: fix isolate sysfs check condition


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