Skip to content

Commit 7e1f4eb

Browse files
arndbarndb
committed
kallsyms: rework symbol lookup return codes
Building with W=1 in some configurations produces a false positive warning for kallsyms: kernel/kallsyms.c: In function '__sprint_symbol.isra': kernel/kallsyms.c:503:17: error: 'strcpy' source argument is the same as destination [-Werror=restrict] 503 | strcpy(buffer, name); | ^~~~~~~~~~~~~~~~~~~~ This originally showed up while building with -O3, but later started happening in other configurations as well, depending on inlining decisions. The underlying issue is that the local 'name' variable is always initialized to the be the same as 'buffer' in the called functions that fill the buffer, which gcc notices while inlining, though it could see that the address check always skips the copy. The calling conventions here are rather unusual, as all of the internal lookup functions (bpf_address_lookup, ftrace_mod_address_lookup, ftrace_func_address_lookup, module_address_lookup and kallsyms_lookup_buildid) already use the provided buffer and either return the address of that buffer to indicate success, or NULL for failure, but the callers are written to also expect an arbitrary other buffer to be returned. Rework the calling conventions to return the length of the filled buffer instead of its address, which is simpler and easier to follow as well as avoiding the warning. Leave only the kallsyms_lookup() calling conventions unchanged, since that is called from 16 different functions and adapting this would be a much bigger change. Link: https://lore.kernel.org/lkml/[email protected]/ Link: https://lore.kernel.org/lkml/[email protected]/ Tested-by: Geert Uytterhoeven <[email protected]> Reviewed-by: Luis Chamberlain <[email protected]> Acked-by: Steven Rostedt (Google) <[email protected]> Signed-off-by: Arnd Bergmann <[email protected]>
1 parent 0fa8ab5 commit 7e1f4eb

File tree

7 files changed

+49
-53
lines changed

7 files changed

+49
-53
lines changed

‎include/linux/filter.h

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1208,18 +1208,18 @@ static inline bool bpf_jit_kallsyms_enabled(void)
12081208
return false;
12091209
}
12101210

1211-
const char *__bpf_address_lookup(unsigned long addr, unsigned long *size,
1211+
int __bpf_address_lookup(unsigned long addr, unsigned long *size,
12121212
unsigned long *off, char *sym);
12131213
bool is_bpf_text_address(unsigned long addr);
12141214
int bpf_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
12151215
char *sym);
12161216
struct bpf_prog *bpf_prog_ksym_find(unsigned long addr);
12171217

1218-
static inline const char *
1218+
static inline int
12191219
bpf_address_lookup(unsigned long addr, unsigned long *size,
12201220
unsigned long *off, char **modname, char *sym)
12211221
{
1222-
const char *ret = __bpf_address_lookup(addr, size, off, sym);
1222+
int ret = __bpf_address_lookup(addr, size, off, sym);
12231223

12241224
if (ret && modname)
12251225
*modname = NULL;
@@ -1263,11 +1263,11 @@ static inline bool bpf_jit_kallsyms_enabled(void)
12631263
return false;
12641264
}
12651265

1266-
static inline const char *
1266+
static inline int
12671267
__bpf_address_lookup(unsigned long addr, unsigned long *size,
12681268
unsigned long *off, char *sym)
12691269
{
1270-
return NULL;
1270+
return 0;
12711271
}
12721272

12731273
static inline bool is_bpf_text_address(unsigned long addr)
@@ -1286,11 +1286,11 @@ static inline struct bpf_prog *bpf_prog_ksym_find(unsigned long addr)
12861286
return NULL;
12871287
}
12881288

1289-
static inline const char *
1289+
static inline int
12901290
bpf_address_lookup(unsigned long addr, unsigned long *size,
12911291
unsigned long *off, char **modname, char *sym)
12921292
{
1293-
return NULL;
1293+
return 0;
12941294
}
12951295

12961296
static inline void bpf_prog_kallsyms_add(struct bpf_prog *fp)

‎include/linux/ftrace.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,15 +86,15 @@ struct ftrace_hash;
8686

8787
#if defined(CONFIG_FUNCTION_TRACER) && defined(CONFIG_MODULES) && \
8888
defined(CONFIG_DYNAMIC_FTRACE)
89-
const char *
89+
int
9090
ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
9191
unsigned long *off, char **modname, char *sym);
9292
#else
93-
static inline const char *
93+
static inline int
9494
ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
9595
unsigned long *off, char **modname, char *sym)
9696
{
97-
return NULL;
97+
return 0;
9898
}
9999
#endif
100100

‎include/linux/module.h

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -931,11 +931,11 @@ int module_kallsyms_on_each_symbol(const char *modname,
931931
* least KSYM_NAME_LEN long: a pointer to namebuf is returned if
932932
* found, otherwise NULL.
933933
*/
934-
const char *module_address_lookup(unsigned long addr,
935-
unsigned long *symbolsize,
936-
unsigned long *offset,
937-
char **modname, const unsigned char **modbuildid,
938-
char *namebuf);
934+
int module_address_lookup(unsigned long addr,
935+
unsigned long *symbolsize,
936+
unsigned long *offset,
937+
char **modname, const unsigned char **modbuildid,
938+
char *namebuf);
939939
int lookup_module_symbol_name(unsigned long addr, char *symname);
940940
int lookup_module_symbol_attrs(unsigned long addr,
941941
unsigned long *size,
@@ -964,14 +964,14 @@ static inline int module_kallsyms_on_each_symbol(const char *modname,
964964
}
965965

966966
/* For kallsyms to ask for address resolution. NULL means not found. */
967-
static inline const char *module_address_lookup(unsigned long addr,
967+
static inline int module_address_lookup(unsigned long addr,
968968
unsigned long *symbolsize,
969969
unsigned long *offset,
970970
char **modname,
971971
const unsigned char **modbuildid,
972972
char *namebuf)
973973
{
974-
return NULL;
974+
return 0;
975975
}
976976

977977
static inline int lookup_module_symbol_name(unsigned long addr, char *symname)

‎kernel/bpf/core.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -736,21 +736,20 @@ static struct bpf_ksym *bpf_ksym_find(unsigned long addr)
736736
return n ? container_of(n, struct bpf_ksym, tnode) : NULL;
737737
}
738738

739-
const char *__bpf_address_lookup(unsigned long addr, unsigned long *size,
739+
int __bpf_address_lookup(unsigned long addr, unsigned long *size,
740740
unsigned long *off, char *sym)
741741
{
742742
struct bpf_ksym *ksym;
743-
char *ret = NULL;
743+
int ret = 0;
744744

745745
rcu_read_lock();
746746
ksym = bpf_ksym_find(addr);
747747
if (ksym) {
748748
unsigned long symbol_start = ksym->start;
749749
unsigned long symbol_end = ksym->end;
750750

751-
strscpy(sym, ksym->name, KSYM_NAME_LEN);
751+
ret = strscpy(sym, ksym->name, KSYM_NAME_LEN);
752752

753-
ret = sym;
754753
if (size)
755754
*size = symbol_end - symbol_start;
756755
if (off)

‎kernel/kallsyms.c

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -388,12 +388,12 @@ int kallsyms_lookup_size_offset(unsigned long addr, unsigned long *symbolsize,
388388
!!__bpf_address_lookup(addr, symbolsize, offset, namebuf);
389389
}
390390

391-
static const char *kallsyms_lookup_buildid(unsigned long addr,
391+
static int kallsyms_lookup_buildid(unsigned long addr,
392392
unsigned long *symbolsize,
393393
unsigned long *offset, char **modname,
394394
const unsigned char **modbuildid, char *namebuf)
395395
{
396-
const char *ret;
396+
int ret;
397397

398398
namebuf[KSYM_NAME_LEN - 1] = 0;
399399
namebuf[0] = 0;
@@ -410,7 +410,7 @@ static const char *kallsyms_lookup_buildid(unsigned long addr,
410410
if (modbuildid)
411411
*modbuildid = NULL;
412412

413-
ret = namebuf;
413+
ret = strlen(namebuf);
414414
goto found;
415415
}
416416

@@ -442,8 +442,13 @@ const char *kallsyms_lookup(unsigned long addr,
442442
unsigned long *offset,
443443
char **modname, char *namebuf)
444444
{
445-
return kallsyms_lookup_buildid(addr, symbolsize, offset, modname,
446-
NULL, namebuf);
445+
int ret = kallsyms_lookup_buildid(addr, symbolsize, offset, modname,
446+
NULL, namebuf);
447+
448+
if (!ret)
449+
return NULL;
450+
451+
return namebuf;
447452
}
448453

449454
int lookup_symbol_name(unsigned long addr, char *symname)
@@ -478,19 +483,15 @@ static int __sprint_symbol(char *buffer, unsigned long address,
478483
{
479484
char *modname;
480485
const unsigned char *buildid;
481-
const char *name;
482486
unsigned long offset, size;
483487
int len;
484488

485489
address += symbol_offset;
486-
name = kallsyms_lookup_buildid(address, &size, &offset, &modname, &buildid,
490+
len = kallsyms_lookup_buildid(address, &size, &offset, &modname, &buildid,
487491
buffer);
488-
if (!name)
492+
if (!len)
489493
return sprintf(buffer, "0x%lx", address - symbol_offset);
490494

491-
if (name != buffer)
492-
strcpy(buffer, name);
493-
len = strlen(buffer);
494495
offset -= symbol_offset;
495496

496497
if (add_offset)

‎kernel/module/kallsyms.c

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -321,14 +321,15 @@ void * __weak dereference_module_function_descriptor(struct module *mod,
321321
* For kallsyms to ask for address resolution. NULL means not found. Careful
322322
* not to lock to avoid deadlock on oopses, simply disable preemption.
323323
*/
324-
const char *module_address_lookup(unsigned long addr,
325-
unsigned long *size,
326-
unsigned long *offset,
327-
char **modname,
328-
const unsigned char **modbuildid,
329-
char *namebuf)
324+
int module_address_lookup(unsigned long addr,
325+
unsigned long *size,
326+
unsigned long *offset,
327+
char **modname,
328+
const unsigned char **modbuildid,
329+
char *namebuf)
330330
{
331-
const char *ret = NULL;
331+
const char *sym;
332+
int ret = 0;
332333
struct module *mod;
333334

334335
preempt_disable();
@@ -344,12 +345,10 @@ const char *module_address_lookup(unsigned long addr,
344345
#endif
345346
}
346347

347-
ret = find_kallsyms_symbol(mod, addr, size, offset);
348-
}
349-
/* Make a copy in here where it's safe */
350-
if (ret) {
351-
strscpy(namebuf, ret, KSYM_NAME_LEN);
352-
ret = namebuf;
348+
sym = find_kallsyms_symbol(mod, addr, size, offset);
349+
350+
if (sym)
351+
ret = strscpy(namebuf, sym, KSYM_NAME_LEN);
353352
}
354353
preempt_enable();
355354

‎kernel/trace/ftrace.c

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6969,7 +6969,7 @@ allocate_ftrace_mod_map(struct module *mod,
69696969
return mod_map;
69706970
}
69716971

6972-
static const char *
6972+
static int
69736973
ftrace_func_address_lookup(struct ftrace_mod_map *mod_map,
69746974
unsigned long addr, unsigned long *size,
69756975
unsigned long *off, char *sym)
@@ -6990,21 +6990,18 @@ ftrace_func_address_lookup(struct ftrace_mod_map *mod_map,
69906990
*size = found_func->size;
69916991
if (off)
69926992
*off = addr - found_func->ip;
6993-
if (sym)
6994-
strscpy(sym, found_func->name, KSYM_NAME_LEN);
6995-
6996-
return found_func->name;
6993+
return strscpy(sym, found_func->name, KSYM_NAME_LEN);
69976994
}
69986995

6999-
return NULL;
6996+
return 0;
70006997
}
70016998

7002-
const char *
6999+
int
70037000
ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
70047001
unsigned long *off, char **modname, char *sym)
70057002
{
70067003
struct ftrace_mod_map *mod_map;
7007-
const char *ret = NULL;
7004+
int ret = 0;
70087005

70097006
/* mod_map is freed via call_rcu() */
70107007
preempt_disable();

0 commit comments

Comments
 (0)