Message ID | 20200123070533.19699-1-thuth@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | target/s390x/translate: Do not leak stack address in translate_one() | expand |
> Am 23.01.2020 um 08:05 schrieb Thomas Huth <thuth@redhat.com>: > > The code in translate_one() leaks a stack address via "s->field" parameter: > > static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s) > { > DisasJumpType ret = DISAS_NEXT; > DisasFields f; > [...] > s->fields = &f; > [...] > return ret; > } > > It's currently harmless since the caller does not seem to use "fields" > anymore, but let's better play safe (and please static code analyzers) > by setting the fields back to NULL before returning. > > Buglink: https://bugs.launchpad.net/qemu/+bug/1661815 > Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: David Hildenbrand <david@redhat.com> > --- > target/s390x/translate.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/target/s390x/translate.c b/target/s390x/translate.c > index 4292bb0dd0..9122fb36da 100644 > --- a/target/s390x/translate.c > +++ b/target/s390x/translate.c > @@ -6435,6 +6435,8 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s) > } > #endif > > + s->fields = NULL; > + > /* Advance to the next instruction. */ > s->base.pc_next = s->pc_tmp; > return ret; > -- > 2.18.1 >
On Thu, 23 Jan 2020 08:05:33 +0100 Thomas Huth <thuth@redhat.com> wrote: > The code in translate_one() leaks a stack address via "s->field" parameter: > > static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s) > { > DisasJumpType ret = DISAS_NEXT; > DisasFields f; > [...] > s->fields = &f; > [...] > return ret; > } > > It's currently harmless since the caller does not seem to use "fields" > anymore, but let's better play safe (and please static code analyzers) > by setting the fields back to NULL before returning. > > Buglink: https://bugs.launchpad.net/qemu/+bug/1661815 > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > target/s390x/translate.c | 2 ++ > 1 file changed, 2 insertions(+) Thanks, applied.
diff --git a/target/s390x/translate.c b/target/s390x/translate.c index 4292bb0dd0..9122fb36da 100644 --- a/target/s390x/translate.c +++ b/target/s390x/translate.c @@ -6435,6 +6435,8 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s) } #endif + s->fields = NULL; + /* Advance to the next instruction. */ s->base.pc_next = s->pc_tmp; return ret;
The code in translate_one() leaks a stack address via "s->field" parameter: static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s) { DisasJumpType ret = DISAS_NEXT; DisasFields f; [...] s->fields = &f; [...] return ret; } It's currently harmless since the caller does not seem to use "fields" anymore, but let's better play safe (and please static code analyzers) by setting the fields back to NULL before returning. Buglink: https://bugs.launchpad.net/qemu/+bug/1661815 Signed-off-by: Thomas Huth <thuth@redhat.com> --- target/s390x/translate.c | 2 ++ 1 file changed, 2 insertions(+)