[PATCH app/xrandr] init the name to 0
There are a few conditions where coverity finds a use of an uninitialized
field of the name_t struct. These are rather messy combinations of conditions,
so let's go with the simple solution here and just init everything to 0.
This may still have side-effects but at least they'll be more obvious than the
previous "use whatever memory is leftover from breakfast".
This patch also adds a missing init_name(), much for the same reason.
Signed-off-by: Peter Hutterer
---
xrandr.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/xrandr.c b/xrandr.c
index 7f1e867..ce3cd91 100644
--- a/xrandr.c
+++ b/xrandr.c
@@ -637,6 +637,7 @@ print_verbose_mode (const XRRModeInfo *mode, Bool current,
Bool preferred)
static void
init_name (name_t *name)
{
+memset(name, 0, sizeof(*name));
name->kind = name_none;
}
@@ -1822,6 +1823,7 @@ get_outputs (void)
output_t*output;
name_t output_name;
if (!output_info) fatal ("could not get output 0x%lx information\n",
res->outputs[o]);
+ init_name(_name);
set_name_xid (_name, res->outputs[o]);
set_name_index (_name, o);
set_name_string (_name, output_info->name);
--
2.17.1
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] devices: break after finding and removing device from lists
On Wed, 12 Sep 2018 at 23:39, Walter Harms wrote: > > > > > Dave Airlie hat am 12. September 2018 um 03:40 > > geschrieben: > > > > > > From: Dave Airlie > > > > Coverity complains about a use after free in here after the > > freeing, I can't follow the linked list so well, but whot > > says the device can only be on one list once, so break should > > fix it. > > > > Signed-off-by: Dave Airlie > > Is a list of Coverity complains some where online available ? I was working from an internal RH coverity process. I think I submitted public X server builds once or twice, but never got the process automated. Dave. ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH app/sessreg] Replace strncpy calls with a sane version that always terminates
On 09/12/18 12:09 AM, Walter Harms wrote:
Peter Hutterer hat am 12. September 2018 um 06:50
geschrieben:
Fixes coverity complaints about potentially unterminated strings
Signed-off-by: Peter Hutterer
---
sessreg.c | 26 +-
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/sessreg.c b/sessreg.c
index 0a8fdb2..53b30b0 100644
--- a/sessreg.c
+++ b/sessreg.c
@@ -192,6 +192,14 @@ sysnerr (int x, const char *s)
return x;
}
+static void
+safe_strncpy(char *dest, const char *src, size_t n)
+{
if you add
if (!src)
return;
you can also remove the if (x) before strncpy()
You'd only be able to remove if's from a couple calls so it doesn't seem
like it really buys you much.
I'd be tempted to handle this for non-glibc platforms with something like
#ifdef HAVE_STRLCPY
#define safe_strncpy strlcpy
#else
static void
safe_strncpy(char *dest, const char *src, size_t n)
{
...
}
#endif
which was going to be another argument against it, but I suppose that
could also just be handled via
#define safe_strncpy(d, s, n) do { \
if (s) { strlcpy(d, s, n); } \
while(0)
--
-Alan Coopersmith- alan.coopersm...@oracle.com
Oracle Solaris Engineering - https://blogs.oracle.com/alanc
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] devices: break after finding and removing device from lists
> Dave Airlie hat am 12. September 2018 um 03:40 > geschrieben: > > > From: Dave Airlie > > Coverity complains about a use after free in here after the > freeing, I can't follow the linked list so well, but whot > says the device can only be on one list once, so break should > fix it. > > Signed-off-by: Dave Airlie Is a list of Coverity complains some where online available ? re, wh > --- > dix/devices.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/dix/devices.c b/dix/devices.c > index 4a628afb0..1b18b168e 100644 > --- a/dix/devices.c > +++ b/dix/devices.c > @@ -1177,6 +1177,7 @@ RemoveDevice(DeviceIntPtr dev, BOOL sendevent) > flags[tmp->id] = IsMaster(tmp) ? XIMasterRemoved : > XISlaveRemoved; > CloseDevice(tmp); > ret = Success; > +break; > } > } > > @@ -1193,6 +1194,7 @@ RemoveDevice(DeviceIntPtr dev, BOOL sendevent) > prev->next = next; > > ret = Success; > +break; > } > } > > -- > 2.17.1 > > ___ > xorg-devel@lists.x.org: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: https://lists.x.org/mailman/listinfo/xorg-devel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH app/sessreg] Replace strncpy calls with a sane version that always terminates
On Wed, Sep 12, 2018 at 09:09:22AM +0200, Walter Harms wrote:
>
>
> > Peter Hutterer hat am 12. September 2018 um 06:50
> > geschrieben:
> >
> >
> > Fixes coverity complaints about potentially unterminated strings
> >
> > Signed-off-by: Peter Hutterer
> > ---
> > sessreg.c | 26 +-
> > 1 file changed, 17 insertions(+), 9 deletions(-)
> >
> > diff --git a/sessreg.c b/sessreg.c
> > index 0a8fdb2..53b30b0 100644
> > --- a/sessreg.c
> > +++ b/sessreg.c
> > @@ -192,6 +192,14 @@ sysnerr (int x, const char *s)
> > return x;
> > }
> >
> > +static void
> > +safe_strncpy(char *dest, const char *src, size_t n)
> > +{
>
> if you add
> if (!src)
> return;
>
> you can also remove the if (x) before strncpy()
tbh, I'd rather touch this code as little as possible. there's more cleanup
possible but...
Cheers,
Peter
>
> > +(void)strncpy(dest, src, n);
> > +if (n > 0)
> > +dest[n - 1] = '\0';
> > +}
> > +
>
>
> just my 2 cents
>
> re,
> wh
>
> > int
> > main (int argc, char **argv)
> > {
> > @@ -406,9 +414,9 @@ main (int argc, char **argv)
> > memset(, 0, sizeof(ll));
> > ll.ll_time = current_time;
> > if (line)
> > -(void) strncpy (ll.ll_line, line, sizeof (ll.ll_line));
> > +safe_strncpy (ll.ll_line, line, sizeof (ll.ll_line));
> > if (host_name)
> > -(void) strncpy (ll.ll_host, host_name, sizeof
> > (ll.ll_host));
> > +safe_strncpy (ll.ll_host, host_name, sizeof
> > (ll.ll_host));
> >
> > sysnerr (write (llog, (char *) , sizeof (ll))
> > == sizeof (ll), "write lastlog entry");
> > @@ -429,11 +437,11 @@ set_utmp (struct utmp *u, char *line, char *user, char
> > *host, time_t date, int a
> > {
> > memset (u, 0, sizeof (*u));
> > if (line)
> > - (void) strncpy (u->ut_line, line, sizeof (u->ut_line));
> > + safe_strncpy (u->ut_line, line, sizeof (u->ut_line));
> > else
> > memset (u->ut_line, 0, sizeof (u->ut_line));
> > if (addp && user)
> > - (void) strncpy (u->ut_name, user, sizeof (u->ut_name));
> > + safe_strncpy (u->ut_name, user, sizeof (u->ut_name));
> > else
> > memset (u->ut_name, 0, sizeof (u->ut_name));
> > #ifdef HAVE_STRUCT_UTMP_UT_ID
> > @@ -451,7 +459,7 @@ set_utmp (struct utmp *u, char *line, char *user, char
> > *host, time_t date, int a
> > i -= sizeof (u->ut_id);
> > else
> > i = 0;
> > - (void) strncpy (u->ut_id, line + i, sizeof (u->ut_id));
> > + safe_strncpy (u->ut_id, line + i, sizeof (u->ut_id));
> > } else
> > memset (u->ut_id, 0, sizeof (u->ut_id));
> > #endif
> > @@ -469,7 +477,7 @@ set_utmp (struct utmp *u, char *line, char *user, char
> > *host, time_t date, int a
> > #endif
> > #ifdef HAVE_STRUCT_UTMP_UT_HOST
> > if (addp && host)
> > - (void) strncpy (u->ut_host, host, sizeof (u->ut_host));
> > + safe_strncpy (u->ut_host, host, sizeof (u->ut_host));
> > else
> > memset (u->ut_host, 0, sizeof (u->ut_host));
> > #endif
> > @@ -513,7 +521,7 @@ set_utmpx (struct utmpx *u, const char *line, const char
> > *user,
> > if(strcmp(line, ":0") == 0)
> > (void) strcpy(u->ut_line, "console");
> > else
> > - (void) strncpy (u->ut_line, line, sizeof (u->ut_line));
> > + safe_strncpy (u->ut_line, line, sizeof (u->ut_line));
> >
> > strncpy(u->ut_host, line, sizeof(u->ut_host));
> > #ifdef HAVE_STRUCT_UTMPX_UT_SYSLEN
> > @@ -523,7 +531,7 @@ set_utmpx (struct utmpx *u, const char *line, const char
> > *user,
> > else
> > memset (u->ut_line, 0, sizeof (u->ut_line));
> > if (addp && user)
> > - (void) strncpy (u->ut_user, user, sizeof (u->ut_user));
> > + safe_strncpy (u->ut_user, user, sizeof (u->ut_user));
> > else
> > memset (u->ut_user, 0, sizeof (u->ut_user));
> >
> > @@ -541,7 +549,7 @@ set_utmpx (struct utmpx *u, const char *line, const char
> > *user,
> > i -= sizeof (u->ut_id);
> > else
> > i = 0;
> > - (void) strncpy (u->ut_id, line + i, sizeof (u->ut_id));
> > + safe_strncpy (u->ut_id, line + i, sizeof (u->ut_id));
> >
> > /* make sure there is no entry using identical ut_id */
> > if (!UtmpxIdOpen(u->ut_id) && addp) {
> > --
> > 2.17.1
> >
> > ___
> > xorg-devel@lists.x.org: X.Org development
> > Archives: http://lists.x.org/archives/xorg-devel
> > Info: https://lists.x.org/mailman/listinfo/xorg-devel
___
xorg-devel@lists.x.org: X.Org development
Re: [PATCH app/sessreg] Replace strncpy calls with a sane version that always terminates
> Peter Hutterer hat am 12. September 2018 um 06:50
> geschrieben:
>
>
> Fixes coverity complaints about potentially unterminated strings
>
> Signed-off-by: Peter Hutterer
> ---
> sessreg.c | 26 +-
> 1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/sessreg.c b/sessreg.c
> index 0a8fdb2..53b30b0 100644
> --- a/sessreg.c
> +++ b/sessreg.c
> @@ -192,6 +192,14 @@ sysnerr (int x, const char *s)
> return x;
> }
>
> +static void
> +safe_strncpy(char *dest, const char *src, size_t n)
> +{
if you add
if (!src)
return;
you can also remove the if (x) before strncpy()
> +(void)strncpy(dest, src, n);
> +if (n > 0)
> +dest[n - 1] = '\0';
> +}
> +
just my 2 cents
re,
wh
> int
> main (int argc, char **argv)
> {
> @@ -406,9 +414,9 @@ main (int argc, char **argv)
> memset(, 0, sizeof(ll));
> ll.ll_time = current_time;
> if (line)
> - (void) strncpy (ll.ll_line, line, sizeof (ll.ll_line));
> + safe_strncpy (ll.ll_line, line, sizeof (ll.ll_line));
> if (host_name)
> - (void) strncpy (ll.ll_host, host_name, sizeof
> (ll.ll_host));
> + safe_strncpy (ll.ll_host, host_name, sizeof
> (ll.ll_host));
>
> sysnerr (write (llog, (char *) , sizeof (ll))
> == sizeof (ll), "write lastlog entry");
> @@ -429,11 +437,11 @@ set_utmp (struct utmp *u, char *line, char *user, char
> *host, time_t date, int a
> {
> memset (u, 0, sizeof (*u));
> if (line)
> - (void) strncpy (u->ut_line, line, sizeof (u->ut_line));
> + safe_strncpy (u->ut_line, line, sizeof (u->ut_line));
> else
> memset (u->ut_line, 0, sizeof (u->ut_line));
> if (addp && user)
> - (void) strncpy (u->ut_name, user, sizeof (u->ut_name));
> + safe_strncpy (u->ut_name, user, sizeof (u->ut_name));
> else
> memset (u->ut_name, 0, sizeof (u->ut_name));
> #ifdef HAVE_STRUCT_UTMP_UT_ID
> @@ -451,7 +459,7 @@ set_utmp (struct utmp *u, char *line, char *user, char
> *host, time_t date, int a
> i -= sizeof (u->ut_id);
> else
> i = 0;
> - (void) strncpy (u->ut_id, line + i, sizeof (u->ut_id));
> + safe_strncpy (u->ut_id, line + i, sizeof (u->ut_id));
> } else
> memset (u->ut_id, 0, sizeof (u->ut_id));
> #endif
> @@ -469,7 +477,7 @@ set_utmp (struct utmp *u, char *line, char *user, char
> *host, time_t date, int a
> #endif
> #ifdef HAVE_STRUCT_UTMP_UT_HOST
> if (addp && host)
> - (void) strncpy (u->ut_host, host, sizeof (u->ut_host));
> + safe_strncpy (u->ut_host, host, sizeof (u->ut_host));
> else
> memset (u->ut_host, 0, sizeof (u->ut_host));
> #endif
> @@ -513,7 +521,7 @@ set_utmpx (struct utmpx *u, const char *line, const char
> *user,
> if(strcmp(line, ":0") == 0)
> (void) strcpy(u->ut_line, "console");
> else
> - (void) strncpy (u->ut_line, line, sizeof (u->ut_line));
> + safe_strncpy (u->ut_line, line, sizeof (u->ut_line));
>
> strncpy(u->ut_host, line, sizeof(u->ut_host));
> #ifdef HAVE_STRUCT_UTMPX_UT_SYSLEN
> @@ -523,7 +531,7 @@ set_utmpx (struct utmpx *u, const char *line, const char
> *user,
> else
> memset (u->ut_line, 0, sizeof (u->ut_line));
> if (addp && user)
> - (void) strncpy (u->ut_user, user, sizeof (u->ut_user));
> + safe_strncpy (u->ut_user, user, sizeof (u->ut_user));
> else
> memset (u->ut_user, 0, sizeof (u->ut_user));
>
> @@ -541,7 +549,7 @@ set_utmpx (struct utmpx *u, const char *line, const char
> *user,
> i -= sizeof (u->ut_id);
> else
> i = 0;
> - (void) strncpy (u->ut_id, line + i, sizeof (u->ut_id));
> + safe_strncpy (u->ut_id, line + i, sizeof (u->ut_id));
>
> /* make sure there is no entry using identical ut_id */
> if (!UtmpxIdOpen(u->ut_id) && addp) {
> --
> 2.17.1
>
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
