[Mesa-dev] [PATCH v2 2/3] gallium/hud: Prevent buffer overflow in hud_thread_counter_install

Robert Foss robert.foss at collabora.com
Thu Jun 29 13:53:22 UTC 2017


Hey Brian,

On Thu, 2017-06-29 at 07:28 -0600, Brian Paul wrote:
> On 06/29/2017 07:21 AM, Robert Foss wrote:
> > Switch to using strncopy to avoid potential overflow of
> > name array in struct hud_graph.
> > 
> > Coverity-id: 1413761
> > 
> > Signed-off-by: Robert Foss <robert.foss at collabora.com>
> > ---
> >   src/gallium/auxiliary/hud/hud_cpu.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/gallium/auxiliary/hud/hud_cpu.c
> > b/src/gallium/auxiliary/hud/hud_cpu.c
> > index 4caaab6977..468c36207b 100644
> > --- a/src/gallium/auxiliary/hud/hud_cpu.c
> > +++ b/src/gallium/auxiliary/hud/hud_cpu.c
> > @@ -362,7 +362,7 @@ void hud_thread_counter_install(struct hud_pane
> > *pane, const char *name,
> >      if (!gr)
> >         return;
> > 
> > -   strcpy(gr->name, name);
> > +   strncpy(gr->name, name, HUD_GRAPH_NAME_LEN);
> 
> strncpy() doesn't null terminate the destination if strlen(name) >= 
> HUD_GRAPH_NAME_LEN
> 
> If you're concerned with overflow, you need to address that too.
> 
> You might looks if we have a gallium util function for strncpy with
> null 
> termination.

Ack, I had a look in u_string.h, and util_strncmp does not exists, so
I'll add it and use it in v3.
> 
> 
> Also, the change in patch 3/3 won't compile.  You didn't change the 
> function name.
> 
> > 
> >      gr->query_data = CALLOC_STRUCT(counter_info);
> >      if (!gr->query_data) {
> > 
> 
> AFAIC, you could combine all three patches.  It's a simple change.

Do you mind if I keep the strcpy changes in separate commits since they
apply to two separate coverity issues?



More information about the mesa-dev mailing list