[PATCH 0/5] kallsyms: make kallsym APIs more safe with scnprintf

[PATCH 0/5] kallsyms: make kallsym APIs more safe with scnprintf

[Maninder Singh]

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/5] kallsyms: pass buffer size in sprint_* APIs

[Maninder Singh]

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 <onkarnath.1@samsung.com>
Signed-off-by: Onkarnath <onkarnath.1@samsung.com>
Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
---
 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, &val);
 			} else {
-				sprint_symbol(str, tbp->fnaddr.val);
+				sprint_symbol(str, KSYM_SYMBOL_LEN, tbp->fnaddr.val);
 				jiffies_to_timespec64(tbp->timestamp.val, &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, &val);
 			} else {
-				sprint_symbol(str, tbp->fnaddr.val);
+				sprint_symbol(str, KSYM_SYMBOL_LEN, tbp->fnaddr.val);
 				jiffies_to_timespec64(tbp->timestamp.val, &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 inline int sprint_symbol_no_offset(char *buffer, unsigned long addr)
+static inline int sprint_symbol_no_offset(char *buffer, size_t size, unsigned long addr)
 {
 	*buffer = '\0';
 	return 0;
 }
 
-static inline int sprint_backtrace(char *buffer, unsigned long addr)
+static inline int sprint_backtrace(char *buffer, size_t size, unsigned long addr)
 {
 	*buffer = '\0';
 	return 0;
 }
 
-static inline int sprint_backtrace_build_id(char *buffer, unsigned long addr)
+static inline int sprint_backtrace_build_id(char *buffer, size_t size, unsigned long addr)
 {
 	*buffer = '\0';
 	return 0;
diff --git a/init/main.c b/init/main.c
index 40255f110885..399a15857bf9 100644
--- a/init/main.c
+++ b/init/main.c
@@ -1207,7 +1207,7 @@ static bool __init_or_module initcall_blacklisted(initcall_t fn)
 		return false;
 
 	addr = (unsigned long) dereference_function_descriptor(fn);
-	sprint_symbol_no_offset(fn_name, addr);
+	sprint_symbol_no_offset(fn_name, KSYM_SYMBOL_LEN, addr);
 
 	/*
 	 * fn will be "function_name [module_name]" where [module_name] is not
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 87e2b1638115..f354378e241f 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -459,7 +459,7 @@ int lookup_symbol_attrs(unsigned long addr, unsigned long *size,
 }
 
 /* Look up a kernel symbol and return it in a text buffer. */
-static int __sprint_symbol(char *buffer, unsigned long address,
+static int __sprint_symbol(char *buffer, size_t buf_size, unsigned long address,
 			   int symbol_offset, int add_offset, int add_buildid)
 {
 	char *modname;
@@ -502,6 +502,7 @@ static int __sprint_symbol(char *buffer, unsigned long address,
 /**
  * sprint_symbol - Look up a kernel symbol and return it in a text buffer
  * @buffer: buffer to be stored
+ * @size: size of buffer
  * @address: address to lookup
  *
  * This function looks up a kernel symbol with @address and stores its name,
@@ -510,15 +511,16 @@ static int __sprint_symbol(char *buffer, unsigned long address,
  *
  * This function returns the number of bytes stored in @buffer.
  */
-int sprint_symbol(char *buffer, unsigned long address)
+int sprint_symbol(char *buffer, size_t size, unsigned long address)
 {
-	return __sprint_symbol(buffer, address, 0, 1, 0);
+	return __sprint_symbol(buffer, size, address, 0, 1, 0);
 }
 EXPORT_SYMBOL_GPL(sprint_symbol);
 
 /**
  * sprint_symbol_build_id - Look up a kernel symbol and return it in a text buffer
  * @buffer: buffer to be stored
+ * @size: size of buffer
  * @address: address to lookup
  *
  * This function looks up a kernel symbol with @address and stores its name,
@@ -527,15 +529,16 @@ EXPORT_SYMBOL_GPL(sprint_symbol);
  *
  * This function returns the number of bytes stored in @buffer.
  */
-int sprint_symbol_build_id(char *buffer, unsigned long address)
+int sprint_symbol_build_id(char *buffer, size_t size, unsigned long address)
 {
-	return __sprint_symbol(buffer, address, 0, 1, 1);
+	return __sprint_symbol(buffer, size, address, 0, 1, 1);
 }
 EXPORT_SYMBOL_GPL(sprint_symbol_build_id);
 
 /**
  * sprint_symbol_no_offset - Look up a kernel symbol and return it in a text buffer
  * @buffer: buffer to be stored
+ * @size: size of buffer
  * @address: address to lookup
  *
  * This function looks up a kernel symbol with @address and stores its name
@@ -544,15 +547,16 @@ EXPORT_SYMBOL_GPL(sprint_symbol_build_id);
  *
  * This function returns the number of bytes stored in @buffer.
  */
-int sprint_symbol_no_offset(char *buffer, unsigned long address)
+int sprint_symbol_no_offset(char *buffer, size_t size, unsigned long address)
 {
-	return __sprint_symbol(buffer, address, 0, 0, 0);
+	return __sprint_symbol(buffer, size, address, 0, 0, 0);
 }
 EXPORT_SYMBOL_GPL(sprint_symbol_no_offset);
 
 /**
  * sprint_backtrace - Look up a backtrace symbol and return it in a text buffer
  * @buffer: buffer to be stored
+ * @size: size of buffer
  * @address: address to lookup
  *
  * This function is for stack backtrace and does the same thing as
@@ -564,14 +568,15 @@ EXPORT_SYMBOL_GPL(sprint_symbol_no_offset);
  *
  * This function returns the number of bytes stored in @buffer.
  */
-int sprint_backtrace(char *buffer, unsigned long address)
+int sprint_backtrace(char *buffer, size_t size, unsigned long address)
 {
-	return __sprint_symbol(buffer, address, -1, 1, 0);
+	return __sprint_symbol(buffer, size, address, -1, 1, 0);
 }
 
 /**
  * sprint_backtrace_build_id - Look up a backtrace symbol and return it in a text buffer
  * @buffer: buffer to be stored
+ * @size: size of buffer
  * @address: address to lookup
  *
  * This function is for stack backtrace and does the same thing as
@@ -584,9 +589,9 @@ int sprint_backtrace(char *buffer, unsigned long address)
  *
  * This function returns the number of bytes stored in @buffer.
  */
-int sprint_backtrace_build_id(char *buffer, unsigned long address)
+int sprint_backtrace_build_id(char *buffer, size_t size, unsigned long address)
 {
-	return __sprint_symbol(buffer, address, -1, 1, 1);
+	return __sprint_symbol(buffer, size, address, -1, 1, 1);
 }
 
 /* To avoid using get_symbol_offset for every symbol, we carry prefix along. */
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 8aa493d25c73..2a6ec049cab5 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -362,7 +362,7 @@ trace_seq_print_sym(struct trace_seq *s, unsigned long address, bool offset)
 	const char *name;
 
 	if (offset)
-		sprint_symbol(str, address);
+		sprint_symbol(str, KSYM_SYMBOL_LEN, address);
 	else
 		kallsyms_lookup(address, NULL, NULL, NULL, str);
 	name = kretprobed(str, address);
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index f8ff861ef24a..cb241b63c967 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -991,15 +991,15 @@ char *symbol_string(char *buf, char *end, void *ptr,
 
 #ifdef CONFIG_KALLSYMS
 	if (*fmt == 'B' && fmt[1] == 'b')
-		sprint_backtrace_build_id(sym, value);
+		sprint_backtrace_build_id(sym, KSYM_SYMBOL_LEN, value);
 	else if (*fmt == 'B')
-		sprint_backtrace(sym, value);
+		sprint_backtrace(sym, KSYM_SYMBOL_LEN, value);
 	else if (*fmt == 'S' && (fmt[1] == 'b' || (fmt[1] == 'R' && fmt[2] == 'b')))
-		sprint_symbol_build_id(sym, value);
+		sprint_symbol_build_id(sym, KSYM_SYMBOL_LEN, value);
 	else if (*fmt != 's')
-		sprint_symbol(sym, value);
+		sprint_symbol(sym, KSYM_SYMBOL_LEN, value);
 	else
-		sprint_symbol_no_offset(sym, value);
+		sprint_symbol_no_offset(sym, KSYM_SYMBOL_LEN, value);
 
 	return string_nocheck(buf, end, sym, spec);
 #else
-- 
2.17.1

Re: [PATCH 1/5] kallsyms: pass buffer size in sprint_* APIs

[Waiman Long]

On 5/20/22 04:36, Maninder Singh wrote:
> 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 <onkarnath.1@samsung.com>
> Signed-off-by: Onkarnath <onkarnath.1@samsung.com>
> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
> ---
>   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);

Instead of hardcoding KSYM_SYMBOL_LEN everywhere, will it better to hide 
it like this:

         extern int __sprint_symbol(char *buffer, size_t size, unsigned 
long address);
         #define sprint_symbol(buf, addr)        __sprint_symbol(buf, 
sizeof(buf), addr)

Or you can use sizeof(buf) directly instead of KSYM_SYMBOL_LEN.

Cheers,
Longman

Re: [PATCH 1/5] kallsyms: pass buffer size in sprint_* APIs

[Andy Shevchenko]

On Fri, May 20, 2022 at 03:52:01PM -0400, Waiman Long wrote:
> On 5/20/22 04:36, Maninder Singh wrote:

...

> > -		sprint_symbol(sym, addr);
> > +		sprint_symbol(sym, KSYM_SYMBOL_LEN, addr);
> 
> Instead of hardcoding KSYM_SYMBOL_LEN everywhere, will it better to hide it
> like this:
> 
>         extern int __sprint_symbol(char *buffer, size_t size, unsigned long
> address);
>         #define sprint_symbol(buf, addr)        __sprint_symbol(buf,
> sizeof(buf), addr)
> 
> Or you can use sizeof(buf) directly instead of KSYM_SYMBOL_LEN.

This assumes that buf is defined as char [], which might be not always the
case. If you are going with the macro, than ARRAY_SIZE() seems appropriate
to perform a check against the above mentioned constraint.


-- 
With Best Regards,
Andy Shevchenko

[PATCH 2/5] kallsyms: replace sprintf with scnprintf

[Maninder Singh]

replace sprintf API with scnprintf which prevents buffer overflow.

Co-developed-by: Onkarnath <onkarnath.1@samsung.com>
Signed-off-by: Onkarnath <onkarnath.1@samsung.com>
Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
---
 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, &size, &offset, &modname, &buildid,
 				       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 3/5] arch:hexagon/powerpc: use KSYM_NAME_LEN as array size

[Maninder Singh]

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 <maninder1.s@samsung.com>
---
 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 4/5] kallsyms: pass buffer size argument in *lookup* APIs

[Maninder Singh]

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 <onkarnath.1@samsung.com>
Signed-off-by: Onkarnath <onkarnath.1@samsung.com>
Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
---
 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, &size, &offset, &modname, tmpstr);
+		name = kallsyms_lookup(ip, &size, &offset, &modname, 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, &size, &offset, NULL, tmpstr);
+		name = kallsyms_lookup(pc, &size, &offset, 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, &size, &offset, &modname,
-				       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 **modname, char *namebuf, size_t size)
 {
 	return NULL;
 }
@@ -148,7 +148,7 @@ static inline int sprint_backtrace_build_id(char *buffer, size_t size, unsigned
 	return 0;
 }
 
-static inline int lookup_symbol_name(unsigned long addr, char *symname)
+static inline int lookup_symbol_name(unsigned long addr, char *symname, size_t size)
 {
 	return -ERANGE;
 }
diff --git a/include/linux/module.h b/include/linux/module.h
index abd9fa916b7d..9b91209d615f 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -656,8 +656,8 @@ const char *module_address_lookup(unsigned long addr,
 			    unsigned long *symbolsize,
 			    unsigned long *offset,
 			    char **modname, const unsigned char **modbuildid,
-			    char *namebuf);
-int lookup_module_symbol_name(unsigned long addr, char *symname);
+			    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);
@@ -756,12 +756,12 @@ static inline const char *module_address_lookup(unsigned long addr,
 					  unsigned long *offset,
 					  char **modname,
 					  const unsigned char **modbuildid,
-					  char *namebuf)
+					  char *namebuf, size_t buf_size)
 {
 	return NULL;
 }
 
-static inline int lookup_module_symbol_name(unsigned long addr, char *symname)
+static inline int lookup_module_symbol_name(unsigned long addr, char *symname, size_t size)
 {
 	return -ERANGE;
 }
diff --git a/kernel/debug/kdb/kdb_support.c b/kernel/debug/kdb/kdb_support.c
index 0a39497140bf..bf19e9587c23 100644
--- a/kernel/debug/kdb/kdb_support.c
+++ b/kernel/debug/kdb/kdb_support.c
@@ -92,7 +92,7 @@ int kdbnearsym(unsigned long addr, kdb_symtab_t *symtab)
 		goto out;
 
 	symtab->sym_name = kallsyms_lookup(addr, &symbolsize , &offset,
-				(char **)(&symtab->mod_name), namebuf);
+				(char **)(&symtab->mod_name), namebuf, KSYM_NAME_LEN);
 	if (offset > 8*1024*1024) {
 		symtab->sym_name = NULL;
 		addr = offset = symbolsize = 0;
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 9e4316fe0ba1..d6efce28505d 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -342,18 +342,18 @@ int kallsyms_lookup_size_offset(unsigned long addr, unsigned long *symbolsize,
 		get_symbol_pos(addr, symbolsize, offset);
 		return 1;
 	}
-	return !!module_address_lookup(addr, symbolsize, offset, NULL, NULL, namebuf) ||
+	return !!module_address_lookup(addr, symbolsize, offset, NULL, NULL, namebuf, KSYM_NAME_LEN) ||
 	       !!__bpf_address_lookup(addr, symbolsize, offset, namebuf);
 }
 
 static const char *kallsyms_lookup_buildid(unsigned long addr,
 			unsigned long *symbolsize,
 			unsigned long *offset, char **modname,
-			const unsigned char **modbuildid, char *namebuf)
+			const unsigned char **modbuildid, char *namebuf, size_t size)
 {
 	const char *ret;
 
-	namebuf[KSYM_NAME_LEN - 1] = 0;
+	namebuf[size - 1] = 0;
 	namebuf[0] = 0;
 
 	if (is_ksym_addr(addr)) {
@@ -362,7 +362,7 @@ static const char *kallsyms_lookup_buildid(unsigned long addr,
 		pos = get_symbol_pos(addr, symbolsize, offset);
 		/* Grab name */
 		kallsyms_expand_symbol(get_symbol_offset(pos),
-				       namebuf, KSYM_NAME_LEN);
+				       namebuf, size);
 		if (modname)
 			*modname = NULL;
 		if (modbuildid)
@@ -374,7 +374,7 @@ static const char *kallsyms_lookup_buildid(unsigned long addr,
 
 	/* See if it's in a module or a BPF JITed image. */
 	ret = module_address_lookup(addr, symbolsize, offset,
-				    modname, modbuildid, namebuf);
+				    modname, modbuildid, namebuf, size);
 	if (!ret)
 		ret = bpf_address_lookup(addr, symbolsize,
 					 offset, modname, namebuf);
@@ -398,18 +398,18 @@ static const char *kallsyms_lookup_buildid(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)
 {
 	return kallsyms_lookup_buildid(addr, symbolsize, offset, modname,
-				       NULL, namebuf);
+				       NULL, namebuf, size);
 }
 
-int lookup_symbol_name(unsigned long addr, char *symname)
+int lookup_symbol_name(unsigned long addr, char *symname, size_t size)
 {
 	int res;
 
 	symname[0] = '\0';
-	symname[KSYM_NAME_LEN - 1] = '\0';
+	symname[size - 1] = '\0';
 
 	if (is_ksym_addr(addr)) {
 		unsigned long pos;
@@ -417,11 +417,11 @@ int lookup_symbol_name(unsigned long addr, char *symname)
 		pos = get_symbol_pos(addr, NULL, NULL);
 		/* Grab name */
 		kallsyms_expand_symbol(get_symbol_offset(pos),
-				       symname, KSYM_NAME_LEN);
+				       symname, size);
 		goto found;
 	}
 	/* See if it's in a module. */
-	res = lookup_module_symbol_name(addr, symname);
+	res = lookup_module_symbol_name(addr, symname, size);
 	if (res)
 		return res;
 
@@ -470,7 +470,7 @@ static int __sprint_symbol(char *buffer, size_t buf_size, unsigned long address,
 
 	address += symbol_offset;
 	name = kallsyms_lookup_buildid(address, &size, &offset, &modname, &buildid,
-				       buffer);
+				       buffer, buf_size);
 	if (!name)
 		return scnprintf(buffer, buf_size, "0x%lx", address - symbol_offset);
 
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index dd58c0be9ce2..3b362b70e72b 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1478,7 +1478,7 @@ bool within_kprobe_blacklist(unsigned long addr)
 		return true;
 
 	/* Check if the address is on a suffixed-symbol */
-	if (!lookup_symbol_name(addr, symname)) {
+	if (!lookup_symbol_name(addr, symname, KSYM_NAME_LEN)) {
 		p = strchr(symname, '.');
 		if (!p)
 			return false;
@@ -2806,7 +2806,7 @@ static int show_kprobe_addr(struct seq_file *pi, void *v)
 	preempt_disable();
 	hlist_for_each_entry_rcu(p, head, hlist) {
 		sym = kallsyms_lookup((unsigned long)p->addr, NULL,
-					&offset, &modname, namebuf);
+					&offset, &modname, namebuf, KSYM_NAME_LEN);
 		if (kprobe_aggrprobe(p)) {
 			list_for_each_entry_rcu(kp, &p->list, list)
 				report_probe(pi, kp, sym, offset, modname, p);
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 81e87280513e..c74bbf90fdfb 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -659,9 +659,9 @@ static const char *usage_str[] =
 };
 #endif
 
-const char *__get_key_name(const struct lockdep_subclass_key *key, char *str)
+const char *__get_key_name(const struct lockdep_subclass_key *key, char *str, size_t size)
 {
-	return kallsyms_lookup((unsigned long)key, NULL, NULL, NULL, str);
+	return kallsyms_lookup((unsigned long)key, NULL, NULL, NULL, str, size);
 }
 
 static inline unsigned long lock_flag(enum lock_usage_bit bit)
@@ -715,7 +715,7 @@ static void __print_lock_name(struct lock_class *class)
 
 	name = class->name;
 	if (!name) {
-		name = __get_key_name(class->key, str);
+		name = __get_key_name(class->key, str, KSYM_NAME_LEN);
 		printk(KERN_CONT "%s", name);
 	} else {
 		printk(KERN_CONT "%s", name);
@@ -746,7 +746,7 @@ static void print_lockdep_cache(struct lockdep_map *lock)
 
 	name = lock->name;
 	if (!name)
-		name = __get_key_name(lock->key->subkeys, str);
+		name = __get_key_name(lock->key->subkeys, str, KSYM_NAME_LEN);
 
 	printk(KERN_CONT "%s", name);
 }
diff --git a/kernel/locking/lockdep_internals.h b/kernel/locking/lockdep_internals.h
index bbe9000260d0..ab32ee6a0c87 100644
--- a/kernel/locking/lockdep_internals.h
+++ b/kernel/locking/lockdep_internals.h
@@ -129,7 +129,7 @@ extern void get_usage_chars(struct lock_class *class,
 			    char usage[LOCK_USAGE_CHARS]);
 
 extern const char *__get_key_name(const struct lockdep_subclass_key *key,
-				  char *str);
+				  char *str, size_t size);
 
 struct lock_class *lock_chain_get_class(struct lock_chain *chain, int i);
 
diff --git a/kernel/locking/lockdep_proc.c b/kernel/locking/lockdep_proc.c
index 15fdc7fa5c68..bf4ca5ec109e 100644
--- a/kernel/locking/lockdep_proc.c
+++ b/kernel/locking/lockdep_proc.c
@@ -63,7 +63,7 @@ static void print_name(struct seq_file *m, struct lock_class *class)
 	const char *name = class->name;
 
 	if (!name) {
-		name = __get_key_name(class->key, str);
+		name = __get_key_name(class->key, str, KSYM_NAME_LEN);
 		seq_printf(m, "%s", name);
 	} else{
 		seq_printf(m, "%s", name);
@@ -485,7 +485,7 @@ static void seq_stats(struct seq_file *m, struct lock_stat_data *data)
 		char str[KSYM_NAME_LEN];
 		const char *key_name;
 
-		key_name = __get_key_name(ckey, str);
+		key_name = __get_key_name(ckey, str, KSYM_NAME_LEN);
 		snprintf(name, namelen, "%s", key_name);
 	} else {
 		snprintf(name, namelen, "%s", cname);
diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index 3e11523bc6f6..c982860405c6 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -320,7 +320,7 @@ const char *module_address_lookup(unsigned long addr,
 			    unsigned long *offset,
 			    char **modname,
 			    const unsigned char **modbuildid,
-			    char *namebuf)
+			    char *namebuf, size_t buf_size)
 {
 	const char *ret = NULL;
 	struct module *mod;
@@ -342,7 +342,7 @@ const char *module_address_lookup(unsigned long addr,
 	}
 	/* Make a copy in here where it's safe */
 	if (ret) {
-		strncpy(namebuf, ret, KSYM_NAME_LEN - 1);
+		strncpy(namebuf, ret, buf_size - 1);
 		ret = namebuf;
 	}
 	preempt_enable();
@@ -350,7 +350,7 @@ const char *module_address_lookup(unsigned long addr,
 	return ret;
 }
 
-int lookup_module_symbol_name(unsigned long addr, char *symname)
+int lookup_module_symbol_name(unsigned long addr, char *symname, size_t size)
 {
 	struct module *mod;
 
@@ -365,7 +365,7 @@ int lookup_module_symbol_name(unsigned long addr, char *symname)
 			if (!sym)
 				goto out;
 
-			strscpy(symname, sym, KSYM_NAME_LEN);
+			strscpy(symname, sym, size);
 			preempt_enable();
 			return 0;
 		}
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index c12bcd26cb17..4d9a8621eaac 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -520,7 +520,7 @@ static int function_stat_show(struct seq_file *m, void *v)
 		goto out;
 #endif
 
-	kallsyms_lookup(rec->ip, NULL, NULL, NULL, str);
+	kallsyms_lookup(rec->ip, NULL, NULL, NULL, str, KSYM_SYMBOL_LEN);
 	seq_printf(m, "  %-30.30s  %10lu", str, rec->counter);
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
@@ -3980,7 +3980,7 @@ ftrace_match_record(struct dyn_ftrace *rec, struct ftrace_glob *func_g,
 	char str[KSYM_SYMBOL_LEN];
 	char *modname;
 
-	kallsyms_lookup(rec->ip, NULL, NULL, &modname, str);
+	kallsyms_lookup(rec->ip, NULL, NULL, &modname, str, KSYM_SYMBOL_LEN);
 
 	if (mod_g) {
 		int mod_matches = (modname) ? ftrace_match(modname, mod_g) : 0;
@@ -4738,7 +4738,7 @@ unregister_ftrace_function_probe_func(char *glob, struct trace_array *tr,
 
 			if (func_g.search) {
 				kallsyms_lookup(entry->ip, NULL, NULL,
-						NULL, str);
+						NULL, str, KSYM_SYMBOL_LEN);
 				if (!ftrace_match(str, &func_g))
 					continue;
 			}
@@ -6846,7 +6846,8 @@ static void save_ftrace_mod_rec(struct ftrace_mod_map *mod_map,
 	char *modname;
 	const char *ret;
 
-	ret = kallsyms_lookup(rec->ip, &symsize, &offset, &modname, str);
+	ret = kallsyms_lookup(rec->ip, &symsize, &offset, &modname,
+				str, KSYM_SYMBOL_LEN);
 	if (!ret)
 		return;
 
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 47cebef78532..8a1d2a0dc2dc 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -456,7 +456,7 @@ static bool within_notrace_func(struct trace_kprobe *tk)
 		return false;
 
 	/* Check if the address is on a suffixed-symbol */
-	if (!lookup_symbol_name(addr, symname)) {
+	if (!lookup_symbol_name(addr, symname, KSYM_NAME_LEN)) {
 		p = strchr(symname, '.');
 		if (!p)
 			return true;
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 2a6ec049cab5..0d3e1c9b59fb 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -364,7 +364,7 @@ trace_seq_print_sym(struct trace_seq *s, unsigned long address, bool offset)
 	if (offset)
 		sprint_symbol(str, KSYM_SYMBOL_LEN, address);
 	else
-		kallsyms_lookup(address, NULL, NULL, NULL, str);
+		kallsyms_lookup(address, NULL, NULL, NULL, str, KSYM_SYMBOL_LEN);
 	name = kretprobed(str, address);
 
 	if (name && strlen(name)) {
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index f755bde42fd0..3a67877ce658 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -89,7 +89,7 @@ find_syscall_meta(unsigned long syscall)
 
 	start = __start_syscalls_metadata;
 	stop = __stop_syscalls_metadata;
-	kallsyms_lookup(syscall, NULL, NULL, NULL, str);
+	kallsyms_lookup(syscall, NULL, NULL, NULL, str, KSYM_SYMBOL_LEN);
 
 	if (arch_syscall_match_sym_name(str, "sys_ni_syscall"))
 		return NULL;
-- 
2.17.1

[PATCH 5/5] kallsyms: remove unsed API lookup_symbol_attrs

[Maninder Singh]

with commit '7878c231dae0 ("slab: remove /proc/slab_allocators")'
lookup_symbol_attrs usage is removed.

Thus removing redundant API.

Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
---
 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, &modules, 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);
-			if (name)
-				strscpy(name, sym, KSYM_NAME_LEN);
-			preempt_enable();
-			return 0;
-		}
-	}
-out:
-	preempt_enable();
-	return -ERANGE;
-}
-
 int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
 		       char *name, char *module_name, int *exported)
 {
-- 
2.17.1

Re: [PATCH 0/5] kallsyms: make kallsym APIs more safe with scnprintf

[Christoph Hellwig]

On Fri, May 20, 2022 at 02:06:56PM +0530, Maninder Singh wrote:
> 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.

This is still a pretty horrible API.  Passing something like
a struct seq_buf seems like the much better API here.  Also with
the amount of arguments and by reference passing it might be worth
to pass them as a structure while you're at it.

Re: [PATCH 0/5] kallsyms: make kallsym APIs more safe with scnprintf

[Kees Cook]

On Sat, May 21, 2022 at 11:07:52PM -0700, Christoph Hellwig wrote:
> On Fri, May 20, 2022 at 02:06:56PM +0530, Maninder Singh wrote:
> > 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.
> 
> This is still a pretty horrible API.  Passing something like
> a struct seq_buf seems like the much better API here.  Also with
> the amount of arguments and by reference passing it might be worth
> to pass them as a structure while you're at it.

Yeah, I agree. It really seems like seq_buf would be nicer.

-- 
Kees Cook

Re: [PATCH 0/5] kallsyms: make kallsym APIs more safe with scnprintf

[Petr Mladek]

On Mon 2022-05-23 12:39:12, Kees Cook wrote:
> On Sat, May 21, 2022 at 11:07:52PM -0700, Christoph Hellwig wrote:
> > On Fri, May 20, 2022 at 02:06:56PM +0530, Maninder Singh wrote:
> > > 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.
> > 
> > This is still a pretty horrible API.  Passing something like
> > a struct seq_buf seems like the much better API here.  Also with
> > the amount of arguments and by reference passing it might be worth
> > to pass them as a structure while you're at it.
> 
> Yeah, I agree. It really seems like seq_buf would be nicer.

There is a new patchset that is trying to use this kind of buffer
in vsprintf.

It introduces another buffer struct because vsprintf() needs a bit
different semantic than the one used in seq_buf. But it actually
replaces seq_buf() in the end. I am not sure if this is the right
approach.

Anyway, the initial API is very simple, see
https://lore.kernel.org/r/20220604193042.1674951-2-kent.overstreet@gmail.com

And it makes the internal vsprintf() API more sane, see
https://lore.kernel.org/r/20220604193042.1674951-4-kent.overstreet@gmail.com

It would eventually solve also concerns about the kallsysms API.
Any comments on the new printbuf API are much appreaciated.

Best Regards,
Petr