[PATCH] PCI: endpoint: Use sysfs_emit() in "show" functions

[PATCH] PCI: endpoint: Use sysfs_emit() in "show" functions

[Kunihiko Hayashi]

Convert sprintf() in sysfs "show" functions to sysfs_emit() in order to
check for buffer overruns in sysfs outputs.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/pci/endpoint/functions/pci-epf-ntb.c |  4 ++--
 drivers/pci/endpoint/pci-ep-cfs.c            | 13 ++++++-------
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-ntb.c b/drivers/pci/endpoint/functions/pci-epf-ntb.c
index bce274d..f5dfc63 100644
--- a/drivers/pci/endpoint/functions/pci-epf-ntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-ntb.c
@@ -1918,7 +1918,7 @@ static ssize_t epf_ntb_##_name##_show(struct config_item *item,		\
 	struct config_group *group = to_config_group(item);		\
 	struct epf_ntb *ntb = to_epf_ntb(group);			\
 									\
-	return sprintf(page, "%d\n", ntb->_name);			\
+	return sysfs_emit(page, "%d\n", ntb->_name);			\
 }
 
 #define EPF_NTB_W(_name)						\
@@ -1949,7 +1949,7 @@ static ssize_t epf_ntb_##_name##_show(struct config_item *item,		\
 									\
 	sscanf(#_name, "mw%d", &win_no);				\
 									\
-	return sprintf(page, "%lld\n", ntb->mws_size[win_no - 1]);	\
+	return sysfs_emit(page, "%lld\n", ntb->mws_size[win_no - 1]);	\
 }
 
 #define EPF_NTB_MW_W(_name)						\
diff --git a/drivers/pci/endpoint/pci-ep-cfs.c b/drivers/pci/endpoint/pci-ep-cfs.c
index f3a8b83..b40a42f 100644
--- a/drivers/pci/endpoint/pci-ep-cfs.c
+++ b/drivers/pci/endpoint/pci-ep-cfs.c
@@ -198,8 +198,7 @@ static ssize_t pci_epc_start_store(struct config_item *item, const char *page,
 
 static ssize_t pci_epc_start_show(struct config_item *item, char *page)
 {
-	return sprintf(page, "%d\n",
-		       to_pci_epc_group(item)->start);
+	return sysfs_emit(page, "%d\n", to_pci_epc_group(item)->start);
 }
 
 CONFIGFS_ATTR(pci_epc_, start);
@@ -321,7 +320,7 @@ static ssize_t pci_epf_##_name##_show(struct config_item *item,	char *page)    \
 	struct pci_epf *epf = to_pci_epf_group(item)->epf;		       \
 	if (WARN_ON_ONCE(!epf->header))					       \
 		return -EINVAL;						       \
-	return sprintf(page, "0x%04x\n", epf->header->_name);		       \
+	return sysfs_emit(page, "0x%04x\n", epf->header->_name);	       \
 }
 
 #define PCI_EPF_HEADER_W_u32(_name)					       \
@@ -390,8 +389,8 @@ static ssize_t pci_epf_msi_interrupts_store(struct config_item *item,
 static ssize_t pci_epf_msi_interrupts_show(struct config_item *item,
 					   char *page)
 {
-	return sprintf(page, "%d\n",
-		       to_pci_epf_group(item)->epf->msi_interrupts);
+	return sysfs_emit(page, "%d\n",
+			  to_pci_epf_group(item)->epf->msi_interrupts);
 }
 
 static ssize_t pci_epf_msix_interrupts_store(struct config_item *item,
@@ -412,8 +411,8 @@ static ssize_t pci_epf_msix_interrupts_store(struct config_item *item,
 static ssize_t pci_epf_msix_interrupts_show(struct config_item *item,
 					    char *page)
 {
-	return sprintf(page, "%d\n",
-		       to_pci_epf_group(item)->epf->msix_interrupts);
+	return sysfs_emit(page, "%d\n",
+			  to_pci_epf_group(item)->epf->msix_interrupts);
 }
 
 PCI_EPF_HEADER_R(vendorid)
-- 
2.7.4

Re: [PATCH] PCI: endpoint: Use sysfs_emit() in "show" functions

[Krzysztof Wilczyński]

Hello Hayashi-san,

Thank you for sending the patch over!

> Convert sprintf() in sysfs "show" functions to sysfs_emit() in order to
> check for buffer overruns in sysfs outputs.

Nice catch!

A small nitpick: what you are changing here are technically not sysfs
objects since all of these are related to configfs.  Having said that,
configfs shares the same semantics for normal attributes with sysfs, so
a maximum size of PAGE_SIZE applies here too, and thus sysfs_emit()
would work fine.

Thank you for taking care of this!

Reviewed-by: Krzysztof Wilczyński <kw@linux.com>

	Krzysztof

Re: [PATCH] PCI: endpoint: Use sysfs_emit() in "show" functions

[Kunihiko Hayashi]

Hi Krzysztof,

Thank you for reviewing.

On 2021/07/19 12:43, Krzysztof Wilczyński wrote:
> Hello Hayashi-san,
> 
> Thank you for sending the patch over!
> 
>> Convert sprintf() in sysfs "show" functions to sysfs_emit() in order to
>> check for buffer overruns in sysfs outputs.
> 
> Nice catch!

I actually executed "cat" against configfs to meet the issue and found
your solution in pci-sysfs.

> 
> A small nitpick: what you are changing here are technically not sysfs
> objects since all of these are related to configfs.  Having said that,
> configfs shares the same semantics for normal attributes with sysfs, so
> a maximum size of PAGE_SIZE applies here too, and thus sysfs_emit()
> would work fine.

Thank you for helpful information.
I understand that applying sysfs_emit() to configfs is no problem.

> 
> Thank you for taking care of this!
> 
> Reviewed-by: Krzysztof Wilczyński <kw@linux.com>
> 
> 	Krzysztof
> 
Thank you,

---
Best Regards
Kunihiko Hayashi

Re: [PATCH] PCI: endpoint: Use sysfs_emit() in "show" functions

[Krzysztof Wilczyński]

[+cc Sasha for visibility]

Hi!

[...]
> > Nice catch!
> 
> I actually executed "cat" against configfs to meet the issue and found
> your solution in pci-sysfs.

Oh!  That's not good...  I am curious, which attribute caused this?

Also, if this is fixing a bug, then it might warrant letting the folks who look
after the long-term and stable kernels know.  I also wonder if there would be
something to add for the "Fixes:" tag, if there is a previous commit this
change fixes.

	Krzysztof

Re: [PATCH] PCI: endpoint: Use sysfs_emit() in "show" functions

[Kunihiko Hayashi]

Hi Krzysztof,

On 2021/07/20 0:18, Krzysztof Wilczy?ski wrote:
 > [+cc Sasha for visibility]
 >
 > Hi!
 >
 > [...]
 >>> Nice catch!
 >>
 >> I actually executed "cat" against configfs to meet the issue and found
 >> your solution in pci-sysfs.
 >
 > Oh!  That's not good...  I am curious, which attribute caused this?

Sorry I misunderstood.

I found this "cat" issue on next-20210713 and all configfs attribues had
the same issue.

However, my patch wasn't the solution for the issue. This has been fixed by
7fe1e79b59ba ("configfs: implement the .read_iter and .write_iter methods").

The function replacement was found in the process of finding the issue.

 > Also, if this is fixing a bug, then it might warrant letting the folks who look
 > after the long-term and stable kernels know.  I also wonder if there would be
 > something to add for the "Fixes:" tag, if there is a previous commit this
 > change fixes.

So my patch doesn't fix any issues.

Thank you,

---
Best Regards
Kunihiko Hayashi