[libvirt] [PATCH 2/5] openvzGetProcessInfo: address clang-detected low-probability flaw
From: Jim Meyering * src/openvz/openvz_driver.c (openvzGetProcessInfo): Reorganize so that unexpected /proc/vz/vestat content cannot make us use uninitialized variables. Without this change, an input line with a matching "readvps", but fewer than 4 numbers would result in our using at least "systime" uninitialized. --- src/openvz/openvz_driver.c | 30 +++--- 1 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 95c4236..47004d6 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1384,14 +1384,15 @@ static int openvzGetProcessInfo(unsigned long long *cpuTime, int vpsid) { int fd; char line[1024] ; unsigned long long usertime, systime, nicetime; -int readvps = 0, ret; +int readvps = vpsid + 1; /* ensure readvps is initially different */ +int ret; /* read statistic from /proc/vz/vestat. sample: Version: 2.2 - VEID user nice system uptime idle other.. -33 78 0 1330 59454597 142650441835148 other.. -55 178 0 5340 59424597 542650441835148 other.. + VEID user nice system uptime idle other.. + 33 78 0 1330 59454597 142650441835148 other.. + 55 178 0 5340 59424597 542650441835148 other.. */ if ((fd = open("/proc/vz/vestat", O_RDONLY)) == -1) @@ -1400,15 +1401,18 @@ Version: 2.2 /*search line with VEID=vpsid*/ while(1) { ret = openvz_readline(fd, line, sizeof(line)); -if(ret <= 0) +if (ret <= 0) break; -if (sscanf(line, "%d %llu %llu %llu", - &readvps, &usertime, &nicetime, &systime) != 4) -continue; - -if (readvps == vpsid) -break; /*found vpsid*/ +if (sscanf (line, "%d %llu %llu %llu", +&readvps, &usertime, &nicetime, &systime) == 4 +&& readvps == vpsid) { /*found vpsid*/ +/* convert jiffies to nanoseconds */ +*cpuTime = (1000ull * 1000ull * 1000ull +* (usertime + nicetime + systime) +/ (unsigned long long)sysconf(_SC_CLK_TCK)); +break; +} } close(fd); @@ -1418,10 +1422,6 @@ Version: 2.2 if (readvps != vpsid) /*not found*/ return -1; -/* convert jiffies to nanoseconds */ -*cpuTime = 1000ull * 1000ull * 1000ull * (usertime + nicetime + systime) - / (unsigned long long)sysconf(_SC_CLK_TCK); - return 0; } -- 1.7.1.rc1.248.gcefbb -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] openvzGetProcessInfo: address clang-detected low-probability flaw
On 04/14/2010 02:46 AM, Jim Meyering wrote: > From: Jim Meyering > > * src/openvz/openvz_driver.c (openvzGetProcessInfo): Reorganize > so that unexpected /proc/vz/vestat content cannot make us use > uninitialized variables. Without this change, an input line with > a matching "readvps", but fewer than 4 numbers would result in our > using at least "systime" uninitialized. > --- > src/openvz/openvz_driver.c | 30 +++--- > 1 files changed, 15 insertions(+), 15 deletions(-) > > diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c > index 95c4236..47004d6 100644 > --- a/src/openvz/openvz_driver.c > +++ b/src/openvz/openvz_driver.c > @@ -1384,14 +1384,15 @@ static int openvzGetProcessInfo(unsigned long long > *cpuTime, int vpsid) { > int fd; > char line[1024] ; > unsigned long long usertime, systime, nicetime; > -int readvps = 0, ret; > +int readvps = vpsid + 1; /* ensure readvps is initially different */ > +int ret; > > -if (sscanf(line, "%d %llu %llu %llu", > - &readvps, &usertime, &nicetime, &systime) != 4) > -continue; > - > -if (readvps == vpsid) > -break; /*found vpsid*/ > +if (sscanf (line, "%d %llu %llu %llu", > +&readvps, &usertime, &nicetime, &systime) == 4 > +&& readvps == vpsid) { /*found vpsid*/ > +/* convert jiffies to nanoseconds */ > +*cpuTime = (1000ull * 1000ull * 1000ull > +* (usertime + nicetime + systime) > +/ (unsigned long long)sysconf(_SC_CLK_TCK)); > +break; > +} ACK that the rewrite fixes the problem. However, there's still the issue that we're using sscanf in the first place, instead of virStrToLong_ull; do you want to prepare a followup patch, or shall I? -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] openvzGetProcessInfo: address clang-detected low-probability flaw
Eric Blake wrote: > On 04/14/2010 02:46 AM, Jim Meyering wrote: >> From: Jim Meyering >> >> * src/openvz/openvz_driver.c (openvzGetProcessInfo): Reorganize >> so that unexpected /proc/vz/vestat content cannot make us use >> uninitialized variables. Without this change, an input line with >> a matching "readvps", but fewer than 4 numbers would result in our >> using at least "systime" uninitialized. >> --- >> src/openvz/openvz_driver.c | 30 +++--- >> 1 files changed, 15 insertions(+), 15 deletions(-) >> >> diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c >> index 95c4236..47004d6 100644 >> --- a/src/openvz/openvz_driver.c >> +++ b/src/openvz/openvz_driver.c >> @@ -1384,14 +1384,15 @@ static int openvzGetProcessInfo(unsigned long long >> *cpuTime, int vpsid) { >> int fd; >> char line[1024] ; >> unsigned long long usertime, systime, nicetime; >> -int readvps = 0, ret; >> +int readvps = vpsid + 1; /* ensure readvps is initially different */ >> +int ret; >> >> -if (sscanf(line, "%d %llu %llu %llu", >> - &readvps, &usertime, &nicetime, &systime) != 4) >> -continue; >> - >> -if (readvps == vpsid) >> -break; /*found vpsid*/ >> +if (sscanf (line, "%d %llu %llu %llu", >> +&readvps, &usertime, &nicetime, &systime) == 4 >> +&& readvps == vpsid) { /*found vpsid*/ >> +/* convert jiffies to nanoseconds */ >> +*cpuTime = (1000ull * 1000ull * 1000ull >> +* (usertime + nicetime + systime) >> +/ (unsigned long long)sysconf(_SC_CLK_TCK)); >> +break; >> +} > > ACK that the rewrite fixes the problem. However, there's still the Thanks for the review. > issue that we're using sscanf in the first place, instead of > virStrToLong_ull; do you want to prepare a followup patch, or shall I? I am in no big hurry on that front, at least not for this particular case, where the risk of bogus input is probably negligible. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list