sysfs: drivers core: Add and use sysfs_emit and sysfs_emit_at
Subsystems (11)
v3 cover sysfs: drivers core: Add and use sysfs_emit and sysfs_emit_at
Output defects can exist in sysfs content using sprintf and snprintf.
sprintf does not know the PAGE_SIZE maximum of the temporary buffer used for outputting sysfs content and it's possible to overrun the PAGE_SIZE buffer length.
Add a generic sysfs_emit function that knows that the size of the temporary buffer and ensures that no overrun is done.
Add a generic sysfs_emit_at function that can be used in multiple call situations that also ensures that no overrun is done.
V2: Add drivers core conversions
V3: Add a few more neatenings and conversions to drivers core
Add mm hugetlb_report_node_meminfo conversion
Joe Perches (8):
sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output drivers core: Use sysfs_emit and sysfs_emit_at for show(device *...) functions drivers core: Remove strcat uses around sysfs_emit and neaten drivers core: Reindent a couple uses around sysfs_emit
drivers core: Miscellaneous changes for sysfs_emit
mm: and drivers core: Convert hugetlb_report_node_meminfo to sysfs_emit drivers core: Use sysfs_emit for shared_cpu_map_show and shared_cpu_list_show drivers core: node: Use a more typical macro definition style for ACCESS_ATTR
Documentation/filesystems/sysfs.rst | 8 +- drivers/base/arch_topology.c | 2 +- drivers/base/bus.c | 2 +- drivers/base/cacheinfo.c | 49 ++-- drivers/base/class.c | 2 +- drivers/base/core.c | 59 +++-- drivers/base/cpu.c | 84 ++++--- drivers/base/dd.c | 3 +- drivers/base/devcoredump.c | 2 +- drivers/base/firmware_loader/fallback.c | 4 +- drivers/base/memory.c | 62 ++--- drivers/base/node.c | 306 ++++++++++++------------ drivers/base/platform.c | 17 +- drivers/base/power/sysfs.c | 160 ++++++++----- drivers/base/power/wakeup_stats.c | 17 +- drivers/base/soc.c | 64 +++-- drivers/base/topology.c | 10 +- fs/sysfs/file.c | 55 +++++ include/linux/hugetlb.h | 4 +- include/linux/sysfs.h | 15 ++ mm/hugetlb.c | 18 +- 21 files changed, 532 insertions(+), 411 deletions(-)
Signature · 1 line
2.26.0
PATCH v3 1/8 sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
Output defects can exist in sysfs content using sprintf and snprintf.
sprintf does not know the PAGE_SIZE maximum of the temporary buffer used for outputting sysfs content and it's possible to overrun the PAGE_SIZE buffer length.
Add a generic sysfs_emit function that knows that the size of the temporary buffer and ensures that no overrun is done.
Add a generic sysfs_emit_at function that can be used in multiple call situations that also ensures that no overrun is done.
Validate the output buffer argument to be page aligned. Validate the offset len argument to be within the PAGE_SIZE buf.
--- Documentation/filesystems/sysfs.rst | 8 ++--- fs/sysfs/file.c | 55 +++++++++++++++++++++++++++++ include/linux/sysfs.h | 15 ++++++++ 3 files changed, 73 insertions(+), 5 deletions(-)
Documentation/filesystems/sysfs.rst +3 -5
@@ -241,12 +241,10 @@ Other notes: is 4096. - show() methods should return the number of bytes printed into the - buffer. This is the return value of scnprintf(). + buffer. -- show() must not use snprintf() when formatting the value to be - returned to user space. If you can guarantee that an overflow - will never happen you can use sprintf() otherwise you must use - scnprintf(). +- show() should only use sysfs_emit() or sysfs_emit_at() when formatting + the value to be returned to user space. - store() should return the number of bytes used from the buffer. If the entire buffer has been used, just return the count argument.
fs/sysfs/file.c +55 -0
64 lines not on this page · raw message
include/linux/sysfs.h +15 -0
@@ -329,6 +329,10 @@ int sysfs_groups_change_owner(struct kobject *kobj,int sysfs_group_change_owner (struct kobject *kobj,const struct attribute_group *groups,kuid_t kuid,kgid_t kgid); +__printf(2, 3) +int sysfs_emit (char *buf,const char *fmt, ...); +__printf(3, 4) +int sysfs_emit_at (char *buf,int at,const char *fmt, ...);#else /* CONFIG_SYSFS */
@@ -576,6 +580,17 @@ static inline int sysfs_group_change_owner(struct kobject *kobj,return 0 ; } +__printf(2, 3) +static inline int sysfs_emit(char *buf, const char *fmt, ...) +{ +return 0 ; +} + +__printf(3, 4) +static inline int sysfs_emit_at (char *buf,int at,const char *fmt, ...) +{ +return 0 ; +}#endif /* CONFIG_SYSFS */ static inline int __must_check sysfs_create_file(struct kobject *kobj,
Signature · 1 line
2.26.0
Greg Kroah-Hartman
maintainerRe: [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
Kees, and Rafael, I don't know if you saw this proposal from Joe for sysfs files, questions below:
Quoted 102 lines · Joe Perches
Output defects can exist in sysfs content using sprintf and snprintf.
sprintf does not know the
PAGE_SIZEmaximum of the temporary buffer used for outputting sysfs content and it's possible to overrun thePAGE_SIZEbuffer length.Add a generic
sysfs_emitfunction that knows that the size of the temporary buffer and ensures that no overrun is done.Add a generic
sysfs_emit_atfunction that can be used in multiple call situations that also ensures that no overrun is done.Validate the output buffer argument to be page aligned. Validate the offset len argument to be within the
PAGE_SIZEbuf.Signed-off-by Joe Perches <[email protected]>--- Documentation/filesystems/sysfs.rst | 8 ++--- fs/sysfs/file.c | 55 +++++++++++++++++++++++++++++ include/linux/sysfs.h | 15 ++++++++ 3 files changed, 73 insertions(+), 5 deletions(-)Documentation/filesystems/sysfs.rst +3
-5@@ -241,12 +241,10 @@ Other notes: is 4096. - show() methods should return the number of bytes printed into the - buffer. This is the return value of scnprintf(). + buffer. -- show() must not use snprintf() when formatting the value to be - returned to user space. If you can guarantee that an overflow - will never happen you can use sprintf() otherwise you must use - scnprintf(). +- show() should only use sysfs_emit() or sysfs_emit_at() when formatting + the value to be returned to user space. - store() should return the number of bytes used from the buffer. If the entire buffer has been used, just return the count argument.fs/sysfs/file.c +45
-0@@ -15,6 +15,7 @@#include <linux/list.h> #include <linux/mutex.h> #include <linux/seq_file.h> +#include <linux/mm.h> #include "sysfs.h" @@ -707,3 +708,57 @@ int sysfs_change_owner(struct kobject *kobj, kuid_t kuid, kgid_t kgid)return 0 ; }EXPORT_SYMBOL_GPL (sysfs_change_owner); + +/** +* sysfs_emit - scnprintf equivalent, aware of PAGE_SIZE buffer. +* @buf: start of PAGE_SIZE buffer. +* @fmt: format +* @...: optional arguments to @format +* +* +* Returns number of characters written to @buf. +*/ +int sysfs_emit (char *buf,const char *fmt, ...) +{ +va_list args; +int len; + +if (WARN (!buf ||offset_in_page (buf), +"invalid sysfs_emit: buf:%p\n" , buf)) +return 0 ; + +va_start (args, fmt); + len =vscnprintf (buf,PAGE_SIZE , fmt, args); +va_end (args); + +return len; +} +EXPORT_SYMBOL_GPL (sysfs_emit); + +/** +* sysfs_emit_at - scnprintf equivalent, aware of PAGE_SIZE buffer. +* @buf: start of PAGE_SIZE buffer. +* @at: offset in @buf to start write in bytes +* @at must be >= 0 && < PAGE_SIZE +* @fmt: format +* @...: optional arguments to @fmt +* +* +* Returns number of characters written starting at &@buf[@at]. +*/ +int sysfs_emit_at (char *buf,int at,const char *fmt, ...) +{ +va_list args; +int len; + + if (WARN(!buf || offset_in_page(buf) || at < 0 || at >= PAGE_SIZE,
fs/sysfs/file.c int sysfs_change_owner(struct kobject *kobj, kuid_t kuid, kgid_t kgid) · line 764
Quoted 10 lines
fs/sysfs/file.c +10
-0+ "invalid sysfs_emit_at: buf:%p at:%d\n", buf, at)) +return 0 ; + +va_start (args, fmt); + len =vscnprintf (buf + at,PAGE_SIZE - at, fmt, args); +va_end (args); + +return len; +} +EXPORT_SYMBOL_GPL (sysfs_emit_at);
These feel sane, but I'm loath to have a ton of churn for no good reason.
If we make all sysfs show/store functions use these calls instead of sprintf(), it "feels" like that might address the objections that people have had in the past where they are nervous about "bare" sprintf() calls, right?
It also might make things easier to audit where we can see much easier where sysfs files are doing "foolish" things by calling sysfs_emit_at() a bunch of times they shouldn't be, and maybe automate the documentation of sysfs files in a better way.
So I guess I'm asking for another developer to at least agree that this feels like the right way forward here. I don't want to start down this path, only to roll them all back as it feels like pointless churn.
thanks,
greg k-h
Re: [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
Quoted 2 lines · Greg Kroah-Hartman
Kees, and Rafael, I don't know if you saw this proposal from Joe for sysfs files, questions below:
Quoted 3 lines
So I guess I'm asking for another developer to at least agree that this feels like the right way forward here. I don't want to start down this path, only to roll them all back as it feels like pointless churn.
https://lore.kernel.org/lkml/[email protected]/T/#mb40d265bc1dabb8bb64b0dfa29dd8eda44be056e
Greg Kroah-Hartman
maintainerRe: [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
Quoted 4 lines · Joe Perches
Quoted 2 lines · Greg Kroah-Hartman
Kees, and Rafael, I don't know if you saw this proposal from Joe for sysfs files, questions below:
Quoted 11 lines
Quoted 3 lines
So I guess I'm asking for another developer to at least agree that this feels like the right way forward here. I don't want to start down this path, only to roll them all back as it feels like pointless churn.
https://lore.kernel.org/lkml/[email protected]/T/#mb40d265bc1dabb8bb64b0dfa29dd8eda44be056e
All now queued up, thanks!
greg k-h
Re: [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
In reply to Greg Kroah-HartmanQuoted 2 lines · Greg Kroah-Hartman
Kees, and Rafael, I don't know if you saw this proposal from Joe for sysfs files, questions below:
I'm a fan. I think the use of sprintf() in sysfs might have been one of my earliest complaints about unsafe code patterns in the kernel. ;)
Quoted 8 lines
Quoted 8 lines
+/**
+ *sysfs_emit- scnprintf equivalent, aware ofPAGE_SIZEbuffer.
+ * @buf: start ofPAGE_SIZEbuffer.
+ * @fmt: format
+ * @...: optional arguments to @format
+ *
+ *
+ * Returns number of characters written to @buf.
Quoted 8 lines
fs/sysfs/file.c int sysfs_change_owner(struct kobject *kobj, kuid_t kuid, kgid_t kgid) · line 727
Quoted 8 lines
+ */
fs/sysfs/file.c +7
-0+int sysfs_emit (char *buf,const char *fmt, ...) +{ +va_list args; +int len; + +if (WARN (!buf ||offset_in_page (buf), +"invalid sysfs_emit: buf:%p\n" , buf))
I don't want the %p here, but otherwise, sure. I'd also make it a _ONCE variant:
if (WARN_ONCE(!buf || offset_in_page(buf), "invalid sysfs_emit: offset_in_page(buf):%zd\n", buf ? offset_in_page(buf) : 0))
Quoted 20 lines
Quoted 20 lines
fs/sysfs/file.c +20
-0+return 0 ; + +va_start (args, fmt); + len =vscnprintf (buf,PAGE_SIZE , fmt, args); +va_end (args); + +return len; +} +EXPORT_SYMBOL_GPL (sysfs_emit); + +/** +* sysfs_emit_at - scnprintf equivalent, aware of PAGE_SIZE buffer. +* @buf: start of PAGE_SIZE buffer. +* @at: offset in @buf to start write in bytes +* @at must be >= 0 && < PAGE_SIZE +* @fmt: format +* @...: optional arguments to @fmt +* +* +* Returns number of characters written starting at &@buf[@at].
Quoted 8 lines
fs/sysfs/file.c int sysfs_change_owner(struct kobject *kobj, kuid_t kuid, kgid_t kgid) · line 755
Quoted 8 lines
+ */
fs/sysfs/file.c +7
-0+int sysfs_emit_at (char *buf,int at,const char *fmt, ...) +{ +va_list args; +int len; + +if (WARN (!buf ||offset_in_page (buf) || at <0 || at >=PAGE_SIZE , +"invalid sysfs_emit_at: buf:%p at:%d\n" , buf, at))
Same:
if (WARN_ONCE(!buf || offset_in_page(buf) || at < 0 || at >= PAGE_SIZE, "invalid sysfs_emit_at: offset_in_page(buf):%zd at:%d\n", buf ? offset_in_page(buf) : 0, at))
Quoted 4 lines
Quoted 4 lines
fs/sysfs/file.c +4
-0+return 0 ; + +va_start (args, fmt); + len =vscnprintf (buf + at,PAGE_SIZE - at, fmt, args);
Quoted 8 lines
fs/sysfs/file.c int sysfs_change_owner(struct kobject *kobj, kuid_t kuid, kgid_t kgid) · line 764
Quoted 5 lines
fs/sysfs/file.c +5
-0+va_end (args); + +return len; +} +EXPORT_SYMBOL_GPL (sysfs_emit_at);These feel sane, but I'm loath to have a ton of churn for no good reason.
I think the churn is worth it if we remove "seemingly wrong" code patterns from the kernel. It's especially useful, IMO, for when there are future mutations/refactorings and we don't end up with a bare sprintf somewhere else.
Quoted 4 lines
If we make all sysfs show/store functions use these calls instead of
sprintf(), it "feels" like that might address the objections that people have had in the past where they are nervous about "bare"sprintf()calls, right?
I would think so. This is the kind of thing we did for %n in seq_file:
remove potential foot-gun API in favor of subsystem-specific safe API.
Quoted 4 lines
It also might make things easier to audit where we can see much easier where sysfs files are doing "foolish" things by calling
sysfs_emit_at()a bunch of times they shouldn't be, and maybe automate the documentation of sysfs files in a better way.
Indeed!
Quoted 3 lines
So I guess I'm asking for another developer to at least agree that this feels like the right way forward here. I don't want to start down this path, only to roll them all back as it feels like pointless churn.
With the changes above, I'd Ack it. :)
Signature · 1 line
Kees Cook
Re: [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
Quoted 6 lines · Kees Cook
Quoted 2 lines · Greg Kroah-Hartman
Kees, and Rafael, I don't know if you saw this proposal from Joe for sysfs files, questions below:
I'm a fan. I think the use of
sprintf()in sysfs might have been one of my earliest complaints about unsafe code patterns in the kernel. ;)
[]
Quoted 2 lines
Quoted 2 lines
fs/sysfs/file.c int sysfs_change_owner(struct kobject *kobj, kuid_t kuid, kgid_t kgid) · line 727
Quoted 2 lines
fs/sysfs/file.c +2
-0+if (WARN (!buf ||offset_in_page (buf), +"invalid sysfs_emit: buf:%p\n" , buf))
The dump_stack() is also going to emit pointers so I don't see how it does anything but help show where the buffer was. It is hashed...
Quoted 6 lines
I don't want the %p here, but otherwise, sure. I'd also make it a
_ONCEvariant:if (WARN_ONCE(!buf || offset_in_page(buf), "invalid sysfs_emit: offset_in_page(buf):%zd\n", buf ? offset_in_page(buf) : 0))
I don't think that helps as multiple defects can easily exist. Log spam in this case isn't horrible either.
Re: [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
Quoted 7 lines · Joe Perches
Quoted 6 lines · Kees Cook
Quoted 2 lines · Greg Kroah-Hartman
Kees, and Rafael, I don't know if you saw this proposal from Joe for sysfs files, questions below:
I'm a fan. I think the use of
sprintf()in sysfs might have been one of my earliest complaints about unsafe code patterns in the kernel. ;)
Quoted 7 lines
[]
Quoted 2 lines
Quoted 2 lines
fs/sysfs/file.c int sysfs_change_owner(struct kobject *kobj, kuid_t kuid, kgid_t kgid) · line 727
Quoted 2 lines
fs/sysfs/file.c +2
-0+if (WARN (!buf ||offset_in_page (buf), +"invalid sysfs_emit: buf:%p\n" , buf))The
dump_stack()is also going to emit pointers
so I don't see how it does anything but help
show where the buffer was. It is hashed...
dump_stack() is going to report symbols and register contents.
I was just pointing out that %p has no value here[1]. The interesting states are: "was it NULL?" "how offset was it?". Its actual content won't matter.
-Kees
[1] "New uses of %p should not be added to the kernel"
https://www.kernel.org/doc/html/latest/process/deprecated.html#p-format-specifier
Signature · 1 line
Kees Cook
Greg Kroah-Hartman
maintainerRe: [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
Quoted 16 lines · Kees Cook
Quoted 7 lines · Joe Perches
Quoted 6 lines · Kees Cook
Quoted 2 lines · Greg Kroah-Hartman
Kees, and Rafael, I don't know if you saw this proposal from Joe for sysfs files, questions below:
I'm a fan. I think the use of
sprintf()in sysfs might have been one of my earliest complaints about unsafe code patterns in the kernel. ;)Quoted 7 lines
[]
Quoted 2 lines
Quoted 2 lines
fs/sysfs/file.c int sysfs_change_owner(struct kobject *kobj, kuid_t kuid, kgid_t kgid) · line 727
Quoted 2 lines
fs/sysfs/file.c +2
-0+if (WARN (!buf ||offset_in_page (buf), +"invalid sysfs_emit: buf:%p\n" , buf))The
dump_stack()is also going to emit pointers
so I don't see how it does anything but help
show where the buffer was. It is hashed...
Quoted 5 lines
dump_stack()is going to report symbols and register contents.I was just pointing out that %p has no value here[1]. The interesting states are: "was it NULL?" "how offset was it?". Its actual content won't matter.
Ok, suggestions for a better error message are always welcome :)
thanks,
greg k-h
Re: [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
Quoted 17 lines · Greg Kroah-Hartman
Quoted 16 lines · Kees Cook
Quoted 14 lines · Joe Perches
Quoted 6 lines · Kees Cook
Quoted 2 lines · Greg Kroah-Hartman
Kees, and Rafael, I don't know if you saw this proposal from Joe for sysfs files, questions below:
I'm a fan. I think the use of
sprintf()in sysfs might have been one of my earliest complaints about unsafe code patterns in the kernel. ;)[]
Quoted 2 lines
Quoted 2 lines
fs/sysfs/file.c int sysfs_change_owner(struct kobject *kobj, kuid_t kuid, kgid_t kgid) · line 727
Quoted 2 lines
fs/sysfs/file.c +2
-0+if (WARN (!buf ||offset_in_page (buf), +"invalid sysfs_emit: buf:%p\n" , buf))The
dump_stack()is also going to emit pointers
so I don't see how it does anything but help
show where the buffer was. It is hashed...
Quoted 7 lines
Quoted 5 lines
dump_stack()is going to report symbols and register contents.I was just pointing out that %p has no value here[1]. The interesting states are: "was it NULL?" "how offset was it?". Its actual content won't matter.
Ok, suggestions for a better error message are always welcome :)
For sysfs_emit, the offset_in_buf test also effectively checks if buf not PAGE_SIZE so it helps identify if it is being called from a non _show function.
That's actually why %p can be somewhat valuable.
Re: [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
In reply to Greg Kroah-HartmanQuoted 17 lines · Greg Kroah-Hartman
Quoted 16 lines · Kees Cook
Quoted 14 lines · Joe Perches
Quoted 6 lines · Kees Cook
Quoted 2 lines · Greg Kroah-Hartman
Kees, and Rafael, I don't know if you saw this proposal from Joe for sysfs files, questions below:
I'm a fan. I think the use of
sprintf()in sysfs might have been one of my earliest complaints about unsafe code patterns in the kernel. ;)[]
Quoted 2 lines
Quoted 2 lines
fs/sysfs/file.c int sysfs_change_owner(struct kobject *kobj, kuid_t kuid, kgid_t kgid) · line 727
Quoted 2 lines
fs/sysfs/file.c +2
-0+if (WARN (!buf ||offset_in_page (buf), +"invalid sysfs_emit: buf:%p\n" , buf))The
dump_stack()is also going to emit pointers
so I don't see how it does anything but help
show where the buffer was. It is hashed...
Quoted 7 lines
Quoted 5 lines
dump_stack()is going to report symbols and register contents.I was just pointing out that %p has no value here[1]. The interesting states are: "was it NULL?" "how offset was it?". Its actual content won't matter.
Ok, suggestions for a better error message are always welcome :)
... I did. :P
https://lore.kernel.org/lkml/202009302108.18B05CA38@keescook/
But it doesn't need to hold up the series; %p is hashed, so I don't care. It's just that the mandate from Linus was to not add new %p uses. :P
Signature · 1 line
Kees Cook
Greg Kroah-Hartman
maintainerRe: [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
In reply to Kees CookQuoted 6 lines · Kees Cook
Quoted 2 lines · Greg Kroah-Hartman
Kees, and Rafael, I don't know if you saw this proposal from Joe for sysfs files, questions below:
I'm a fan. I think the use of
sprintf()in sysfs might have been one of my earliest complaints about unsafe code patterns in the kernel. ;)
Ok, great.
Quoted 20 lines
Quoted 8 lines
Quoted 8 lines
+/**
+ *sysfs_emit- scnprintf equivalent, aware ofPAGE_SIZEbuffer.
+ * @buf: start ofPAGE_SIZEbuffer.
+ * @fmt: format
+ * @...: optional arguments to @format
+ *
+ *
+ * Returns number of characters written to @buf.Quoted 8 lines
fs/sysfs/file.c int sysfs_change_owner(struct kobject *kobj, kuid_t kuid, kgid_t kgid) · line 727
Quoted 8 lines
+ */
fs/sysfs/file.c +7
-0+int sysfs_emit (char *buf,const char *fmt, ...) +{ +va_list args; +int len; + +if (WARN (!buf ||offset_in_page (buf), +"invalid sysfs_emit: buf:%p\n" , buf))I don't want the %p here, but otherwise, sure. I'd also make it a
_ONCEvariant:
Quoted 3 lines
if (WARN_ONCE(!buf || offset_in_page(buf), "invalid sysfs_emit: offset_in_page(buf):%zd\n", buf ? offset_in_page(buf) : 0))
As Joe points out, _ONCE doesn't work because this happens from all sysfs files, not just one.
thanks,
greg k-h
Re: [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
Quoted 30 lines · Greg Kroah-Hartman
Quoted 6 lines · Kees Cook
Quoted 2 lines · Greg Kroah-Hartman
Kees, and Rafael, I don't know if you saw this proposal from Joe for sysfs files, questions below:
I'm a fan. I think the use of
sprintf()in sysfs might have been one of my earliest complaints about unsafe code patterns in the kernel. ;)Ok, great.
Quoted 20 lines
Quoted 8 lines
Quoted 8 lines
+/**
+ *sysfs_emit- scnprintf equivalent, aware ofPAGE_SIZEbuffer.
+ * @buf: start ofPAGE_SIZEbuffer.
+ * @fmt: format
+ * @...: optional arguments to @format
+ *
+ *
+ * Returns number of characters written to @buf.Quoted 8 lines
fs/sysfs/file.c int sysfs_change_owner(struct kobject *kobj, kuid_t kuid, kgid_t kgid) · line 727
Quoted 8 lines
+ */
fs/sysfs/file.c +7
-0+int sysfs_emit (char *buf,const char *fmt, ...) +{ +va_list args; +int len; + +if (WARN (!buf ||offset_in_page (buf), +"invalid sysfs_emit: buf:%p\n" , buf))I don't want the %p here, but otherwise, sure. I'd also make it a
_ONCEvariant:
Quoted 6 lines
Quoted 3 lines
if (WARN_ONCE(!buf || offset_in_page(buf), "invalid sysfs_emit: offset_in_page(buf):%zd\n", buf ? offset_in_page(buf) : 0))As Joe points out,
_ONCEdoesn't work because this happens from all sysfs files, not just one.
Sure, it's just a question if you want log spamming vs how reachable you think something might be. I would expect this to be uncommon to encounter, but very repeatable for whatever system DOES hit it, so doing _ONCE means they see the report and don't get completely flooded with it.
I'm fine either way.
Signature · 1 line
Kees Cook
PATCH v3 2/8 drivers core: Use sysfs_emit and sysfs_emit_at for show(device *...) functions
In reply to Joe PerchesConvert the various sprintf fmaily calls in sysfs device show functions to sysfs_emit and sysfs_emit_at for PAGE_SIZE buffer safety.
Done with:
$ spatch -sp-file sysfs_emit_dev.cocci --in-place --max-width=80 .
And cocci script:
$ cat sysfs_emit_dev.cocci @@ identifier d_show; identifier dev, attr, buf; @@
ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
<...
return
- sprintf(buf,
+ sysfs_emit(buf,
...);
...>
}@@
identifier d_show;
identifier dev, attr, buf;
@@
ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
<...
return
- snprintf(buf, PAGE_SIZE,
+ sysfs_emit(buf,
...);
...>
}@@
identifier d_show;
identifier dev, attr, buf;
@@
ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
<...
return
- scnprintf(buf, PAGE_SIZE,
+ sysfs_emit(buf,
...);
...>
}@@
identifier d_show;
identifier dev, attr, buf;
expression chr;
@@
ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
<...
return
- strcpy(buf, chr);
+ sysfs_emit(buf, chr);
...>
}@@
identifier d_show;
identifier dev, attr, buf;
identifier len;
@@
ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
<...
len =
- sprintf(buf,
+ sysfs_emit(buf,
...);
...>
return len;
}@@
identifier d_show;
identifier dev, attr, buf;
identifier len;
@@
ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
<...
len =
- snprintf(buf, PAGE_SIZE,
+ sysfs_emit(buf,
...);
...>
return len;
}@@
identifier d_show;
identifier dev, attr, buf;
identifier len;
@@
ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
<...
len =
- scnprintf(buf, PAGE_SIZE,
+ sysfs_emit(buf,
...);
...>
return len;
}@@
identifier d_show;
identifier dev, attr, buf;
identifier len;
@@
ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
<...
- len += scnprintf(buf + len, PAGE_SIZE - len,
+ len += sysfs_emit_at(buf, len,
...);
...>
return len;
}@@
identifier d_show;
identifier dev, attr, buf;
expression chr;
@@
ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
...- strcpy(buf, chr);
- return strlen(buf);
+ return sysfs_emit(buf, chr);
}
--- drivers/base/arch_topology.c | 2 +- drivers/base/cacheinfo.c | 18 ++++----- drivers/base/core.c | 19 +++++----- drivers/base/cpu.c | 32 ++++++++-------- drivers/base/dd.c | 2 +- drivers/base/firmware_loader/fallback.c | 2 +- drivers/base/memory.c | 24 ++++++------ drivers/base/node.c | 28 +++++++------- drivers/base/platform.c | 4 +- drivers/base/power/sysfs.c | 50 ++++++++++++------------- drivers/base/power/wakeup_stats.c | 12 +++--- drivers/base/soc.c | 10 ++--- 12 files changed, 102 insertions(+), 101 deletions(-)
drivers/base/arch_topology.c +1 -1
@@ -71,7 +71,7 @@ static ssize_t cpu_capacity_show(struct device *dev, { struct cpu *cpu = container_of(dev, struct cpu, dev); -return sprintf (buf,"%lu\n" ,topology_get_cpu_scale (cpu->dev.id)); +return sysfs_emit (buf,"%lu\n" ,topology_get_cpu_scale (cpu->dev.id)); }static void update_topology_flags_workfn (struct work_struct *work);
drivers/base/cacheinfo.c +9 -9
@@ -377,7 +377,7 @@ static ssize_t size_show(struct device *dev, {struct cacheinfo *this_leaf =dev_get_drvdata (dev); -return sprintf (buf,"%uK\n" , this_leaf->size >>10 ); +return sysfs_emit (buf,"%uK\n" , this_leaf->size >>10 ); }static ssize_t shared_cpumap_show_func (struct device *dev,bool list,char *buf)
@@ -407,11 +407,11 @@ static ssize_t type_show(struct device *dev,switch (this_leaf->type) {case CACHE_TYPE_DATA : -return sprintf (buf,"Data\n" ); +return sysfs_emit (buf,"Data\n" );case CACHE_TYPE_INST : -return sprintf (buf,"Instruction\n" ); +return sysfs_emit (buf,"Instruction\n" );case CACHE_TYPE_UNIFIED : -return sprintf (buf,"Unified\n" ); +return sysfs_emit (buf,"Unified\n" );default :return -EINVAL ; }
@@ -425,11 +425,11 @@ static ssize_t allocation_policy_show(struct device *dev,int n =0 ;if ((ci_attr &CACHE_READ_ALLOCATE ) && (ci_attr &CACHE_WRITE_ALLOCATE )) - n =sprintf (buf,"ReadWriteAllocate\n" ); + n =sysfs_emit (buf,"ReadWriteAllocate\n" );else if (ci_attr &CACHE_READ_ALLOCATE ) - n =sprintf (buf,"ReadAllocate\n" ); + n =sysfs_emit (buf,"ReadAllocate\n" );else if (ci_attr &CACHE_WRITE_ALLOCATE ) - n =sprintf (buf,"WriteAllocate\n" ); + n =sysfs_emit (buf,"WriteAllocate\n" );return n; }
@@ -441,9 +441,9 @@ static ssize_t write_policy_show(struct device *dev,int n =0 ;if (ci_attr &CACHE_WRITE_THROUGH ) - n =sprintf (buf,"WriteThrough\n" ); + n =sysfs_emit (buf,"WriteThrough\n" );else if (ci_attr &CACHE_WRITE_BACK ) - n =sprintf (buf,"WriteBack\n" ); + n =sysfs_emit (buf,"WriteBack\n" );return n; }
drivers/base/core.c +10 -9
@@ -259,7 +259,7 @@ static ssize_t status_show(struct device *dev,default : status ="unknown" ;break ; } -return sprintf (buf,"%s\n" , status); +return sysfs_emit (buf,"%s\n" , status); }static DEVICE_ATTR_RO (status);
@@ -276,7 +276,7 @@ static ssize_t auto_remove_on_show(struct device *dev,else str ="never" ; -return sprintf (buf,"%s\n" , str); +return sysfs_emit (buf,"%s\n" , str); }static DEVICE_ATTR_RO (auto_remove_on);
@@ -285,7 +285,7 @@ static ssize_t runtime_pm_show(struct device *dev, {struct device_link *link =to_devlink (dev); -return sprintf (buf,"%d\n" , !!(link->flags &DL_FLAG_PM_RUNTIME )); +return sysfs_emit (buf,"%d\n" , !!(link->flags &DL_FLAG_PM_RUNTIME )); }static DEVICE_ATTR_RO (runtime_pm);
@@ -294,7 +294,8 @@ static ssize_t sync_state_only_show(struct device *dev, {struct device_link *link =to_devlink (dev); -return sprintf (buf,"%d\n" , !!(link->flags &DL_FLAG_SYNC_STATE_ONLY )); +return sysfs_emit (buf,"%d\n" , + !!(link->flags &DL_FLAG_SYNC_STATE_ONLY )); }static DEVICE_ATTR_RO (sync_state_only);
@@ -1059,7 +1060,7 @@ static ssize_t waiting_for_supplier_show(struct device *dev, && dev->links.need_for_probe;mutex_unlock (&wfs_lock);device_unlock (dev); -return sprintf (buf,"%u\n" , val); +return sysfs_emit (buf,"%u\n" , val); }static DEVICE_ATTR_RO (waiting_for_supplier);
@@ -1709,7 +1710,7 @@ ssize_t device_show_ulong(struct device *dev, char *buf) {struct dev_ext_attribute *ea =to_ext_attr (attr); -return snprintf (buf,PAGE_SIZE ,"%lx\n" , *(unsigned long *)(ea->var)); +return sysfs_emit (buf,"%lx\n" , *(unsigned long *)(ea->var)); }EXPORT_SYMBOL_GPL (device_show_ulong);
@@ -1739,7 +1740,7 @@ ssize_t device_show_int(struct device *dev, {struct dev_ext_attribute *ea =to_ext_attr (attr); -return snprintf (buf,PAGE_SIZE ,"%d\n" , *(int *)(ea->var)); +return sysfs_emit (buf,"%d\n" , *(int *)(ea->var)); }EXPORT_SYMBOL_GPL (device_show_int);
@@ -1760,7 +1761,7 @@ ssize_t device_show_bool(struct device *dev, struct device_attribute *attr, {struct dev_ext_attribute *ea =to_ext_attr (attr); -return snprintf (buf,PAGE_SIZE ,"%d\n" , *(bool *)(ea->var)); +return sysfs_emit (buf,"%d\n" , *(bool *)(ea->var)); }EXPORT_SYMBOL_GPL (device_show_bool);
@@ -1992,7 +1993,7 @@ static ssize_t online_show(struct device *dev, struct device_attribute *attr,device_lock (dev); val = !dev->offline;device_unlock (dev); -return sprintf (buf,"%u\n" , val); +return sysfs_emit (buf,"%u\n" , val); } static ssize_t online_store(struct device *dev, struct device_attribute *attr,
drivers/base/cpu.c +16 -16
@@ -156,7 +156,7 @@ static ssize_t show_crash_notes(struct device *dev, struct device_attribute *att* operation should be safe. No locking required. */ addr =per_cpu_ptr_to_phys (per_cpu_ptr (crash_notes, cpunum)); - rc =sprintf (buf,"%Lx\n" , addr); + rc =sysfs_emit (buf,"%Lx\n" , addr);return rc; } static DEVICE_ATTR(crash_notes, 0400, show_crash_notes, NULL);
@@ -167,7 +167,7 @@ static ssize_t show_crash_notes_size(struct device *dev, {ssize_t rc; - rc =sprintf (buf,"%zu\n" ,sizeof (note_buf_t)); + rc =sysfs_emit (buf,"%zu\n" ,sizeof (note_buf_t));return rc; } static DEVICE_ATTR(crash_notes_size, 0400, show_crash_notes_size, NULL);
@@ -231,7 +231,7 @@ static struct cpu_attr cpu_attrs[] = {static ssize_t print_cpus_kernel_max (struct device *dev,struct device_attribute *attr,char *buf) { -return sprintf (buf,"%d\n" ,NR_CPUS -1 ); +return sysfs_emit (buf,"%d\n" ,NR_CPUS -1 ); } static DEVICE_ATTR(kernel_max, 0444, print_cpus_kernel_max, NULL);
@@ -279,7 +279,7 @@ static ssize_t print_cpus_isolated(struct device *dev,cpumask_andnot (isolated, cpu_possible_mask,housekeeping_cpumask (HK_FLAG_DOMAIN )); - n =sprintf (buf,"%*pbl\n" ,cpumask_pr_args (isolated)); + n =sysfs_emit (buf,"%*pbl\n" ,cpumask_pr_args (isolated));free_cpumask_var (isolated);
@@ -291,7 +291,7 @@ static DEVICE_ATTR(isolated, 0444, print_cpus_isolated, NULL);static ssize_t print_cpus_nohz_full (struct device *dev,struct device_attribute *attr,char *buf) { -return sprintf (buf,"%*pbl\n" ,cpumask_pr_args (tick_nohz_full_mask)); +return sysfs_emit (buf,"%*pbl\n" ,cpumask_pr_args (tick_nohz_full_mask)); } static DEVICE_ATTR(nohz_full, 0444, print_cpus_nohz_full, NULL);#endif
@@ -323,8 +323,8 @@ static ssize_t print_cpu_modalias(struct device *dev,ssize_t n;u32 i; - n =sprintf (buf,"cpu:type:" CPU_FEATURE_TYPEFMT ":feature:" , -CPU_FEATURE_TYPEVAL ); + n =sysfs_emit (buf,"cpu:type:" CPU_FEATURE_TYPEFMT ":feature:" , +CPU_FEATURE_TYPEVAL );for (i =0 ; i <MAX_CPU_FEATURES ; i++)if (cpu_have_feature (i)) {
@@ -516,56 +516,56 @@ static void __init cpu_dev_register_generic(void)ssize_t __weak cpu_show_meltdown (struct device *dev,struct device_attribute *attr,char *buf) { -return sprintf (buf,"Not affected\n" ); +return sysfs_emit (buf,"Not affected\n" ); }ssize_t __weak cpu_show_spectre_v1 (struct device *dev,struct device_attribute *attr,char *buf) { -return sprintf (buf,"Not affected\n" ); +return sysfs_emit (buf,"Not affected\n" ); }ssize_t __weak cpu_show_spectre_v2 (struct device *dev,struct device_attribute *attr,char *buf) { -return sprintf (buf,"Not affected\n" ); +return sysfs_emit (buf,"Not affected\n" ); }ssize_t __weak cpu_show_spec_store_bypass (struct device *dev,struct device_attribute *attr,char *buf) { -return sprintf (buf,"Not affected\n" ); +return sysfs_emit (buf,"Not affected\n" ); }ssize_t __weak cpu_show_l1tf (struct device *dev,struct device_attribute *attr,char *buf) { -return sprintf (buf,"Not affected\n" ); +return sysfs_emit (buf,"Not affected\n" ); }ssize_t __weak cpu_show_mds (struct device *dev,struct device_attribute *attr,char *buf) { -return sprintf (buf,"Not affected\n" ); +return sysfs_emit (buf,"Not affected\n" ); }ssize_t __weak cpu_show_tsx_async_abort (struct device *dev,struct device_attribute *attr,char *buf) { -return sprintf (buf,"Not affected\n" ); +return sysfs_emit (buf,"Not affected\n" ); }ssize_t __weak cpu_show_itlb_multihit (struct device *dev,struct device_attribute *attr,char *buf) { -return sprintf (buf,"Not affected\n" ); +return sysfs_emit (buf,"Not affected\n" ); }ssize_t __weak cpu_show_srbds (struct device *dev,struct device_attribute *attr,char *buf) { -return sprintf (buf,"Not affected\n" ); +return sysfs_emit (buf,"Not affected\n" ); } static DEVICE_ATTR(meltdown, 0444, cpu_show_meltdown, NULL);
drivers/base/dd.c +1 -1
@@ -486,7 +486,7 @@ static ssize_t state_synced_show(struct device *dev,device_lock (dev); val = dev->state_synced;device_unlock (dev); -return sprintf (buf,"%u\n" , val); +return sysfs_emit (buf,"%u\n" , val); }static DEVICE_ATTR_RO (state_synced);
drivers/base/firmware_loader/fallback.c +1 -1
@@ -219,7 +219,7 @@ static ssize_t firmware_loading_show(struct device *dev, loading =fw_sysfs_loading (fw_sysfs->fw_priv);mutex_unlock (&fw_lock); -return sprintf (buf,"%d\n" , loading); +return sysfs_emit (buf,"%d\n" , loading); }/**
drivers/base/memory.c +12 -12
@@ -119,7 +119,7 @@ static ssize_t phys_index_show(struct device *dev,unsigned long phys_index; phys_index = mem->start_section_nr / sections_per_block; -return sprintf (buf,"%08lx\n" , phys_index); +return sysfs_emit (buf,"%08lx\n" , phys_index); }/*
@@ -129,7 +129,7 @@ static ssize_t phys_index_show(struct device *dev,static ssize_t removable_show (struct device *dev,struct device_attribute *attr,char *buf) { -return sprintf (buf,"%d\n" , (int )IS_ENABLED (CONFIG_MEMORY_HOTREMOVE )); +return sysfs_emit (buf,"%d\n" , (int )IS_ENABLED (CONFIG_MEMORY_HOTREMOVE )); }/*
@@ -147,17 +147,17 @@ static ssize_t state_show(struct device *dev, struct device_attribute *attr,*/ switch (mem->state) {case MEM_ONLINE : - len =sprintf (buf,"online\n" ); + len =sysfs_emit (buf,"online\n" );break ;case MEM_OFFLINE : - len =sprintf (buf,"offline\n" ); + len =sysfs_emit (buf,"offline\n" );break ;case MEM_GOING_OFFLINE : - len =sprintf (buf,"going-offline\n" ); + len =sysfs_emit (buf,"going-offline\n" );break ;default : - len =sprintf (buf,"ERROR-UNKNOWN-%ld\n" , - mem->state); + len =sysfs_emit (buf,"ERROR-UNKNOWN-%ld\n" , + mem->state);WARN_ON (1 );break ; }
@@ -303,7 +303,7 @@ static ssize_t phys_device_show(struct device *dev, struct device_attribute *attr, char *buf) { struct memory_block *mem = to_memory_block(dev); -return sprintf (buf,"%d\n" , mem->phys_device); +return sysfs_emit (buf,"%d\n" , mem->phys_device); }#ifdef CONFIG_MEMORY_HOTREMOVE
@@ -341,7 +341,7 @@ static ssize_t valid_zones_show(struct device *dev, default_zone =test_pages_in_a_zone (start_pfn, start_pfn + nr_pages);if (!default_zone) -return sprintf (buf,"none\n" ); +return sysfs_emit (buf,"none\n" );strcat (buf, default_zone->name); goto out; }
@@ -374,7 +374,7 @@ static DEVICE_ATTR_RO(removable);static ssize_t block_size_bytes_show (struct device *dev,struct device_attribute *attr,char *buf) { -return sprintf (buf,"%lx\n" ,memory_block_size_bytes ()); +return sysfs_emit (buf,"%lx\n" ,memory_block_size_bytes ()); }static DEVICE_ATTR_RO (block_size_bytes);
@@ -386,8 +386,8 @@ static DEVICE_ATTR_RO(block_size_bytes);static ssize_t auto_online_blocks_show (struct device *dev,struct device_attribute *attr,char *buf) { -return sprintf (buf,"%s\n" , - online_type_to_str[memhp_default_online_type]); +return sysfs_emit (buf,"%s\n" , + online_type_to_str[memhp_default_online_type]); } static ssize_t auto_online_blocks_store(struct device *dev,
drivers/base/node.c +14 -14
@@ -370,7 +370,7 @@ static ssize_t node_read_meminfo(struct device *dev,si_meminfo_node (&i, nid); sreclaimable =node_page_state_pages (pgdat,NR_SLAB_RECLAIMABLE_B ); sunreclaimable =node_page_state_pages (pgdat,NR_SLAB_UNRECLAIMABLE_B ); - n =sprintf (buf, + n =sysfs_emit (buf,"Node %d MemTotal: %8lu kB\n" "Node %d MemFree: %8lu kB\n" "Node %d MemUsed: %8lu kB\n"
@@ -477,19 +477,19 @@ static DEVICE_ATTR(meminfo, S_IRUGO, node_read_meminfo, NULL);static ssize_t node_read_numastat (struct device *dev,struct device_attribute *attr,char *buf) { -return sprintf (buf, -"numa_hit %lu\n" -"numa_miss %lu\n" -"numa_foreign %lu\n" -"interleave_hit %lu\n" -"local_node %lu\n" -"other_node %lu\n" , -sum_zone_numa_state (dev->id,NUMA_HIT ), -sum_zone_numa_state (dev->id,NUMA_MISS ), -sum_zone_numa_state (dev->id,NUMA_FOREIGN ), -sum_zone_numa_state (dev->id,NUMA_INTERLEAVE_HIT ), -sum_zone_numa_state (dev->id,NUMA_LOCAL ), -sum_zone_numa_state (dev->id,NUMA_OTHER )); +return sysfs_emit (buf, +"numa_hit %lu\n" +"numa_miss %lu\n" +"numa_foreign %lu\n" +"interleave_hit %lu\n" +"local_node %lu\n" +"other_node %lu\n" , +sum_zone_numa_state (dev->id,NUMA_HIT ), +sum_zone_numa_state (dev->id,NUMA_MISS ), +sum_zone_numa_state (dev->id,NUMA_FOREIGN ), +sum_zone_numa_state (dev->id,NUMA_INTERLEAVE_HIT ), +sum_zone_numa_state (dev->id,NUMA_LOCAL ), +sum_zone_numa_state (dev->id,NUMA_OTHER )); } static DEVICE_ATTR(numastat, S_IRUGO, node_read_numastat, NULL);
drivers/base/platform.c +2 -2
@@ -1084,7 +1084,7 @@ static ssize_t driver_override_show(struct device *dev,ssize_t len;device_lock (dev); - len =sprintf (buf,"%s\n" , pdev->driver_override); + len =sysfs_emit (buf,"%s\n" , pdev->driver_override);device_unlock (dev);return len; }
@@ -1093,7 +1093,7 @@ static DEVICE_ATTR_RW(driver_override);static ssize_t numa_node_show (struct device *dev,struct device_attribute *attr,char *buf) { -return sprintf (buf,"%d\n" ,dev_to_node (dev)); +return sysfs_emit (buf,"%d\n" ,dev_to_node (dev)); }static DEVICE_ATTR_RO (numa_node);
drivers/base/power/sysfs.c +25 -25
135 lines not on this page · raw message
drivers/base/power/wakeup_stats.c +6 -6
@@ -42,7 +42,7 @@ static ssize_t active_time_ms_show(struct device *dev,ktime_t active_time = ws->active ?ktime_sub (ktime_get (), ws->last_time) :0 ; -return sprintf (buf,"%lld\n" ,ktime_to_ms (active_time)); +return sysfs_emit (buf,"%lld\n" ,ktime_to_ms (active_time)); }static DEVICE_ATTR_RO (active_time_ms);
@@ -57,7 +57,7 @@ static ssize_t total_time_ms_show(struct device *dev, active_time =ktime_sub (ktime_get (), ws->last_time); total_time =ktime_add (total_time, active_time); } -return sprintf (buf,"%lld\n" ,ktime_to_ms (total_time)); +return sysfs_emit (buf,"%lld\n" ,ktime_to_ms (total_time)); }static DEVICE_ATTR_RO (total_time_ms);
@@ -73,7 +73,7 @@ static ssize_t max_time_ms_show(struct device *dev,if (active_time > max_time) max_time = active_time; } -return sprintf (buf,"%lld\n" ,ktime_to_ms (max_time)); +return sysfs_emit (buf,"%lld\n" ,ktime_to_ms (max_time)); }static DEVICE_ATTR_RO (max_time_ms);
@@ -82,7 +82,7 @@ static ssize_t last_change_ms_show(struct device *dev, {struct wakeup_source *ws =dev_get_drvdata (dev); -return sprintf (buf,"%lld\n" ,ktime_to_ms (ws->last_time)); +return sysfs_emit (buf,"%lld\n" ,ktime_to_ms (ws->last_time)); }static DEVICE_ATTR_RO (last_change_ms);
@@ -91,7 +91,7 @@ static ssize_t name_show(struct device *dev, struct device_attribute *attr, {struct wakeup_source *ws =dev_get_drvdata (dev); -return sprintf (buf,"%s\n" , ws->name); +return sysfs_emit (buf,"%s\n" , ws->name); }static DEVICE_ATTR_RO (name);
@@ -106,7 +106,7 @@ static ssize_t prevent_suspend_time_ms_show(struct device *dev, prevent_sleep_time =ktime_add (prevent_sleep_time,ktime_sub (ktime_get (), ws->start_prevent_time)); } -return sprintf (buf,"%lld\n" ,ktime_to_ms (prevent_sleep_time)); +return sysfs_emit (buf,"%lld\n" ,ktime_to_ms (prevent_sleep_time)); }static DEVICE_ATTR_RO (prevent_suspend_time_ms);
drivers/base/soc.c +5 -5
@@ -76,15 +76,15 @@ static ssize_t soc_info_get(struct device *dev, struct soc_device *soc_dev = container_of(dev, struct soc_device, dev);if (attr == &dev_attr_machine) -return sprintf (buf,"%s\n" , soc_dev->attr->machine); +return sysfs_emit (buf,"%s\n" , soc_dev->attr->machine);if (attr == &dev_attr_family) -return sprintf (buf,"%s\n" , soc_dev->attr->family); +return sysfs_emit (buf,"%s\n" , soc_dev->attr->family);if (attr == &dev_attr_revision) -return sprintf (buf,"%s\n" , soc_dev->attr->revision); +return sysfs_emit (buf,"%s\n" , soc_dev->attr->revision);if (attr == &dev_attr_serial_number) -return sprintf (buf,"%s\n" , soc_dev->attr->serial_number); +return sysfs_emit (buf,"%s\n" , soc_dev->attr->serial_number);if (attr == &dev_attr_soc_id) -return sprintf (buf,"%s\n" , soc_dev->attr->soc_id); +return sysfs_emit (buf,"%s\n" , soc_dev->attr->soc_id);return -EINVAL ;
Signature · 1 line
2.26.0
PATCH v3 3/8 drivers core: Remove strcat uses around sysfs_emit and neaten
In reply to Joe Perchesstrcat is no longer necessary for sysfs_emit and sysfs_emit_at uses.
Convert the strcat uses to sysfs_emit calls and neaten other block uses of direct returns to use an intermediate const char *.
--- drivers/base/cacheinfo.c | 25 +++++++++++++------- drivers/base/core.c | 10 ++++---- drivers/base/memory.c | 47 ++++++++++++++++++-------------------- drivers/base/power/sysfs.c | 23 +++++++++++-------- 4 files changed, 58 insertions(+), 47 deletions(-)
drivers/base/cacheinfo.c +17 -8
@@ -404,17 +404,23 @@ static ssize_t type_show(struct device *dev, struct device_attribute *attr, char *buf) { struct cacheinfo *this_leaf = dev_get_drvdata(dev); +const char *output;switch (this_leaf->type) {case CACHE_TYPE_DATA : -return sysfs_emit (buf,"Data\n" ); + output ="Data" ; +break ;case CACHE_TYPE_INST : -return sysfs_emit (buf,"Instruction\n" ); + output ="Instruction" ; +break ;case CACHE_TYPE_UNIFIED : -return sysfs_emit (buf,"Unified\n" ); + output ="Unified" ; +break ;default :return -EINVAL ; } + +return sysfs_emit (buf,"%s\n" , output); } static ssize_t allocation_policy_show(struct device *dev,
@@ -422,15 +428,18 @@ static ssize_t allocation_policy_show(struct device *dev, {struct cacheinfo *this_leaf =dev_get_drvdata (dev);unsigned int ci_attr = this_leaf->attributes; -int n =0 ; +const char *output;if ((ci_attr &CACHE_READ_ALLOCATE ) && (ci_attr &CACHE_WRITE_ALLOCATE )) - n =sysfs_emit (buf,"ReadWriteAllocate\n" ); + output ="ReadWriteAllocate" ;else if (ci_attr &CACHE_READ_ALLOCATE ) - n =sysfs_emit (buf,"ReadAllocate\n" ); + output ="ReadAllocate" ;else if (ci_attr &CACHE_WRITE_ALLOCATE ) - n =sysfs_emit (buf,"WriteAllocate\n" ); -return n; + output ="WriteAllocate" ; +else +return 0 ; + +return sysfs_emit (buf,"%s\n" , output); } static ssize_t write_policy_show(struct device *dev,
drivers/base/core.c +5 -5
@@ -267,16 +267,16 @@ static ssize_t auto_remove_on_show(struct device *dev, struct device_attribute *attr, char *buf) { struct device_link *link = to_devlink(dev); -char *str; +const char *output;if (link->flags &DL_FLAG_AUTOREMOVE_SUPPLIER ) - str ="supplier unbind" ; + output ="supplier unbind" ;else if (link->flags &DL_FLAG_AUTOREMOVE_CONSUMER ) - str ="consumer unbind" ; + output ="consumer unbind" ;else - str ="never" ; + output ="never" ; -return sysfs_emit (buf,"%s\n" , str); +return sysfs_emit (buf,"%s\n" , output); }static DEVICE_ATTR_RO (auto_remove_on);
drivers/base/memory.c +22 -25
98 lines not on this page · raw message
drivers/base/power/sysfs.c +14 -9
@@ -255,9 +255,9 @@ static ssize_t pm_qos_latency_tolerance_us_show(struct device *dev,s32 value =dev_pm_qos_get_user_latency_tolerance (dev);if (value <0 ) -return sysfs_emit (buf,"auto\n" ); +return sysfs_emit (buf,"%s\n" ,"auto" );if (value ==PM_QOS_LATENCY_ANY ) -return sysfs_emit (buf,"any\n" ); +return sysfs_emit (buf,"%s\n" ,"any" );return sysfs_emit (buf,"%d\n" , value); }
@@ -538,13 +538,18 @@ static DEVICE_ATTR_RO(runtime_active_kids);static ssize_t runtime_enabled_show (struct device *dev,struct device_attribute *attr,char *buf) { -if (dev->power.disable_depth && (dev->power.runtime_auto == false)) -return sysfs_emit (buf,"disabled & forbidden\n" ); -if (dev->power.disable_depth) -return sysfs_emit (buf,"disabled\n" ); -if (dev->power.runtime_auto == false) -return sysfs_emit (buf,"forbidden\n" ); -return sysfs_emit (buf,"enabled\n" ); +const char *output; + +if (dev->power.disable_depth && !dev->power.runtime_auto) + output ="disabled & forbidden" ; +else if (dev->power.disable_depth) + output ="disabled" ; +else if (!dev->power.runtime_auto) + output ="forbidden" ; +else + output ="enabled" ; + +return sysfs_emit (buf,"%s\n" , output); }static DEVICE_ATTR_RO (runtime_enabled);
Signature · 1 line
2.26.0
PATCH v3 4/8 drivers core: Reindent a couple uses around sysfs_emit
In reply to Joe PerchesJust a couple of whitespace realignment to open parenthesis for multi-line statements.
--- drivers/base/node.c | 4 ++-- drivers/base/power/sysfs.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
drivers/base/node.c +2 -2
@@ -386,9 +386,9 @@ static ssize_t node_read_meminfo(struct device *dev, nid,K (i.freeram), nid,K (i.totalram - i.freeram), nid,K (node_page_state (pgdat,NR_ACTIVE_ANON ) + -node_page_state (pgdat,NR_ACTIVE_FILE )), +node_page_state (pgdat,NR_ACTIVE_FILE )), nid,K (node_page_state (pgdat,NR_INACTIVE_ANON ) + -node_page_state (pgdat,NR_INACTIVE_FILE )), +node_page_state (pgdat,NR_INACTIVE_FILE )), nid,K (node_page_state (pgdat,NR_ACTIVE_ANON )), nid,K (node_page_state (pgdat,NR_INACTIVE_ANON )), nid, K(node_page_state(pgdat, NR_ACTIVE_FILE)),
drivers/base/power/sysfs.c +1 -1
@@ -102,7 +102,7 @@ static ssize_t control_show(struct device *dev, struct device_attribute *attr, char *buf) {return sysfs_emit (buf,"%s\n" , - dev->power.runtime_auto ? ctrl_auto : ctrl_on); + dev->power.runtime_auto ? ctrl_auto : ctrl_on); } static ssize_t control_store(struct device * dev, struct device_attribute *attr,
Signature · 1 line
2.26.0
PATCH v3 5/8 drivers core: Miscellaneous changes for sysfs_emit
In reply to Joe PerchesChange additional instances that could use sysfs_emit and sysfs_emit_at that the coccinelle script could not convert.
o macros creating show functions with ## concatenation
o unbound sprintf uses with buf+len for start of output to sysfs_emit_at
o returns with ?: tests and sprintf to sysfs_emit
o sysfs output with struct class * not struct device * arguments
Miscellanea:
o remove unnecessary initializations around these changes
o consistently use int len for return length of show functions
o use octal permissions and not S_<FOO>
o rename a few show function names so DEVICE_ATTR_<FOO> can be used
o use DEVICE_ATTR_ADMIN_RO where appropriate
o consistently use const char *output for strings
o checkpatch/style neatening
--- drivers/base/bus.c | 2 +- drivers/base/cacheinfo.c | 2 +- drivers/base/class.c | 2 +- drivers/base/core.c | 34 +-- drivers/base/cpu.c | 62 +++--- drivers/base/dd.c | 1 + drivers/base/devcoredump.c | 2 +- drivers/base/firmware_loader/fallback.c | 2 +- drivers/base/memory.c | 5 +- drivers/base/node.c | 268 ++++++++++++------------ drivers/base/platform.c | 13 +- drivers/base/power/sysfs.c | 103 +++++---- drivers/base/power/wakeup_stats.c | 5 +- drivers/base/soc.c | 64 +++--- drivers/base/topology.c | 10 +- 15 files changed, 308 insertions(+), 267 deletions(-)
drivers/base/bus.c +1 -1
@@ -229,7 +229,7 @@ static DRIVER_ATTR_IGNORE_LOCKDEP(bind, S_IWUSR, NULL, bind_store);static ssize_t drivers_autoprobe_show (struct bus_type *bus,char *buf) { -return sprintf (buf,"%d\n" , bus->p->drivers_autoprobe); +return sysfs_emit (buf,"%d\n" , bus->p->drivers_autoprobe); } static ssize_t drivers_autoprobe_store(struct bus_type *bus,
drivers/base/cacheinfo.c +1 -1
@@ -362,7 +362,7 @@ static ssize_t file_name##_show(struct device *dev, \ struct device_attribute *attr, char *buf) \ { \ struct cacheinfo *this_leaf = dev_get_drvdata(dev); \ -return sprintf (buf,"%u\n" , this_leaf->object); \ +return sysfs_emit (buf,"%u\n" , this_leaf->object); \ }show_one (id, id);
drivers/base/class.c +1 -1
@@ -478,7 +478,7 @@ ssize_t show_class_attr_string(struct class *class,struct class_attribute_string *cs; cs = container_of(attr, struct class_attribute_string, attr); -return snprintf (buf,PAGE_SIZE ,"%s\n" , cs->str); +return sysfs_emit (buf,"%s\n" , cs->str); }EXPORT_SYMBOL_GPL (show_class_attr_string);
drivers/base/core.c +21 -13
@@ -239,27 +239,35 @@ void device_pm_move_to_tail(struct device *dev)#define to_devlink (dev) container_of((dev), struct device_link, link_dev)static ssize_t status_show (struct device *dev, -struct device_attribute *attr,char *buf) +struct device_attribute *attr,char *buf) { -char *status; +const char *output;switch (to_devlink (dev)->status) {case DL_STATE_NONE : - status ="not tracked" ;break ; + output ="not tracked" ; +break ;case DL_STATE_DORMANT : - status ="dormant" ;break ; + output ="dormant" ; +break ;case DL_STATE_AVAILABLE : - status ="available" ;break ; + output ="available" ; +break ;case DL_STATE_CONSUMER_PROBE : - status ="consumer probing" ;break ; + output ="consumer probing" ; +break ;case DL_STATE_ACTIVE : - status ="active" ;break ; + output ="active" ; +break ;case DL_STATE_SUPPLIER_UNBIND : - status ="supplier unbinding" ;break ; + output ="supplier unbinding" ; +break ;default : - status ="unknown" ;break ; + output ="unknown" ; +break ; } -return sysfs_emit (buf,"%s\n" , status); + +return sysfs_emit (buf,"%s\n" , output); }static DEVICE_ATTR_RO (status);
@@ -1933,7 +1941,7 @@ static ssize_t uevent_show(struct device *dev, struct device_attribute *attr,struct kset *kset;struct kobj_uevent_env *env =NULL ;int i; -size_t count =0 ; +int len =0 ;int retval;/* search the kset, the device belongs to */
@@ -1963,10 +1971,10 @@ static ssize_t uevent_show(struct device *dev, struct device_attribute *attr,/* copy keys to file */ for (i =0 ; i < env->envp_idx; i++) - count +=sprintf (&buf[count],"%s\n" , env->envp[i]); + len +=sysfs_emit_at (buf, len,"%s\n" , env->envp[i]); out:kfree (env); -return count; +return len; } static ssize_t uevent_store(struct device *dev, struct device_attribute *attr,
drivers/base/cpu.c +30 -32
145 lines not on this page · raw message
drivers/base/dd.c +1 -0
@@ -486,6 +486,7 @@ static ssize_t state_synced_show(struct device *dev,device_lock (dev); val = dev->state_synced;device_unlock (dev); +return sysfs_emit (buf,"%u\n" , val); }static DEVICE_ATTR_RO (state_synced);
drivers/base/devcoredump.c +1 -1
@@ -123,7 +123,7 @@ static int devcd_free(struct device *dev, void *data)static ssize_t disabled_show (struct class *class,struct class_attribute *attr,char *buf) { -return sprintf (buf,"%d\n" , devcd_disabled); +return sysfs_emit (buf,"%d\n" , devcd_disabled); } static ssize_t disabled_store(struct class *class, struct class_attribute *attr,
drivers/base/firmware_loader/fallback.c +1 -1
@@ -124,7 +124,7 @@ void kill_pending_fw_fallback_reqs(bool only_kill_custom)static ssize_t timeout_show (struct class *class,struct class_attribute *attr,char *buf) { -return sprintf (buf,"%d\n" ,__firmware_loading_timeout ()); +return sysfs_emit (buf,"%d\n" ,__firmware_loading_timeout ()); }/**
drivers/base/memory.c +4 -1
@@ -119,6 +119,7 @@ static ssize_t phys_index_show(struct device *dev,unsigned long phys_index; phys_index = mem->start_section_nr / sections_per_block; +return sysfs_emit (buf,"%08lx\n" , phys_index); }
@@ -301,6 +302,7 @@ static ssize_t phys_device_show(struct device *dev, struct device_attribute *attr, char *buf) { struct memory_block *mem = to_memory_block(dev); +return sysfs_emit (buf,"%d\n" , mem->phys_device); }
@@ -314,6 +316,7 @@ static int print_allowed_zone(char *buf, int len, int nid, zone =zone_for_pfn_range (online_type, nid, start_pfn, nr_pages);if (zone == default_zone)return 0 ; +return sysfs_emit_at (buf, len," %s" , zone->name); }
@@ -354,7 +357,7 @@ static ssize_t valid_zones_show(struct device *dev, len +=print_allowed_zone (buf, len, nid, start_pfn, nr_pages,MMOP_ONLINE_MOVABLE , default_zone); out: - len +=sysfs_emit_at (buf, len,"%s" ,"\n" ); + len +=sysfs_emit_at (buf, len,"\n" );return len; }static DEVICE_ATTR_RO (valid_zones);
drivers/base/node.c +135 -133
365 lines not on this page · raw message
drivers/base/platform.c +6 -7
@@ -1023,10 +1023,10 @@ EXPORT_SYMBOL_GPL(platform_unregister_drivers);* (b) sysfs attribute lets new-style coldplug recover from hotplug events * mishandled before system is fully running: "modprobe $(cat modalias)" */ -static ssize_t modalias_show (struct device *dev,struct device_attribute *a, -char *buf) +static ssize_t modalias_show (struct device *dev, +struct device_attribute *attr,char *buf) { -struct platform_device *pdev =to_platform_device (dev); +struct platform_device *pdev =to_platform_device (dev);int len; len =of_device_modalias (dev, buf,PAGE_SIZE );
@@ -1037,9 +1037,7 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *a,if (len != -ENODEV )return len; - len =snprintf (buf,PAGE_SIZE ,"platform:%s\n" , pdev->name); - -return (len >=PAGE_SIZE ) ? (PAGE_SIZE -1 ) : len; +return sysfs_emit (buf,"platform:%s\n" , pdev->name); }static DEVICE_ATTR_RO (modalias);
@@ -1086,12 +1084,13 @@ static ssize_t driver_override_show(struct device *dev,device_lock (dev); len =sysfs_emit (buf,"%s\n" , pdev->driver_override);device_unlock (dev); +return len; }static DEVICE_ATTR_RW (driver_override);static ssize_t numa_node_show (struct device *dev, -struct device_attribute *attr,char *buf) +struct device_attribute *attr,char *buf) {return sysfs_emit (buf,"%d\n" ,dev_to_node (dev)); }
drivers/base/power/sysfs.c +67 -36
259 lines not on this page · raw message
drivers/base/power/wakeup_stats.c +4 -1
@@ -26,7 +26,7 @@ static ssize_t _name##_show(struct device *dev, \ { \struct wakeup_source *ws =dev_get_drvdata (dev); \ \ -return sprintf (buf,"%lu\n" , ws->_name); \ +return sysfs_emit (buf,"%lu\n" , ws->_name); \ } \static DEVICE_ATTR_RO (_name)
@@ -57,6 +57,7 @@ static ssize_t total_time_ms_show(struct device *dev, active_time =ktime_sub (ktime_get (), ws->last_time); total_time =ktime_add (total_time, active_time); } +return sysfs_emit (buf,"%lld\n" ,ktime_to_ms (total_time)); }static DEVICE_ATTR_RO (total_time_ms);
@@ -73,6 +74,7 @@ static ssize_t max_time_ms_show(struct device *dev,if (active_time > max_time) max_time = active_time; } +return sysfs_emit (buf,"%lld\n" ,ktime_to_ms (max_time)); }static DEVICE_ATTR_RO (max_time_ms);
@@ -106,6 +108,7 @@ static ssize_t prevent_suspend_time_ms_show(struct device *dev, prevent_sleep_time =ktime_add (prevent_sleep_time,ktime_sub (ktime_get (), ws->start_prevent_time)); } +return sysfs_emit (buf,"%lld\n" ,ktime_to_ms (prevent_sleep_time)); }static DEVICE_ATTR_RO (prevent_suspend_time_ms);
drivers/base/soc.c +30 -34
95 lines not on this page · raw message
drivers/base/topology.c +5 -5
@@ -14,11 +14,11 @@#include <linux/hardirq.h> #include <linux/topology.h> -#define define_id_show_func (name) \ -static ssize_t name##_show(struct device *dev, \ - struct device_attribute *attr, char *buf) \ -{ \ - return sprintf(buf, "%d\n", topology_##name(dev->id)); \ +#define define_id_show_func (name) \ +static ssize_t name##_show(struct device *dev, \ + struct device_attribute *attr, char *buf) \ +{ \ + return sysfs_emit(buf, "%d\n", topology_##name(dev->id)); \ }#define define_siblings_show_map (name, mask) \
Signature · 1 line
2.26.0
PATCH v3 6/8 mm: and drivers core: Convert hugetlb_report_node_meminfo to sysfs_emit
In reply to Joe PerchesConvert the unbound sprintf in hugetlb_report_node_meminfo to use sysfs_emit_at so that no possible overrun of a PAGE_SIZE buf can occur.
--- drivers/base/node.c | 2 +- include/linux/hugetlb.h | 4 ++-- mm/hugetlb.c | 18 ++++++++++-------- 3 files changed, 13 insertions(+), 11 deletions(-)
drivers/base/node.c +1 -1
@@ -473,7 +473,7 @@ static ssize_t node_read_meminfo(struct device *dev, HPAGE_PMD_NR)#endif ); - len +=hugetlb_report_node_meminfo (nid, buf + len); + len +=hugetlb_report_node_meminfo (buf, len, nid);return len; }
include/linux/hugetlb.h +2 -2
@@ -129,7 +129,7 @@ void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma, unsigned long start, unsigned long end, struct page *ref_page);void hugetlb_report_meminfo (struct seq_file *); -int hugetlb_report_node_meminfo (int ,char *); +int hugetlb_report_node_meminfo (char *buf,int len,int nid);void hugetlb_show_meminfo (void );unsigned long hugetlb_total_pages (void ); vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
@@ -245,7 +245,7 @@ static inline void hugetlb_report_meminfo(struct seq_file *m) { } -static inline int hugetlb_report_node_meminfo (int nid,char *buf) +static inline int hugetlb_report_node_meminfo (char *buf,int len,int nid) {return 0 ; }
mm/hugetlb.c +10 -8
@@ -3571,18 +3571,20 @@ void hugetlb_report_meminfo(struct seq_file *m)seq_printf (m,"Hugetlb: %8lu kB\n" , total /1024 ); } -int hugetlb_report_node_meminfo (int nid,char *buf) +int hugetlb_report_node_meminfo (char *buf,int len,int nid) {struct hstate *h = &default_hstate; +if (!hugepages_supported ())return 0 ; -return sprintf (buf, -"Node %d HugePages_Total: %5u\n" -"Node %d HugePages_Free: %5u\n" -"Node %d HugePages_Surp: %5u\n" , - nid, h->nr_huge_pages_node[nid], - nid, h->free_huge_pages_node[nid], - nid, h->surplus_huge_pages_node[nid]); + +return sysfs_emit_at (buf, len, +"Node %d HugePages_Total: %5u\n" +"Node %d HugePages_Free: %5u\n" +"Node %d HugePages_Surp: %5u\n" , + nid, h->nr_huge_pages_node[nid], + nid, h->free_huge_pages_node[nid], + nid, h->surplus_huge_pages_node[nid]); }void hugetlb_show_meminfo (void )
Signature · 1 line
2.26.0
Re: [PATCH V3 6/8] mm: and drivers core: Convert hugetlb_report_node_meminfo to sysfs_emit
Quoted 4 lines · Joe Perches
Convert the unbound sprintf in
hugetlb_report_node_meminfoto usesysfs_emit_atso that no possible overrun of aPAGE_SIZEbuf can occur.Signed-off-by Joe Perches <[email protected]>
Signature · 1 line
Mike Kravetz
Greg Kroah-Hartman
maintainerRe: [PATCH V3 6/8] mm: and drivers core: Convert hugetlb_report_node_meminfo to sysfs_emit
In reply to Joe PerchesQuoted 66 lines · Joe Perches
Convert the unbound sprintf in
hugetlb_report_node_meminfoto usesysfs_emit_atso that no possible overrun of aPAGE_SIZEbuf can occur.Signed-off-by Joe Perches <[email protected]>--- drivers/base/node.c | 2 +- include/linux/hugetlb.h | 4 ++-- mm/hugetlb.c | 18 ++++++++++-------- 3 files changed, 13 insertions(+), 11 deletions(-)drivers/base/node.c +1
-1@@ -473,7 +473,7 @@ static ssize_t node_read_meminfo(struct device *dev, HPAGE_PMD_NR)#endif ); - len +=hugetlb_report_node_meminfo (nid, buf + len); + len +=hugetlb_report_node_meminfo (buf, len, nid);return len; }include/linux/hugetlb.h +2
-2@@ -129,7 +129,7 @@ void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma, unsigned long start, unsigned long end, struct page *ref_page);void hugetlb_report_meminfo (struct seq_file *); -int hugetlb_report_node_meminfo (int ,char *); +int hugetlb_report_node_meminfo (char *buf,int len,int nid);void hugetlb_show_meminfo (void );unsigned long hugetlb_total_pages (void ); vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,@@ -245,7 +245,7 @@ static inline void hugetlb_report_meminfo(struct seq_file *m) { } -static inline int hugetlb_report_node_meminfo (int nid,char *buf) +static inline int hugetlb_report_node_meminfo (char *buf,int len,int nid) {return 0 ; }mm/hugetlb.c +2
-7@@ -3571,18 +3571,20 @@ void hugetlb_report_meminfo(struct seq_file *m)seq_printf (m,"Hugetlb: %8lu kB\n" , total /1024 ); } -int hugetlb_report_node_meminfo (int nid,char *buf) +int hugetlb_report_node_meminfo (char *buf,int len,int nid) {struct hstate *h = &default_hstate; +if (!hugepages_supported ())return 0 ; - return sprintf(buf, - "Node %d HugePages_Total: %5u\n" - "Node %d HugePages_Free: %5u\n" - "Node %d HugePages_Surp: %5u\n", - nid, h->nr_huge_pages_node[nid], - nid, h->free_huge_pages_node[nid],
mm/hugetlb.c void hugetlb_report_meminfo(struct seq_file *m) · line 3588
Quoted 10 lines
mm/hugetlb.c +8
-1- nid, h->surplus_huge_pages_node[nid]); + +return sysfs_emit_at (buf, len, +"Node %d HugePages_Total: %5u\n" +"Node %d HugePages_Free: %5u\n" +"Node %d HugePages_Surp: %5u\n" , + nid, h->nr_huge_pages_node[nid], + nid, h->free_huge_pages_node[nid], + nid, h->surplus_huge_pages_node[nid]); }
That is NOT one-value-per-file, which is required for sysfs files. This should be 3 different sysfs files.
Ugh.
But that's separate from this series, thanks for redoing this. I'll take a look at it on Monday...
thanks,
greg k-h
Re: [PATCH V3 6/8] mm: and drivers core: Convert hugetlb_report_node_meminfo to sysfs_emit
Quoted 1 line · Greg Kroah-Hartman
I'll take a look at it on Monday...
It's decidedly not urgent/critical.
There are several logical defects all over the kernel for these unbounded sprintfs, but I don't know of an actual instance where the PAGE_SIZE buf is overfilled.
Re: [PATCH V3 6/8] mm: and drivers core: Convert hugetlb_report_node_meminfo to sysfs_emit
In reply to Greg Kroah-HartmanQuoted 1 line · Greg Kroah-Hartman
I'll take a look at it on Monday...
Thoughts?
Re: [PATCH V3 6/8] mm: and drivers core: Convert hugetlb_report_node_meminfo to sysfs_emit
In reply to Greg Kroah-HartmanQuoted 3 lines · Greg Kroah-Hartman
Quoted 2 lines · Joe Perches
Convert the unbound sprintf in
hugetlb_report_node_meminfoto usesysfs_emit_atso that no possible overrun of aPAGE_SIZEbuf can occur.
[]
Quoted 1 line
I'll take a look at it on Monday...
(noting that you didn't say _which_ Monday...)
Greg are you going to take this or should I ask someone else?
The first sysfs_emit addition patch was first posted about a month ago without much change.
I think it'd be useful to get it into -next before 5.9 is released.
Greg Kroah-Hartman
maintainerRe: [PATCH V3 6/8] mm: and drivers core: Convert hugetlb_report_node_meminfo to sysfs_emit
Quoted 8 lines · Joe Perches
Quoted 3 lines · Greg Kroah-Hartman
Quoted 2 lines · Joe Perches
Convert the unbound sprintf in
hugetlb_report_node_meminfoto usesysfs_emit_atso that no possible overrun of aPAGE_SIZEbuf can occur.[]
Quoted 1 line
I'll take a look at it on Monday...
(noting that you didn't say _which_ Monday...)
True :)
Quoted 6 lines
Greg are you going to take this or should I ask someone else?
The first
sysfs_emitaddition patch was first posted about a month ago without much change.I think it'd be useful to get it into -next before 5.9 is released.
I'll dig into this today...
PATCH v3 7/8 drivers core: Use sysfs_emit for shared_cpu_map_show and shared_cpu_list_show
In reply to Joe PerchesDo not indirect the bitmap printing of these shared_cpu show functions by using cpumap_print_to_pagebuf/bitmap_print_to_pagebuf.
Use the more typical style with the vsnprintf %*pb and %*pbl extensions directly so there is no possible mixup about the use of offset_in_page(buf) by bitmap_print_to_pagebuf.
--- drivers/base/cacheinfo.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
drivers/base/cacheinfo.c +7 -9
@@ -380,24 +380,22 @@ static ssize_t size_show(struct device *dev,return sysfs_emit (buf,"%uK\n" , this_leaf->size >>10 ); } -static ssize_t shared_cpumap_show_func (struct device *dev,bool list,char *buf) +static ssize_t shared_cpu_map_show (struct device *dev, +struct device_attribute *attr,char *buf) {struct cacheinfo *this_leaf =dev_get_drvdata (dev);const struct cpumask *mask = &this_leaf->shared_cpu_map; -return cpumap_print_to_pagebuf (list, buf, mask); -} - -static ssize_t shared_cpu_map_show (struct device *dev, -struct device_attribute *attr,char *buf) -{ -return shared_cpumap_show_func (dev, false, buf); +return sysfs_emit (buf,"%*pb\n" , nr_cpu_ids, mask); }static ssize_t shared_cpu_list_show (struct device *dev,struct device_attribute *attr,char *buf) { -return shared_cpumap_show_func (dev, true, buf); +struct cacheinfo *this_leaf =dev_get_drvdata (dev); +const struct cpumask *mask = &this_leaf->shared_cpu_map; + +return sysfs_emit (buf,"%*pbl\n" , nr_cpu_ids, mask); } static ssize_t type_show(struct device *dev,
Signature · 1 line
2.26.0
PATCH v3 8/8 drivers core: node: Use a more typical macro definition style for ACCESS_ATTR
In reply to Joe PerchesRemove the trailing semicolon from the macro and add it to its uses.
--- drivers/base/node.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/base/node.c +5 -5
@@ -165,12 +165,12 @@ static ssize_t name##_show(struct device *dev, \return sysfs_emit (buf,"%u\n" , \to_access_nodes (dev)->hmem_attrs.name); \ } \ -static DEVICE_ATTR_RO (name); +static DEVICE_ATTR_RO (name) -ACCESS_ATTR (read_bandwidth) -ACCESS_ATTR (read_latency ) -ACCESS_ATTR (write_bandwidth) -ACCESS_ATTR (write_latency) +ACCESS_ATTR (read_bandwidth); +ACCESS_ATTR (read_latency); +ACCESS_ATTR (write_bandwidth); +ACCESS_ATTR (write_latency);static struct attribute *access_attrs[] = { &dev_attr_read_bandwidth.attr,
Signature · 1 line
2.26.0