objtool: check: give big enough buffer for pv_ops
PATCH 1/1 objtool: check: give big enough buffer for pv_ops
On gcc-12 build fails flagging possible buffer overflow:
check.c: In function 'validate_call':
check.c:2865:58: error: '%d' directive output may be truncated writing between 1 and 10 bytes into a region of size 9 [-Werror=format-truncation=]
2865 | snprintf(pvname, sizeof(pvname), "pv_ops[%d]", idx);
| ^~
I think it's a valid warning:
static char pvname[16];
int idx;
...
idx = (rel->addend / sizeof(void *));
snprintf(pvname, sizeof(pvname), "pv_ops[%d]", idx);we have only 7 chars for %d while it could take up to 9.
--- tools/objtool/check.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/objtool/check.c +1 -1
@@ -2852,7 +2852,7 @@ static inline bool func_uaccess_safe(struct symbol *func)static inline const char *call_dest_name (struct instruction *insn) { -static char pvname[16 ]; +static char pvname[32 ];struct reloc *rel;int idx;
Signature · 1 line
2.34.1
Peter Zijlstra
maintainerQuoted 9 lines · Sergei Trofimovich
On gcc-12 build fails flagging possible buffer overflow:
check.c: In function 'validate_call':
check.c:2865:58: error: '%d' directive output may be truncated writing between 1 and 10 bytes into a region of size 9 [-Werror=format-truncation=]
2865 |snprintf(pvname, sizeof(pvname), "pv_ops[%d]", idx);
| ^~I think it's a valid warning:
Quoted 7 lines
static char pvname[16]; int idx; ... idx = (rel->addend / sizeof(void *)); snprintf(pvname, sizeof(pvname), "pv_ops[%d]", idx);we have only 7 chars for %d while it could take up to 9.
Right, very unlikely to have that many pv_ops, but it doesn't hurt to fix this.
Thanks!
Quoted 16 lines · Peter Zijlstra
Quoted 15 lines · Sergei Trofimovich
On gcc-12 build fails flagging possible buffer overflow:
check.c: In function 'validate_call':
check.c:2865:58: error: '%d' directive output may be truncated writing between 1 and 10 bytes into a region of size 9 [-Werror=format-truncation=]
2865 |snprintf(pvname, sizeof(pvname), "pv_ops[%d]", idx);
| ^~I think it's a valid warning:
static char pvname[16]; int idx; ... idx = (rel->addend / sizeof(void *)); snprintf(pvname, sizeof(pvname), "pv_ops[%d]", idx);
Quoted 6 lines
Quoted 1 line
we have only 7 chars for %d while it could take up to 9.
Right, very unlikely to have that many
pv_ops, but it doesn't hurt to fix this.Thanks!
Alternatively, 'idx' could just be unsigned char, since pv_ops only has about ~80 entries max, but either way works for me. I'll queue it up.
Signature · 1 line
Josh
Quoted 19 lines · Josh Poimboeuf
Quoted 18 lines · Peter Zijlstra
Quoted 16 lines · Sergei Trofimovich
On gcc-12 build fails flagging possible buffer overflow:
check.c: In function 'validate_call':
check.c:2865:58: error: '%d' directive output may be truncated writing between 1 and 10 bytes into a region of size 9 [-Werror=format-truncation=]
2865 |snprintf(pvname, sizeof(pvname), "pv_ops[%d]", idx);
| ^~I think it's a valid warning:
static char pvname[16]; int idx; ... idx = (rel->addend / sizeof(void *)); snprintf(pvname, sizeof(pvname), "pv_ops[%d]", idx);we have only 7 chars for %d while it could take up to 9.
Quoted 7 lines
Quoted 4 lines
Right, very unlikely to have that many
pv_ops, but it doesn't hurt to fix this.Thanks!
Alternatively, 'idx' could just be unsigned char, since
pv_opsonly has about ~80 entries max, but either way works for me. I'll queue it up.
Sergei, can you send a v2 with a valid Signed-off-by tag?
Signature · 1 line
Josh
PATCH v2 1/1 objtool: check: give big enough buffer for pv_ops
On gcc-12 build fails flagging possible buffer overflow:
check.c: In function 'validate_call':
check.c:2865:58: error: '%d' directive output may be truncated writing between 1 and 10 bytes into a region of size 9 [-Werror=format-truncation=]
2865 | snprintf(pvname, sizeof(pvname), "pv_ops[%d]", idx);
| ^~
I think it's a valid warning:
static char pvname[16];
int idx;
...
idx = (rel->addend / sizeof(void *));
snprintf(pvname, sizeof(pvname), "pv_ops[%d]", idx);we have only 7 chars for %d while it could take up to 9.
---
Change since v1: added missing S-O-B.
tools/objtool/check.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/objtool/check.c +1 -1
@@ -2854,7 +2854,7 @@ static inline bool func_uaccess_safe(struct symbol *func)static inline const char *call_dest_name (struct instruction *insn) { -static char pvname[16 ]; +static char pvname[32 ];struct reloc *rel;int idx;
Signature · 1 line
2.34.1
[tip: objtool/urgent] objtool: Fix truncated string warning
The following commit has been merged into the objtool/urgent branch of tip:
The following commit has been merged into the objtool/urgent branch of tip:
Commit-ID: 82880283d7fcd0a1d20964a56d6d1a5cc0df0713 Gitweb: https://git.kernel.org/tip/82880283d7fcd0a1d20964a56d6d1a5cc0df0713 Author: Sergei Trofimovich <[email protected]> AuthorDate: Thu, 20 Jan 2022 23:37:48 Committer: Josh Poimboeuf <[email protected]> CommitterDate: Mon, 24 Jan 2022 10:09:06 -08:00
objtool: Fix truncated string warning
On GCC 12, the build fails due to a possible truncated string:
check.c: In function 'validate_call':
check.c:2865:58: error: '%d' directive output may be truncated writing between 1 and 10 bytes into a region of size 9 [-Werror=format-truncation=]
2865 | snprintf(pvname, sizeof(pvname), "pv_ops[%d]", idx);
| ^~
In theory it's a valid bug:
static char pvname[16];
int idx;
...
idx = (rel->addend / sizeof(void *));
snprintf(pvname, sizeof(pvname), "pv_ops[%d]", idx);There are only 7 chars for %d while it could take up to 9, so the printed "pv_ops[%d]" string could get truncated.
In reality the bug should never happen, because pv_ops only has ~80 entries, so 7 chars for the integer is more than enough. Still, it's worth fixing. Bump the buffer size by 2 bytes to silence the warning.
[ jpoimboe: changed size to 19; massaged changelog ]
--- tools/objtool/check.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/objtool/check.c +1 -1
@@ -2854,7 +2854,7 @@ static inline bool func_uaccess_safe(struct symbol *func)static inline const char *call_dest_name (struct instruction *insn) { -static char pvname[16 ]; +static char pvname[19 ];struct reloc *rel;int idx;