RE: [PATCH 1/1] arch:hexagon/powerpc: use KSYM_NAME_LEN in array size
Hi Peter, > > The best solution would be to pass the buffer size as an extra > parameter. Especially when some code passes buffers that are > allocated/reserved dynamically. > > Sigh, I am not sure how many changes it would require in kallsyms > API and all the callers. But it would be really appreciated, IMHO. > yes we already prepared size changes 5-6 months back: https://lore.kernel.org/lkml/yontol4zc4cyt...@infradead.org/t/ [PATCH 1/5] kallsyms: pass buffer size in sprint_* APIs But at that time new API development(for replacement of seq_buf) was in progress and we decided to wait for that completion. https://lore.kernel.org/r/20220604193042.1674951-2-kent.overstr...@gmail.com https://lore.kernel.org/r/20220604193042.1674951-4-kent.overstr...@gmail.com As I checeked these APIs are not pushed to mainline. we will try to prepare new patch set for kallsym changes again with seq_buf to take care of length argument. Thanks, Maninder Singh
[PATCH 2/2] powerpc/xmon: use KSYM_NAME_LEN in array size
kallsyms_lookup which in turn calls for kallsyms_lookup_buildid()
writes on index "KSYM_NAME_LEN - 1".
Thus array size should be KSYM_NAME_LEN.
for powerpc it was defined as "128" directly.
and commit '61968dbc2d5d' changed define value to 512,
So both were missed to update with new size.
Fixes: 61968dbc2d5d ("kallsyms: increase maximum kernel symbol length to 512")
Co-developed-by: Onkarnath
Signed-off-by: Onkarnath
Signed-off-by: Maninder Singh
---
arch/powerpc/xmon/xmon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 728d3c257e4a..70c4c59a1a8f 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -88,7 +88,7 @@ static unsigned long ndump = 64;
static unsigned long nidump = 16;
static unsigned long ncsum = 4096;
static int termch;
-static char tmpstr[128];
+static char tmpstr[KSYM_NAME_LEN];
static int tracing_enabled;
static long bus_error_jmp[JMP_BUF_LEN];
--
2.17.1
[PATCH 1/2] hexagon/traps.c: use KSYM_NAME_LEN in array size
kallsyms_lookup which in turn calls for kallsyms_lookup_buildid()
writes on index "KSYM_NAME_LEN - 1".
Thus array size should be KSYM_NAME_LEN.
for hexagon it was defined as "128" directly.
and commit '61968dbc2d5d' changed define value to 512,
So both were missed to update with new size.
Fixes: 61968dbc2d5d ("kallsyms: increase maximum kernel symbol length to 512")
Co-developed-by: Onkarnath
Signed-off-by: Onkarnath
Signed-off-by: Maninder Singh
---
arch/hexagon/kernel/traps.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/hexagon/kernel/traps.c b/arch/hexagon/kernel/traps.c
index 6447763ce5a9..65b30b6ea226 100644
--- a/arch/hexagon/kernel/traps.c
+++ b/arch/hexagon/kernel/traps.c
@@ -82,7 +82,7 @@ static void do_show_stack(struct task_struct *task, unsigned
long *fp,
const char *name = NULL;
unsigned long *newfp;
unsigned long low, high;
- char tmpstr[128];
+ char tmpstr[KSYM_NAME_LEN];
char *modname;
int i;
--
2.17.1
RE: [PATCH 1/1] arch:hexagon/powerpc: use KSYM_NAME_LEN in array size
Hi,
>>
>> kallsyms_lookup which in turn calls for kallsyms_lookup_buildid()
>> writes on index "KSYM_NAME_LEN - 1".
>>
>> Thus array size should be KSYM_NAME_LEN.
>>
>> for powerpc and hexagon it was defined as "128" directly.
>> and commit '61968dbc2d5d' changed define value to 512,
>> So both were missed to update with new size.
>>
>> Fixes: 61968dbc2d5d ("kallsyms: increase maximum kernel symbol length to
>> 512")
>> Signed-off-by: Onkarnath
>> Signed-off-by: Maninder Singh
> Thanks for this!
>
> There is no `From:` at the top. Since I cannot locate the patch in
> Lore, did you mean to put both of you as authors perhaps? In that
> case, please use a `Co-developed-by` as needed.
>
I Will add co-developed-by` tag.
because this change was identified while we were working on kallsyms some time
back.
https://lore.kernel.org/lkml/yontol4zc4cyt...@infradead.org/t/
this patch set is pending and we will start working on that again, so i thought
better
to send bugfix first.
> Perhaps it is a good idea to submit each arch independently, too.
>
ok, I will share 2 separate patches.
> The changes themselves look fine on a quick inspection, though the
> `xmon.c` one is a global buffer (and there is another equally-sized
> buffer in `xmon.c` with a hard-coded `128` constant that would be nice
> to clarify).
Yes, I think second buffer was not related to kallsyms, so I have not touched
that.
Thanks,
Maninder Singh
[PATCH 1/1] arch:hexagon/powerpc: use KSYM_NAME_LEN in array size
kallsyms_lookup which in turn calls for kallsyms_lookup_buildid()
writes on index "KSYM_NAME_LEN - 1".
Thus array size should be KSYM_NAME_LEN.
for powerpc and hexagon it was defined as "128" directly.
and commit '61968dbc2d5d' changed define value to 512,
So both were missed to update with new size.
Fixes: 61968dbc2d5d ("kallsyms: increase maximum kernel symbol length to 512")
Signed-off-by: Onkarnath
Signed-off-by: Maninder Singh
---
arch/hexagon/kernel/traps.c | 2 +-
arch/powerpc/xmon/xmon.c| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/hexagon/kernel/traps.c b/arch/hexagon/kernel/traps.c
index 6447763ce5a9..65b30b6ea226 100644
--- a/arch/hexagon/kernel/traps.c
+++ b/arch/hexagon/kernel/traps.c
@@ -82,7 +82,7 @@ static void do_show_stack(struct task_struct *task, unsigned
long *fp,
const char *name = NULL;
unsigned long *newfp;
unsigned long low, high;
- char tmpstr[128];
+ char tmpstr[KSYM_NAME_LEN];
char *modname;
int i;
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 728d3c257e4a..70c4c59a1a8f 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -88,7 +88,7 @@ static unsigned long ndump = 64;
static unsigned long nidump = 16;
static unsigned long ncsum = 4096;
static int termch;
-static char tmpstr[128];
+static char tmpstr[KSYM_NAME_LEN];
static int tracing_enabled;
static long bus_error_jmp[JMP_BUF_LEN];
--
2.17.1
[PATCH 4/5] kallsyms: pass buffer size argument in *lookup* APIs
Although *lookup* APIs are safe, but better to pass size
as an argument rather than using define KSYM_NAME_LEN.
Because it can cause issue if called with lesser array size.
Co-developed-by: Onkarnath
Signed-off-by: Onkarnath
Signed-off-by: Maninder Singh
---
arch/hexagon/kernel/traps.c| 2 +-
arch/powerpc/xmon/xmon.c | 4 ++--
fs/proc/base.c | 2 +-
include/linux/kallsyms.h | 8
include/linux/module.h | 8
kernel/debug/kdb/kdb_support.c | 2 +-
kernel/kallsyms.c | 24
kernel/kprobes.c | 4 ++--
kernel/locking/lockdep.c | 8
kernel/locking/lockdep_internals.h | 2 +-
kernel/locking/lockdep_proc.c | 4 ++--
kernel/module/kallsyms.c | 8
kernel/trace/ftrace.c | 9 +
kernel/trace/trace_kprobe.c| 2 +-
kernel/trace/trace_output.c| 2 +-
kernel/trace/trace_syscalls.c | 2 +-
16 files changed, 46 insertions(+), 45 deletions(-)
diff --git a/arch/hexagon/kernel/traps.c b/arch/hexagon/kernel/traps.c
index 65b30b6ea226..a0306e96e82c 100644
--- a/arch/hexagon/kernel/traps.c
+++ b/arch/hexagon/kernel/traps.c
@@ -118,7 +118,7 @@ static void do_show_stack(struct task_struct *task,
unsigned long *fp,
for (i = 0; i < kstack_depth_to_print; i++) {
- name = kallsyms_lookup(ip, , , , tmpstr);
+ name = kallsyms_lookup(ip, , , , tmpstr,
KSYM_NAME_LEN);
printk("%s[%p] 0x%lx: %s + 0x%lx", loglvl, fp, ip, name,
offset);
if (((unsigned long) fp < low) || (high < (unsigned long) fp))
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 3441fc70ac92..183e2a55ba5c 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -1710,7 +1710,7 @@ static void get_function_bounds(unsigned long pc,
unsigned long *startp,
if (setjmp(bus_error_jmp) == 0) {
catch_memory_errors = 1;
sync();
- name = kallsyms_lookup(pc, , , NULL, tmpstr);
+ name = kallsyms_lookup(pc, , , NULL, tmpstr,
KSYM_NAME_LEN);
if (name != NULL) {
*startp = pc - offset;
*endp = pc - offset + size;
@@ -3730,7 +3730,7 @@ static void xmon_print_symbol(unsigned long address,
const char *mid,
catch_memory_errors = 1;
sync();
name = kallsyms_lookup(address, , , ,
- tmpstr);
+ tmpstr, KSYM_NAME_LEN);
sync();
/* wait a little while to see if we get a machine check */
__delay(200);
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 617816168748..939006f3b2b0 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -392,7 +392,7 @@ static int proc_pid_wchan(struct seq_file *m, struct
pid_namespace *ns,
goto print0;
wchan = get_wchan(task);
- if (wchan && !lookup_symbol_name(wchan, symname)) {
+ if (wchan && !lookup_symbol_name(wchan, symname, KSYM_NAME_LEN)) {
seq_puts(m, symname);
return 0;
}
diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index 598ff08c72d6..8fe535fd848a 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -81,7 +81,7 @@ extern int kallsyms_lookup_size_offset(unsigned long addr,
const char *kallsyms_lookup(unsigned long addr,
unsigned long *symbolsize,
unsigned long *offset,
- char **modname, char *namebuf);
+ char **modname, char *namebuf, size_t size);
/* Look up a kernel symbol and return it in a text buffer. */
extern int sprint_symbol(char *buffer, size_t size, unsigned long address);
@@ -90,7 +90,7 @@ extern int sprint_symbol_no_offset(char *buffer, size_t size,
unsigned long addr
extern int sprint_backtrace(char *buffer, size_t size, unsigned long address);
extern int sprint_backtrace_build_id(char *buffer, size_t size, unsigned long
address);
-int lookup_symbol_name(unsigned long addr, char *symname);
+int lookup_symbol_name(unsigned long addr, char *symname, size_t size);
int lookup_symbol_attrs(unsigned long addr, unsigned long *size, unsigned long
*offset, char *modname, char *name);
/* How and when do we show kallsyms values? */
@@ -113,7 +113,7 @@ static inline int kallsyms_lookup_size_offset(unsigned long
addr,
static inline const char *kallsyms_lookup(unsigned long addr,
unsigned long *symbolsize,
unsigned long *offset,
- char **modname, char *namebuf)
+ char **m
[PATCH 5/5] kallsyms: remove unsed API lookup_symbol_attrs
with commit '7878c231dae0 ("slab: remove /proc/slab_allocators")'
lookup_symbol_attrs usage is removed.
Thus removing redundant API.
Signed-off-by: Maninder Singh
---
include/linux/kallsyms.h | 6 --
include/linux/module.h | 6 --
kernel/kallsyms.c| 28
kernel/module/kallsyms.c | 28
4 files changed, 68 deletions(-)
diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index 8fe535fd848a..b78e9d942a77 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -91,7 +91,6 @@ extern int sprint_backtrace(char *buffer, size_t size,
unsigned long address);
extern int sprint_backtrace_build_id(char *buffer, size_t size, unsigned long
address);
int lookup_symbol_name(unsigned long addr, char *symname, size_t size);
-int lookup_symbol_attrs(unsigned long addr, unsigned long *size, unsigned long
*offset, char *modname, char *name);
/* How and when do we show kallsyms values? */
extern bool kallsyms_show_value(const struct cred *cred);
@@ -153,11 +152,6 @@ static inline int lookup_symbol_name(unsigned long addr,
char *symname, size_t s
return -ERANGE;
}
-static inline int lookup_symbol_attrs(unsigned long addr, unsigned long *size,
unsigned long *offset, char *modname, char *name)
-{
- return -ERANGE;
-}
-
static inline bool kallsyms_show_value(const struct cred *cred)
{
return false;
diff --git a/include/linux/module.h b/include/linux/module.h
index 9b91209d615f..4c5f8f99a252 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -658,7 +658,6 @@ const char *module_address_lookup(unsigned long addr,
char **modname, const unsigned char **modbuildid,
char *namebuf, size_t buf_size);
int lookup_module_symbol_name(unsigned long addr, char *symname, size_t size);
-int lookup_module_symbol_attrs(unsigned long addr, unsigned long *size,
unsigned long *offset, char *modname, char *name);
int register_module_notifier(struct notifier_block *nb);
int unregister_module_notifier(struct notifier_block *nb);
@@ -766,11 +765,6 @@ static inline int lookup_module_symbol_name(unsigned long
addr, char *symname, s
return -ERANGE;
}
-static inline int lookup_module_symbol_attrs(unsigned long addr, unsigned long
*size, unsigned long *offset, char *modname, char *name)
-{
- return -ERANGE;
-}
-
static inline int module_get_kallsym(unsigned int symnum, unsigned long *value,
char *type, char *name,
char *module_name, int *exported)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index d6efce28505d..96ad59b5b2fd 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -430,34 +430,6 @@ int lookup_symbol_name(unsigned long addr, char *symname,
size_t size)
return 0;
}
-int lookup_symbol_attrs(unsigned long addr, unsigned long *size,
- unsigned long *offset, char *modname, char *name)
-{
- int res;
-
- name[0] = '\0';
- name[KSYM_NAME_LEN - 1] = '\0';
-
- if (is_ksym_addr(addr)) {
- unsigned long pos;
-
- pos = get_symbol_pos(addr, size, offset);
- /* Grab name */
- kallsyms_expand_symbol(get_symbol_offset(pos),
- name, KSYM_NAME_LEN);
- modname[0] = '\0';
- goto found;
- }
- /* See if it's in a module. */
- res = lookup_module_symbol_attrs(addr, size, offset, modname, name);
- if (res)
- return res;
-
-found:
- cleanup_symbol_name(name);
- return 0;
-}
-
/* Look up a kernel symbol and return it in a text buffer. */
static int __sprint_symbol(char *buffer, size_t buf_size, unsigned long
address,
int symbol_offset, int add_offset, int add_buildid)
diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index c982860405c6..e6f16c62a888 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -375,34 +375,6 @@ int lookup_module_symbol_name(unsigned long addr, char
*symname, size_t size)
return -ERANGE;
}
-int lookup_module_symbol_attrs(unsigned long addr, unsigned long *size,
- unsigned long *offset, char *modname, char *name)
-{
- struct module *mod;
-
- preempt_disable();
- list_for_each_entry_rcu(mod, , list) {
- if (mod->state == MODULE_STATE_UNFORMED)
- continue;
- if (within_module(addr, mod)) {
- const char *sym;
-
- sym = find_kallsyms_symbol(mod, addr, size, offset);
- if (!sym)
- goto out;
- if (modname)
- strscpy(modname, mod->name, MODULE_NAME_LEN);
-
[PATCH 3/5] arch:hexagon/powerpc: use KSYM_NAME_LEN as array size
kallsyms_lookup which in turn calls for kallsyms_lookup_buildid()
writes on index "KSYM_NAME_LEN - 1".
Thus array size should be KSYM_NAME_LEN.
for powerpc and hexagon it was defined as "128" directly.
and commit '61968dbc2d5d' changed define value to 512,
So both were missed to update with new size.
Fixes: 61968dbc2d5d("kallsyms: increase maximum kernel symbol length to 512")
Signed-off-by: Maninder Singh
---
arch/hexagon/kernel/traps.c | 2 +-
arch/powerpc/xmon/xmon.c| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/hexagon/kernel/traps.c b/arch/hexagon/kernel/traps.c
index 6447763ce5a9..65b30b6ea226 100644
--- a/arch/hexagon/kernel/traps.c
+++ b/arch/hexagon/kernel/traps.c
@@ -82,7 +82,7 @@ static void do_show_stack(struct task_struct *task, unsigned
long *fp,
const char *name = NULL;
unsigned long *newfp;
unsigned long low, high;
- char tmpstr[128];
+ char tmpstr[KSYM_NAME_LEN];
char *modname;
int i;
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index afc827c65ff2..3441fc70ac92 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -91,7 +91,7 @@ static unsigned long ndump = 64;
static unsigned long nidump = 16;
static unsigned long ncsum = 4096;
static int termch;
-static char tmpstr[128];
+static char tmpstr[KSYM_NAME_LEN];
static int tracing_enabled;
static long bus_error_jmp[JMP_BUF_LEN];
--
2.17.1
[PATCH 1/5] kallsyms: pass buffer size in sprint_* APIs
As of now sprint_* APIs don't pass buffer size as an argument
and use sprintf directly.
To replace dangerous sprintf API to scnprintf,
buffer size is required in arguments.
Co-developed-by: Onkarnath
Signed-off-by: Onkarnath
Signed-off-by: Maninder Singh
---
arch/s390/lib/test_unwind.c| 2 +-
drivers/scsi/fnic/fnic_trace.c | 8
include/linux/kallsyms.h | 20 ++--
init/main.c| 2 +-
kernel/kallsyms.c | 27 ---
kernel/trace/trace_output.c| 2 +-
lib/vsprintf.c | 10 +-
7 files changed, 38 insertions(+), 33 deletions(-)
diff --git a/arch/s390/lib/test_unwind.c b/arch/s390/lib/test_unwind.c
index 5a053b393d5c..adbc2b53db16 100644
--- a/arch/s390/lib/test_unwind.c
+++ b/arch/s390/lib/test_unwind.c
@@ -75,7 +75,7 @@ static noinline int test_unwind(struct task_struct *task,
struct pt_regs *regs,
ret = -EINVAL;
break;
}
- sprint_symbol(sym, addr);
+ sprint_symbol(sym, KSYM_SYMBOL_LEN, addr);
if (bt_pos < BT_BUF_SIZE) {
bt_pos += snprintf(bt + bt_pos, BT_BUF_SIZE - bt_pos,
state.reliable ? " [%-7s%px] %pSR\n"
:
diff --git a/drivers/scsi/fnic/fnic_trace.c b/drivers/scsi/fnic/fnic_trace.c
index 4a7536bb0ab3..33acaa9bb4ba 100644
--- a/drivers/scsi/fnic/fnic_trace.c
+++ b/drivers/scsi/fnic/fnic_trace.c
@@ -128,10 +128,10 @@ int fnic_get_trace_data(fnic_dbgfs_t *fnic_dbgfs_prt)
}
/* Convert function pointer to function name */
if (sizeof(unsigned long) < 8) {
- sprint_symbol(str, tbp->fnaddr.low);
+ sprint_symbol(str, KSYM_SYMBOL_LEN,
tbp->fnaddr.low);
jiffies_to_timespec64(tbp->timestamp.low, );
} else {
- sprint_symbol(str, tbp->fnaddr.val);
+ sprint_symbol(str, KSYM_SYMBOL_LEN,
tbp->fnaddr.val);
jiffies_to_timespec64(tbp->timestamp.val, );
}
/*
@@ -170,10 +170,10 @@ int fnic_get_trace_data(fnic_dbgfs_t *fnic_dbgfs_prt)
}
/* Convert function pointer to function name */
if (sizeof(unsigned long) < 8) {
- sprint_symbol(str, tbp->fnaddr.low);
+ sprint_symbol(str, KSYM_SYMBOL_LEN,
tbp->fnaddr.low);
jiffies_to_timespec64(tbp->timestamp.low, );
} else {
- sprint_symbol(str, tbp->fnaddr.val);
+ sprint_symbol(str, KSYM_SYMBOL_LEN,
tbp->fnaddr.val);
jiffies_to_timespec64(tbp->timestamp.val, );
}
/*
diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index 649faac31ddb..598ff08c72d6 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -84,11 +84,11 @@ const char *kallsyms_lookup(unsigned long addr,
char **modname, char *namebuf);
/* Look up a kernel symbol and return it in a text buffer. */
-extern int sprint_symbol(char *buffer, unsigned long address);
-extern int sprint_symbol_build_id(char *buffer, unsigned long address);
-extern int sprint_symbol_no_offset(char *buffer, unsigned long address);
-extern int sprint_backtrace(char *buffer, unsigned long address);
-extern int sprint_backtrace_build_id(char *buffer, unsigned long address);
+extern int sprint_symbol(char *buffer, size_t size, unsigned long address);
+extern int sprint_symbol_build_id(char *buffer, size_t size, unsigned long
address);
+extern int sprint_symbol_no_offset(char *buffer, size_t size, unsigned long
address);
+extern int sprint_backtrace(char *buffer, size_t size, unsigned long address);
+extern int sprint_backtrace_build_id(char *buffer, size_t size, unsigned long
address);
int lookup_symbol_name(unsigned long addr, char *symname);
int lookup_symbol_attrs(unsigned long addr, unsigned long *size, unsigned long
*offset, char *modname, char *name);
@@ -118,31 +118,31 @@ static inline const char *kallsyms_lookup(unsigned long
addr,
return NULL;
}
-static inline int sprint_symbol(char *buffer, unsigned long addr)
+static inline int sprint_symbol(char *buffer, size_t size, unsigned long addr)
{
*buffer = '\0';
return 0;
}
-static inline int sprint_symbol_build_id(char *buffer, unsigned long address)
+static inline int sprint_symbol_build_id(char *buffer, size_t size, unsigned
long address)
{
*buffer = '\0';
return 0;
}
-static in
[PATCH 2/5] kallsyms: replace sprintf with scnprintf
replace sprintf API with scnprintf which prevents buffer overflow.
Co-developed-by: Onkarnath
Signed-off-by: Onkarnath
Signed-off-by: Maninder Singh
---
kernel/kallsyms.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index f354378e241f..9e4316fe0ba1 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -472,28 +472,29 @@ static int __sprint_symbol(char *buffer, size_t buf_size,
unsigned long address,
name = kallsyms_lookup_buildid(address, , , ,
,
buffer);
if (!name)
- return sprintf(buffer, "0x%lx", address - symbol_offset);
+ return scnprintf(buffer, buf_size, "0x%lx", address -
symbol_offset);
if (name != buffer)
- strcpy(buffer, name);
+ strncpy(buffer, name, buf_size);
+
len = strlen(buffer);
offset -= symbol_offset;
if (add_offset)
- len += sprintf(buffer + len, "+%#lx/%#lx", offset, size);
+ len += scnprintf(buffer + len, buf_size - len, "+%#lx/%#lx",
offset, size);
if (modname) {
- len += sprintf(buffer + len, " [%s", modname);
+ len += scnprintf(buffer + len, buf_size - len, " [%s", modname);
#if IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID)
if (add_buildid && buildid) {
/* build ID should match length of sprintf */
#if IS_ENABLED(CONFIG_MODULES)
static_assert(sizeof(typeof_member(struct module,
build_id)) == 20);
#endif
- len += sprintf(buffer + len, " %20phN", buildid);
+ len += scnprintf(buffer + len, buf_size - len, "
%20phN", buildid);
}
#endif
- len += sprintf(buffer + len, "]");
+ len += scnprintf(buffer + len, buf_size - len, "]");
}
return len;
--
2.17.1
[PATCH 0/5] kallsyms: make kallsym APIs more safe with scnprintf
kallsyms functionality depends on KSYM_NAME_LEN directly. but if user passed array length lesser than it, sprintf can cause issues of buffer overflow attack. So changing *sprint* and *lookup* APIs in this patch set to have buffer size as an argument and replacing sprintf with scnprintf. patch 1 and 2 can be clubbed, but then it will be difficult to review, so patch 1 changes prototype only and patch 2 includes passed argument usage. Patch 3 and patch 5 are bug fixes. Patch 1, 2 and 4 are changing prorotypes. Tried build and kallsyms test on ARM64 environment. APIs are called at multiple places. So build can be failed if updation missed at any place. lets see if autobot reports any build failure with any config combination. [ 12.247313] ps function_check [crash] [ 12.247906] pS function_check+0x4/0x40 [crash] [ 12.247999] pSb function_check+0x4/0x40 [crash df48d71893b7fb2688ac9739346449e89e8a76ca] [ 12.248092] pB function_check+0x4/0x40 [crash] [ 12.248190] pBb function_check+0x4/0x40 [crash df48d71893b7fb2688ac9739346449e89e8a76ca] ... [ 12.261175] Call trace: [ 12.261361] function_2+0x74/0x88 [crash df48d71893b7fb2688ac9739346449e89e8a76ca] [ 12.261859] function_1+0x10/0x1c [crash df48d71893b7fb2688ac9739346449e89e8a76ca] [ 12.262237] hello_init+0x24/0x34 [crash df48d71893b7fb2688ac9739346449e89e8a76ca] [ 12.262603] do_one_initcall+0x54/0x1c8 [ 12.262803] do_init_module+0x44/0x1d0 [ 12.262992] load_module+0x1688/0x19f0 [ 12.263179] __do_sys_init_module+0x1a0/0x210 [ 12.263387] __arm64_sys_init_module+0x1c/0x28 [ 12.263596] invoke_syscall+0x44/0x108 [ 12.263788] el0_svc_common.constprop.0+0x44/0xf0 [ 12.264014] do_el0_svc_compat+0x1c/0x50 [ 12.264209] el0_svc_compat+0x2c/0x88 [ 12.264397] el0t_32_sync_handler+0x90/0x140 [ 12.264600] el0t_32_sync+0x190/0x194 Maninder Singh, Onkarnath (5): kallsyms: pass buffer size in sprint_* APIs kallsyms: replace sprintf with scprintf arch:hexagon/powerpc: use KSYM_NAME_LEN as array size kallsyms: pass buffer size argument in *lookup* APIs kallsyms: remove unsed API lookup_symbol_attrs arch/hexagon/kernel/traps.c| 4 +- arch/powerpc/xmon/xmon.c | 6 +- arch/s390/lib/test_unwind.c| 2 +- drivers/scsi/fnic/fnic_trace.c | 8 +-- fs/proc/base.c | 2 +- include/linux/kallsyms.h | 34 +-- include/linux/module.h | 14 ++--- init/main.c| 2 +- kernel/debug/kdb/kdb_support.c | 2 +- kernel/kallsyms.c | 92 -- kernel/kprobes.c | 4 +- kernel/locking/lockdep.c | 8 +-- kernel/locking/lockdep_internals.h | 2 +- kernel/locking/lockdep_proc.c | 4 +- kernel/module/kallsyms.c | 36 ++-- kernel/trace/ftrace.c | 9 +-- kernel/trace/trace_kprobe.c| 2 +- kernel/trace/trace_output.c| 4 +- kernel/trace/trace_syscalls.c | 2 +- lib/vsprintf.c | 10 ++-- 20 files changed, 93 insertions(+), 154 deletions(-) -- 2.17.1
[PATCH 1/1] cxl/vphb.c: Use phb pointer after NULL check
static Anlaysis detected below error:- (error) Possible null pointer dereference: phb So, Use phb after NULL check. Signed-off-by: Maninder Singh maninder...@samsung.com --- drivers/misc/cxl/vphb.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/misc/cxl/vphb.c b/drivers/misc/cxl/vphb.c index b1d1983a..2eba002 100644 --- a/drivers/misc/cxl/vphb.c +++ b/drivers/misc/cxl/vphb.c @@ -112,9 +112,10 @@ static int cxl_pcie_config_info(struct pci_bus *bus, unsigned int devfn, unsigned long addr; phb = pci_bus_to_host(bus); - afu = (struct cxl_afu *)phb-private_data; if (phb == NULL) return PCIBIOS_DEVICE_NOT_FOUND; + afu = (struct cxl_afu *)phb-private_data; + if (cxl_pcie_cfg_record(bus-number, devfn) afu-crs_num) return PCIBIOS_DEVICE_NOT_FOUND; if (offset = (unsigned long)phb-cfg_data) -- 1.7.9.5 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
