All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] objtool: check: give big enough buffer for pv_ops
@ 2022-01-14  7:57 Sergei Trofimovich
  2022-01-14  8:53 ` Peter Zijlstra
  0 siblings, 1 reply; 6+ messages in thread
From: Sergei Trofimovich @ 2022-01-14  7:57 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: linux-kernel, Sergei Trofimovich, Josh Poimboeuf

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.

CC: Josh Poimboeuf <jpoimboe@redhat.com>
CC: Peter Zijlstra <peterz@infradead.org>
---
 tools/objtool/check.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 8c1931eab5f1..0fae132ea59f 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -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;
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] objtool: check: give big enough buffer for pv_ops
  2022-01-14  7:57 [PATCH] objtool: check: give big enough buffer for pv_ops Sergei Trofimovich
@ 2022-01-14  8:53 ` Peter Zijlstra
  2022-01-20 22:58   ` Josh Poimboeuf
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2022-01-14  8:53 UTC (permalink / raw)
  To: Sergei Trofimovich; +Cc: linux-kernel, Josh Poimboeuf

On Fri, Jan 14, 2022 at 07:57:56AM +0000, Sergei Trofimovich wrote:
> 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.

Right, very unlikely to have that many pv_ops, but it doesn't hurt to
fix this.

Thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] objtool: check: give big enough buffer for pv_ops
  2022-01-14  8:53 ` Peter Zijlstra
@ 2022-01-20 22:58   ` Josh Poimboeuf
  2022-01-20 23:09     ` Josh Poimboeuf
  0 siblings, 1 reply; 6+ messages in thread
From: Josh Poimboeuf @ 2022-01-20 22:58 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: Sergei Trofimovich, linux-kernel

On Fri, Jan 14, 2022 at 09:53:07AM +0100, Peter Zijlstra wrote:
> On Fri, Jan 14, 2022 at 07:57:56AM +0000, Sergei Trofimovich wrote:
> > 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.
> 
> 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.

-- 
Josh


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] objtool: check: give big enough buffer for pv_ops
  2022-01-20 22:58   ` Josh Poimboeuf
@ 2022-01-20 23:09     ` Josh Poimboeuf
  2022-01-20 23:37       ` [PATCH v2] " Sergei Trofimovich
  0 siblings, 1 reply; 6+ messages in thread
From: Josh Poimboeuf @ 2022-01-20 23:09 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: Sergei Trofimovich, linux-kernel

On Thu, Jan 20, 2022 at 02:58:13PM -0800, Josh Poimboeuf wrote:
> On Fri, Jan 14, 2022 at 09:53:07AM +0100, Peter Zijlstra wrote:
> > On Fri, Jan 14, 2022 at 07:57:56AM +0000, Sergei Trofimovich wrote:
> > > 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.
> > 
> > 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.

Sergei, can you send a v2 with a valid Signed-off-by tag?

-- 
Josh


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] objtool: check: give big enough buffer for pv_ops
  2022-01-20 23:09     ` Josh Poimboeuf
@ 2022-01-20 23:37       ` Sergei Trofimovich
  2022-01-31 12:04         ` [tip: objtool/urgent] objtool: Fix truncated string warning tip-bot2 for Sergei Trofimovich
  0 siblings, 1 reply; 6+ messages in thread
From: Sergei Trofimovich @ 2022-01-20 23:37 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: linux-kernel, Sergei Trofimovich, Peter Zijlstra

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.

CC: Josh Poimboeuf <jpoimboe@redhat.com>
CC: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Sergei Trofimovich <slyich@gmail.com>
---
Change since v1: added missing S-O-B.
 tools/objtool/check.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index c2d2ab9a2861..f5bed94e4558 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -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;
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [tip: objtool/urgent] objtool: Fix truncated string warning
  2022-01-20 23:37       ` [PATCH v2] " Sergei Trofimovich
@ 2022-01-31 12:04         ` tip-bot2 for Sergei Trofimovich
  0 siblings, 0 replies; 6+ messages in thread
From: tip-bot2 for Sergei Trofimovich @ 2022-01-31 12:04 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Adam Borowski, mliska, Sergei Trofimovich, Josh Poimboeuf, x86,
	linux-kernel

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 <slyich@gmail.com>
AuthorDate:    Thu, 20 Jan 2022 23:37:48 
Committer:     Josh Poimboeuf <jpoimboe@redhat.com>
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 ]

Fixes: db2b0c5d7b6f ("objtool: Support pv_opsindirect calls for noinstr")
Reported-by: Adam Borowski <kilobyte@angband.pl>
Reported-by: Martin Liška <mliska@suse.cz>
Signed-off-by: Sergei Trofimovich <slyich@gmail.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Link: https://lore.kernel.org/r/20220120233748.2062559-1-slyich@gmail.com
---
 tools/objtool/check.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index c2d2ab9..7c33ec6 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -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;
 

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-01-31 12:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-14  7:57 [PATCH] objtool: check: give big enough buffer for pv_ops Sergei Trofimovich
2022-01-14  8:53 ` Peter Zijlstra
2022-01-20 22:58   ` Josh Poimboeuf
2022-01-20 23:09     ` Josh Poimboeuf
2022-01-20 23:37       ` [PATCH v2] " Sergei Trofimovich
2022-01-31 12:04         ` [tip: objtool/urgent] objtool: Fix truncated string warning tip-bot2 for Sergei Trofimovich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.