[PATCH v2 0/3] configfs: fix bugs

[PATCH v2 0/3] configfs: fix bugs

[Zijun Hu]

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
Changes in v2:
- Drop the last patch which seems wrong.
- Link to v1: https://lore.kernel.org/r/20250408-fix_configfs-v1-0-5a4c88805df7@quicinc.com

---
Zijun Hu (3):
      configfs: Delete semicolon from macro type_print() definition
      configfs: Do not override creating attribute file failure in populate_attrs()
      configfs: Correct error value returned by API config_item_set_name()

 fs/configfs/dir.c  | 4 ++--
 fs/configfs/item.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)
---
base-commit: 38fec10eb60d687e30c8c6b5420d86e8149f7557
change-id: 20250408-fix_configfs-699743163c64

Best regards,
-- 
Zijun Hu <quic_zijuhu@quicinc.com>

[PATCH v2 1/3] configfs: Delete semicolon from macro type_print() definition

[Zijun Hu]

From: Zijun Hu <quic_zijuhu@quicinc.com>

Macro type_print() definition ends with semicolon, so will cause
the subsequent macro invocations end with two semicolons.

Fix by deleting the semicolon from the macro definition.

Reviewed-by: Joel Becker <jlbec@evilplan.org>
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 fs/configfs/dir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index 7d10278db30d667d0ef7e1140e54961c5a440c41..0a011bdad98c492227859ff328d61aeed2071e24 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -970,7 +970,7 @@ static void configfs_dump_one(struct configfs_dirent *sd, int level)
 {
 	pr_info("%*s\"%s\":\n", level, " ", configfs_get_name(sd));
 
-#define type_print(_type) if (sd->s_type & _type) pr_info("%*s %s\n", level, " ", #_type);
+#define type_print(_type) if (sd->s_type & _type) pr_info("%*s %s\n", level, " ", #_type)
 	type_print(CONFIGFS_ROOT);
 	type_print(CONFIGFS_DIR);
 	type_print(CONFIGFS_ITEM_ATTR);

-- 
2.34.1

[PATCH v2 2/3] configfs: Do not override creating attribute file failure in populate_attrs()

[Zijun Hu]

From: Zijun Hu <quic_zijuhu@quicinc.com>

populate_attrs() may override failure for creating attribute files
by success for creating subsequent bin attribute files, and have
wrong return value.

Fix by creating bin attribute files under successfully creating
attribute files.

Fixes: 03607ace807b ("configfs: implement binary attributes")
Cc: stable@vger.kernel.org
Reviewed-by: Joel Becker <jlbec@evilplan.org>
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 fs/configfs/dir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index 0a011bdad98c492227859ff328d61aeed2071e24..64272d3946cc40757dca063190829958517eceb3 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -619,7 +619,7 @@ static int populate_attrs(struct config_item *item)
 				break;
 		}
 	}
-	if (t->ct_bin_attrs) {
+	if (!error && t->ct_bin_attrs) {
 		for (i = 0; (bin_attr = t->ct_bin_attrs[i]) != NULL; i++) {
 			if (ops && ops->is_bin_visible && !ops->is_bin_visible(item, bin_attr, i))
 				continue;

-- 
2.34.1

[PATCH v2 3/3] configfs: Correct error value returned by API config_item_set_name()

[Zijun Hu]

From: Zijun Hu <quic_zijuhu@quicinc.com>

kvasprintf() failure is often caused by memory allocation which has error
code -ENOMEM, but config_item_set_name() returns -EFAULT for the failure.

Fix by returning -ENOMEM instead of -EFAULT for the failure.

Reviewed-by: Joel Becker <jlbec@evilplan.org>
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 fs/configfs/item.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/configfs/item.c b/fs/configfs/item.c
index 254170a82aa337d95cbfbdf1a2db1023db3a2907..c378b5cbf87d28387a509c3cabb93eccfb520c9c 100644
--- a/fs/configfs/item.c
+++ b/fs/configfs/item.c
@@ -66,7 +66,7 @@ int config_item_set_name(struct config_item *item, const char *fmt, ...)
 		name = kvasprintf(GFP_KERNEL, fmt, args);
 		va_end(args);
 		if (!name)
-			return -EFAULT;
+			return -ENOMEM;
 	}
 
 	/* Free the old name, if necessary. */

-- 
2.34.1

Re: [PATCH v2 3/3] configfs: Correct error value returned by API config_item_set_name()

[Breno Leitao]

On Tue, Apr 15, 2025 at 08:34:27PM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
> 
> kvasprintf() failure is often caused by memory allocation which has error
> code -ENOMEM, but config_item_set_name() returns -EFAULT for the failure.
> 
> Fix by returning -ENOMEM instead of -EFAULT for the failure.
> 
> Reviewed-by: Joel Becker <jlbec@evilplan.org>
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>

Reviewed-by: Breno Leitao <leitao@debian.org>

> ---
>  fs/configfs/item.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/configfs/item.c b/fs/configfs/item.c
> index 254170a82aa337d95cbfbdf1a2db1023db3a2907..c378b5cbf87d28387a509c3cabb93eccfb520c9c 100644
> --- a/fs/configfs/item.c
> +++ b/fs/configfs/item.c
> @@ -66,7 +66,7 @@ int config_item_set_name(struct config_item *item, const char *fmt, ...)
>  		name = kvasprintf(GFP_KERNEL, fmt, args);
>  		va_end(args);
>  		if (!name)
> -			return -EFAULT;
> +			return -ENOMEM;

I've checked that all the config_item_set_name() callers, who check for
the return value, are fine with this change also.

--breno

Re: [PATCH v2 2/3] configfs: Do not override creating attribute file failure in populate_attrs()

[Breno Leitao]

On Tue, Apr 15, 2025 at 08:34:26PM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
> 
> populate_attrs() may override failure for creating attribute files
> by success for creating subsequent bin attribute files, and have
> wrong return value.
> 
> Fix by creating bin attribute files under successfully creating
> attribute files.
> 
> Fixes: 03607ace807b ("configfs: implement binary attributes")
> Cc: stable@vger.kernel.org
> Reviewed-by: Joel Becker <jlbec@evilplan.org>
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>

Reviewed-by: Breno Leitao <leitao@debian.org>

Re: [PATCH v2 1/3] configfs: Delete semicolon from macro type_print() definition

[Breno Leitao]

On Tue, Apr 15, 2025 at 08:34:25PM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
> 
> Macro type_print() definition ends with semicolon, so will cause
> the subsequent macro invocations end with two semicolons.
> 
> Fix by deleting the semicolon from the macro definition.
> 
> Reviewed-by: Joel Becker <jlbec@evilplan.org>
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
> ---
>  fs/configfs/dir.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
> index 7d10278db30d667d0ef7e1140e54961c5a440c41..0a011bdad98c492227859ff328d61aeed2071e24 100644
> --- a/fs/configfs/dir.c
> +++ b/fs/configfs/dir.c
> @@ -970,7 +970,7 @@ static void configfs_dump_one(struct configfs_dirent *sd, int level)
>  {
>  	pr_info("%*s\"%s\":\n", level, " ", configfs_get_name(sd));
>  
> -#define type_print(_type) if (sd->s_type & _type) pr_info("%*s %s\n", level, " ", #_type);
> +#define type_print(_type) if (sd->s_type & _type) pr_info("%*s %s\n", level, " ", #_type)
>  	type_print(CONFIGFS_ROOT);
>  	type_print(CONFIGFS_DIR);
>  	type_print(CONFIGFS_ITEM_ATTR);

As I've asked in V1. Is this macro being used?

--breno

Re: [PATCH v2 1/3] configfs: Delete semicolon from macro type_print() definition

[huzijun]

> 在 2025年4月17日，22:58，Breno Leitao <leitao@debian.org> 写道：
> 
> On Tue, Apr 15, 2025 at 08:34:25PM +0800, Zijun Hu wrote:
>> From: Zijun Hu <quic_zijuhu@quicinc.com>
>> 
>> Macro type_print() definition ends with semicolon, so will cause
>> the subsequent macro invocations end with two semicolons.
>> 
>> Fix by deleting the semicolon from the macro definition.
>> 
>> Reviewed-by: Joel Becker <jlbec@evilplan.org>
>> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
>> ---
>> fs/configfs/dir.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
>> index 7d10278db30d667d0ef7e1140e54961c5a440c41..0a011bdad98c492227859ff328d61aeed2071e24 100644
>> --- a/fs/configfs/dir.c
>> +++ b/fs/configfs/dir.c
>> @@ -970,7 +970,7 @@ static void configfs_dump_one(struct configfs_dirent *sd, int level)
>> {
>>    pr_info("%*s\"%s\":\n", level, " ", configfs_get_name(sd));
>> 
>> -#define type_print(_type) if (sd->s_type & _type) pr_info("%*s %s\n", level, " ", #_type);
>> +#define type_print(_type) if (sd->s_type & _type) pr_info("%*s %s\n", level, " ", #_type)
>>    type_print(CONFIGFS_ROOT);
>>    type_print(CONFIGFS_DIR);
>>    type_print(CONFIGFS_ITEM_ATTR);
> 
> As I've asked in V1. Is this macro being used?
> 
yes. it is being used by those 3 statements which follows the macro definition (^^)
> --breno