From 4a2b06ca33763b363038d333274e212db6ff0de1 Mon Sep 17 00:00:00 2001 From: Li Zhijian Date: Mon, 22 Jan 2024 13:39:41 +0800 Subject: firewire: Kill unnecessary buf check in device_attribute.show Per Documentation/filesystems/sysfs.rst: > sysfs allocates a buffer of size (PAGE_SIZE) and passes it to the > method. So we can kill the unnecessary buf check safely. Signed-off-by: Li Zhijian Link: https://lore.kernel.org/r/20240122053942.80648-1-lizhijian@fujitsu.com Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-device.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) (limited to 'drivers/firewire/core-device.c') diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index 7d3346b3a2bf..3a1a2bf1717c 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -322,7 +322,7 @@ static ssize_t show_immediate(struct device *dev, if (value < 0) return -ENOENT; - return snprintf(buf, buf ? PAGE_SIZE : 0, "0x%06x\n", value); + return snprintf(buf, PAGE_SIZE, "0x%06x\n", value); } #define IMMEDIATE_ATTR(name, key) \ @@ -334,8 +334,6 @@ static ssize_t show_text_leaf(struct device *dev, struct config_rom_attribute *attr = container_of(dattr, struct config_rom_attribute, attr); const u32 *directories[] = {NULL, NULL}; - size_t bufsize; - char dummy_buf[2]; int i, ret = -ENOENT; down_read(&fw_device_rwsem); @@ -357,15 +355,9 @@ static ssize_t show_text_leaf(struct device *dev, } } - if (buf) { - bufsize = PAGE_SIZE - 1; - } else { - buf = dummy_buf; - bufsize = 1; - } - for (i = 0; i < ARRAY_SIZE(directories) && !!directories[i]; ++i) { - int result = fw_csr_string(directories[i], attr->key, buf, bufsize); + int result = fw_csr_string(directories[i], attr->key, buf, + PAGE_SIZE - 1); // Detected. if (result >= 0) { ret = result; -- cgit v1.2.3 From d4db89c34521a83371fd46bea34834dff128a5cf Mon Sep 17 00:00:00 2001 From: Li Zhijian Date: Mon, 22 Jan 2024 13:39:42 +0800 Subject: firewire: Convert snprintf/sprintf to sysfs_emit Per filesystems/sysfs.rst, show() should only use sysfs_emit() or sysfs_emit_at() when formatting the value to be returned to user space. coccinelle complains that there are still a couple of functions that use snprintf(). Convert them to sysfs_emit(). > drivers/firewire/core-device.c:326:8-16: WARNING: please use sysfs_emit or sysfs_emit_at No functional change intended Signed-off-by: Li Zhijian Link: https://lore.kernel.org/r/20240122053942.80648-2-lizhijian@fujitsu.com Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/firewire/core-device.c') diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index 3a1a2bf1717c..a802c6d4f4fd 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -322,7 +322,7 @@ static ssize_t show_immediate(struct device *dev, if (value < 0) return -ENOENT; - return snprintf(buf, PAGE_SIZE, "0x%06x\n", value); + return sysfs_emit(buf, "0x%06x\n", value); } #define IMMEDIATE_ATTR(name, key) \ @@ -482,7 +482,7 @@ static ssize_t is_local_show(struct device *dev, { struct fw_device *device = fw_device(dev); - return sprintf(buf, "%u\n", device->is_local); + return sysfs_emit(buf, "%u\n", device->is_local); } static int units_sprintf(char *buf, const u32 *directory) -- cgit v1.2.3 From 04f082d39b99f0b7b4b1cada14280f41d99f1e1f Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Mon, 5 Feb 2024 15:04:48 +0900 Subject: firewire: core: fix build failure due to the caller of fw_csr_string() A commit 47dc55181dcb ("firewire: core: search descriptor leaf just after vendor directory entry in root directory") for v6.8-rc3 and a commit 67a5a58c0443 ("firewire: Kill unnecessary buf check in device_attribute.show") for v6.9 bring build failure in for-next tree due to the change of the name of local variable. This commit fixes it. Reported-by: Stephen Rothwell Closes: https://lore.kernel.org/lkml/20240202111602.6f6e2c1a@canb.auug.org.au/ Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202402022343.NkgsMITA-lkp@intel.com/ Link: https://lore.kernel.org/r/20240205060448.13881-1-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/firewire/core-device.c') diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index a802c6d4f4fd..c0976f6268d3 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -366,7 +366,7 @@ static ssize_t show_text_leaf(struct device *dev, // in the root directory follows to the directory entry for vendor ID // instead of the immediate value for vendor ID. result = fw_csr_string(directories[i], CSR_DIRECTORY | attr->key, buf, - bufsize); + PAGE_SIZE - 1); if (result >= 0) ret = result; } -- cgit v1.2.3 From 946593d1555921720fa674432e998a1b5931ddac Mon Sep 17 00:00:00 2001 From: Li Zhijian Date: Mon, 18 Mar 2024 14:05:32 +0800 Subject: Revert "firewire: Kill unnecessary buf check in device_attribute.show" This reverts commit 4a2b06ca33763b363038d333274e212db6ff0de1. The previous fix didn't consider callers from other than sysfs. Revert it to fix the NULL dereference kernel: ? sysfs_emit+0xb5/0xc0 kernel: show_immediate+0x13f/0x1d0 [firewire_core] kernel: init_fw_attribute_group+0x81/0x150 [firewire_core] kernel: create_units+0x119/0x160 [firewire_core] kernel: fw_device_init+0x1a9/0x330 [firewire_core] kernel: fw_device_workfn+0x12/0x20 [firewire_core] kernel: process_one_work+0x16f/0x350 kernel: worker_thread+0x306/0x440 kernel: ? __pfx_worker_thread+0x10/0x10 kernel: kthread+0xf2/0x120 kernel: ? __pfx_kthread+0x10/0x10 kernel: ret_from_fork+0x47/0x70 kernel: ? __pfx_kthread+0x10/0x10 kernel: ret_from_fork_asm+0x1b/0x30 kernel: kernel: ---[ end trace 0000000000000000 ]--- kernel: ------------[ cut here ]------------ Fixes: 4a2b06ca3376 ("firewire: Kill unnecessary buf check in device_attribute.show") Reported-by: Takashi Sakamoto Signed-off-by: Li Zhijian Link: https://lore.kernel.org/lkml/625470f3-b196-43f7-9844-fa1cb6da99f8@fujitsu.com/ Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-device.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) (limited to 'drivers/firewire/core-device.c') diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index c0976f6268d3..f208a02d0ebf 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -322,7 +322,7 @@ static ssize_t show_immediate(struct device *dev, if (value < 0) return -ENOENT; - return sysfs_emit(buf, "0x%06x\n", value); + return buf ? sysfs_emit(buf, "0x%06x\n", value) : 0; } #define IMMEDIATE_ATTR(name, key) \ @@ -334,6 +334,8 @@ static ssize_t show_text_leaf(struct device *dev, struct config_rom_attribute *attr = container_of(dattr, struct config_rom_attribute, attr); const u32 *directories[] = {NULL, NULL}; + size_t bufsize; + char dummy_buf[2]; int i, ret = -ENOENT; down_read(&fw_device_rwsem); @@ -355,9 +357,15 @@ static ssize_t show_text_leaf(struct device *dev, } } + if (buf) { + bufsize = PAGE_SIZE - 1; + } else { + buf = dummy_buf; + bufsize = 1; + } + for (i = 0; i < ARRAY_SIZE(directories) && !!directories[i]; ++i) { - int result = fw_csr_string(directories[i], attr->key, buf, - PAGE_SIZE - 1); + int result = fw_csr_string(directories[i], attr->key, buf, bufsize); // Detected. if (result >= 0) { ret = result; @@ -366,7 +374,7 @@ static ssize_t show_text_leaf(struct device *dev, // in the root directory follows to the directory entry for vendor ID // instead of the immediate value for vendor ID. result = fw_csr_string(directories[i], CSR_DIRECTORY | attr->key, buf, - PAGE_SIZE - 1); + bufsize); if (result >= 0) ret = result; } -- cgit v1.2.3 From bfb1ad3c6aab2341ace13222ac0a78e5b4c239c8 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Mon, 18 Mar 2024 17:59:14 +0900 Subject: firewire: core: add memo about the caller of show functions for device attributes In the case of firewire core function, the caller of show functions for device attributes is not only sysfs user, but also device initialization. This commit adds memo about it against the typical assumption that the functions are just dedicated to sysfs user. Link: https://lore.kernel.org/lkml/20240318091759.678326-1-o-takashi@sakamocchi.jp/ Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-device.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/firewire/core-device.c') diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index f208a02d0ebf..e6cdb905eeac 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -322,6 +322,7 @@ static ssize_t show_immediate(struct device *dev, if (value < 0) return -ENOENT; + // Note that this function is also called by init_fw_attribute_group() with NULL pointer. return buf ? sysfs_emit(buf, "0x%06x\n", value) : 0; } @@ -357,6 +358,7 @@ static ssize_t show_text_leaf(struct device *dev, } } + // Note that this function is also called by init_fw_attribute_group() with NULL pointer. if (buf) { bufsize = PAGE_SIZE - 1; } else { -- cgit v1.2.3